Move duplicated code to service [SCI-1665]

Gemfile

@@ -73,6 +73,7 @@ gem 'ruby-graphviz', '~> 1.2' # Graphviz for rails
 gem 'tinymce-rails', '~> 4.5.7' # Rich text editor
 
 gem 'base62' # Used for smart annotations
+gem 'newrelic_rpm'
 
 group :development, :test do
   gem 'byebug'
@@ -85,7 +86,6 @@ group :development, :test do
 end
 
 group :production do
-  gem 'newrelic_rpm'
   gem 'puma'
   gem 'rails_12factor'
 end

app/controllers/repositories_controller.rb

@@ -205,8 +205,7 @@ class RepositoriesController < ApplicationController
           return repository_response(t('teams.parse_sheet.errors.empty_file'))
         end
 
-        @temp_file = parsed_file.generate_temp_file
+        if (@temp_file = parsed_file.generate_temp_file)
-        if @temp_file
           respond_to do |format|
             format.json do
               render json: {

app/controllers/teams_controller.rb

@@ -17,30 +17,10 @@ class TeamsController < ApplicationController
     end
 
     begin
-      sheet = Team.open_spreadsheet(params[:file])
+      sheet = SpreadsheetParser.open_spreadsheet(params[:file])
-      # Get data (it will trigger any errors as well)
+      @header, @columns = SpreadsheetParser.first_two_rows(sheet)
-      if sheet.is_a?(Roo::CSV)
-        @header = sheet.row(1)
-        @columns = sheet.row(2)
-      elsif sheet.is_a?(Roo::Excelx)
-        i = 1
-        sheet.each_row_streaming do |row|
-          @header = row.map(&:cell_value) if i == 1
-          @columns = row.map(&:cell_value) if i == 2
-          i += 1
-          break if i > 2
-        end
-      else
-        i = 1
-        sheet.rows.each do |row|
-          @header = row.values if i == 1
-          @columns = row.values if i == 2
-          i += 1
-          break if i > 2
-        end
-      end
 
-      unless @header && @header.any? && @columns && @columns.any?
+      unless @header.any? && @columns.any?
         return parse_sheet_error(t('teams.parse_sheet.errors.empty_file'))
       end
 
@@ -93,7 +73,7 @@ class TeamsController < ApplicationController
           if @temp_file.session_id == session.id
             # Check if mappings exists or else we don't have anything to parse
             if params[:mappings]
-              @sheet = Team.open_spreadsheet(@temp_file.file)
+              @sheet = SpreadsheetParser.open_spreadsheet(@temp_file.file)
 
               # Check for duplicated values
               h1 = params[:mappings].clone.delete_if { |k, v| v.empty? }

app/models/repository.rb

@@ -58,17 +58,6 @@ class Repository < ActiveRecord::Base
     end
   end
 
-  def open_spreadsheet(file)
-    filename = file.original_filename
-    file_path = file.path
-
-    if file.class == Paperclip::Attachment && file.is_stored_on_s3?
-      fa = file.fetch
-      file_path = fa.path
-    end
-    generate_file(filename, file_path)
-  end
-
   def available_repository_fields
     fields = {}
     # First and foremost add record name
@@ -131,13 +120,7 @@ class Repository < ActiveRecord::Base
     unless col_compact.map(&:id).uniq.length == col_compact.length
       return { status: :error, nr_of_added: nr_of_added, total_nr: total_nr }
     end
-    rows = if sheet.is_a?(Roo::CSV)
+    rows = SpreadsheetParser.spreadsheet_enumerator(sheet)
-             sheet
-           elsif sheet.is_a?(Roo::Excelx)
-             sheet.each_row_streaming
-           else
-             sheet.rows
-           end
 
     # Now we can iterate through record data and save stuff into db
     rows.each do |row|
@@ -202,25 +185,4 @@ class Repository < ActiveRecord::Base
     end
     { status: :ok, nr_of_added: nr_of_added, total_nr: total_nr }
   end
-
-  private
-
-  def generate_file(filename, file_path)
-    case File.extname(filename)
-    when '.csv'
-      Roo::CSV.new(file_path, extension: :csv)
-    when '.tsv'
-      Roo::CSV.new(file_path, csv_options: { col_sep: "\t" })
-    when '.txt'
-      # This assumption is based purely on biologist's habits
-      Roo::CSV.new(file_path, csv_options: { col_sep: "\t" })
-    when '.xlsx'
-      # Roo Excel parcel was replaced with Creek, but it can be enabled back,
-      # just swap lines below. But only one can be enabled at the same time.
-      # Roo::Excelx.new(file_path)
-      Creek::Book.new(file_path).sheets[0]
-    else
-      raise TypeError
-    end
-  end
 end

app/models/team.rb

@@ -26,33 +26,6 @@ class Team < ActiveRecord::Base
   has_many :protocol_keywords, inverse_of: :team, dependent: :destroy
   has_many :tiny_mce_assets, inverse_of: :team, dependent: :destroy
   has_many :repositories, dependent: :destroy
-  # Based on file's extension opens file (used for importing)
-  def self.open_spreadsheet(file)
-    filename = file.original_filename
-    file_path = file.path
-
-    if file.class == Paperclip::Attachment and file.is_stored_on_s3?
-      fa = file.fetch
-      file_path = fa.path
-    end
-
-    case File.extname(filename)
-    when '.csv' then
-      Roo::CSV.new(file_path, extension: :csv)
-    when '.tsv' then
-      Roo::CSV.new(file_path, csv_options: { col_sep: "\t" })
-    when '.txt' then
-      # This assumption is based purely on biologist's habits
-      Roo::CSV.new(file_path, csv_options: { col_sep: "\t" })
-    when '.xlsx' then
-      # Roo Excel parcel was replaced with Creek, but it can be enabled back,
-      # just swap lines below. But only one can be enabled at the same time.
-      # Roo::Excelx.new(file_path)
-      Creek::Book.new(file_path).sheets[0]
-    else
-      raise TypeError
-    end
-  end
 
   def search_users(query = nil)
     a_query = "%#{query}%"
@@ -96,13 +69,7 @@ class Team < ActiveRecord::Base
       end
     end
 
-    rows = if sheet.is_a?(Roo::CSV)
+    rows = SpreadsheetParser.spreadsheet_enumerator(sheet)
-             sheet
-           elsif sheet.is_a?(Roo::Excelx)
-             sheet.each_row_streaming
-           else
-             sheet.rows
-           end
 
     # Now we can iterate through sample data and save stuff into db
     rows.each do |row|

app/services/import_repository/import_records.rb

@@ -17,9 +17,11 @@ module ImportRepository
     private
 
     def run_import_actions
-      @repository.import_records(@repository.open_spreadsheet(@temp_file.file),
+      @repository.import_records(
-                                 @mappings,
+        SpreadsheetParser.open_spreadsheet(@temp_file.file),
-                                 @user)
+        @mappings,
+        @user
+      )
     end
 
     def run_checks

app/services/import_repository/parse_repository.rb

@@ -5,37 +5,15 @@ module ImportRepository
       @file = options.fetch(:file)
       @repository = options.fetch(:repository)
       @session = options.fetch(:session)
-      @sheet = @repository.open_spreadsheet(@file)
+      @sheet = SpreadsheetParser.open_spreadsheet(@file)
     end
 
     def data
-      # Get data (it will trigger any errors as well)
+      header, columns = SpreadsheetParser.first_two_rows(@sheet)
-      if @sheet.is_a?(Roo::CSV)
-        header = @sheet.row(1)
-        columns = @sheet.row(2)
-      elsif @sheet.is_a?(Roo::Excelx)
-        i = 1
-        @sheet.each_row_streaming do |row|
-          header = row.map(&:cell_value) if i == 1
-          columns = row.map(&:cell_value) if i == 2
-          i += 1
-          break if i > 2
-        end
-      else
-        i = 1
-        @sheet.rows.each do |row|
-          header = row.values if i == 1
-          columns = row.values if i == 2
-          i += 1
-          break if i > 2
-        end
-      end
       # Fill in fields for dropdown
       @repository.available_repository_fields.transform_values! do |name|
         truncate(name, length: Constants::NAME_TRUNCATION_LENGTH_DROPDOWN)
       end
-      header ||= []
-      columns ||= []
       Data.new(header,
                columns,
                @repository.available_repository_fields,

app/services/spreadsheet_parser.rb

@@ -0,0 +1,55 @@
+class SpreadsheetParser
+  # Based on file's extension opens file (used for importing)
+  def self.open_spreadsheet(file)
+    filename = file.original_filename
+    file_path = file.path
+
+    if file.class == Paperclip::Attachment && file.is_stored_on_s3?
+      fa = file.fetch
+      file_path = fa.path
+    end
+
+    case File.extname(filename)
+    when '.csv'
+      Roo::CSV.new(file_path, extension: :csv)
+    when '.tsv'
+      Roo::CSV.new(file_path, csv_options: { col_sep: "\t" })
+    when '.txt'
+      # This assumption is based purely on biologist's habits
+      Roo::CSV.new(file_path, csv_options: { col_sep: "\t" })
+    when '.xlsx'
+      # Roo Excel parcel was replaced with Creek, but it can be enabled back,
+      # just swap lines below. But only one can be enabled at the same time.
+      # Roo::Excelx.new(file_path)
+      Creek::Book.new(file_path).sheets[0]
+    else
+      raise TypeError
+    end
+  end
+
+  def self.spreadsheet_enumerator(sheet)
+    if sheet.is_a?(Roo::CSV)
+      sheet
+    elsif sheet.is_a?(Roo::Excelx)
+      sheet.each_row_streaming
+    else
+      sheet.rows
+    end
+  end
+
+  def self.first_two_rows(sheet)
+    rows = spreadsheet_enumerator(sheet)
+    header = []
+    columns = []
+    i = 1
+    rows.each do |row|
+      # Creek XLSX parser returns Hash of the row, Roo - Array
+      row = row.is_a?(Hash) ? row.values.map(&:to_s) : row.map(&:to_s)
+      header = row if i == 1 && row
+      columns = row if i == 2 && row
+      i += 1
+      break if i > 2
+    end
+    return header, columns
+  end
+end