Outcomes import job runner with status and errors

Fixes OUT-1997

Test Plan:
- On an account with no outcomes,
- In the rails console, run the following:
  Attachment.skip_3rd_party_submits(true)
  a = Attachment.new
  a.context = Account.first
  path = 'spec/lib/outcomes/fixtures/demo.csv'
  a.uploaded_data = File.open(path,'rb')
  a.display_name = 'qa-test-outcomes-import'
  a.save!
  i = OutcomeImport.create!(
    context: Account.first,
    workflow_state: :created,
    attachment: a
  )
  i.run
- You should see a bunch of sql commands. Now run:
  i.reload
  i.progress # should be 100
  i.workflow_state # should be "succeeded"

Change-Id: I90a2e7808fcf1baf662832e5955a41c3b36e3add
Reviewed-on: https://gerrit.instructure.com/142220
Reviewed-by: Neil Gupta <[email protected]>
Tested-by: Jenkins
Product-Review: Neil Gupta <[email protected]>
QA-Review: Neil Gupta <[email protected]>

app/models/outcome_import.rb

@@ -95,4 +95,37 @@ def as_json(_options={})
     data["processing_errors"] = self.outcome_import_errors.limit(25).pluck(:row, :message)
     data
   end
+
+  def run
+    job_started!
+    file = self.attachment.open(need_local_file: true)
+    begin
+      Outcomes::CsvImporter.new(self, file).run do |status|
+        status[:errors].each do |row, error|
+          add_error row, error
+        end
+        self.update!(progress: status[:progress])
+      end
+
+      if outcome_import_errors.count == 0
+        job_completed!
+      else
+        job_failed!
+      end
+    rescue
+      add_error(1, I18n.t('An unexpected error has occurred'))
+      job_failed!
+      raise
+    ensure
+      file.close
+    end
+  end
+
+  def add_error(row, error)
+    OutcomeImportError.create!(
+      outcome_import: self,
+      row: row,
+      message: error
+    )
+  end
 end

lib/outcomes/csv_importer.rb

@@ -40,25 +40,41 @@ class CsvImporter
 
     BATCH_SIZE = 1000
 
-    def initialize(path, context)
-      @path = path
-      @context = context
+    def initialize(import, file)
+      @import = import
+      @file = file
     end
 
-    def run
-      headers = nil
-      errors = []
+    delegate :context, to: :@import
+
+    def run(&update)
+      status = { progress: 0, errors: [] }
+      yield status
+
       begin
-        rows = CSV.new(File.new(@path, 'rb')).to_enum
-        rows.with_index(1).each_slice(BATCH_SIZE) do |batch|
-          headers ||= validate_headers(*batch.shift)
-          errors += parse_batch(headers, batch)
-        end
+        parse_file(&update)
       rescue ParseError => e
-        return [[1, e.message]]
+        status = {
+          errors: [[1, e.message]],
+          progress: 100,
+        }
+        yield status
       end
+    end
 
-      errors
+    def parse_file
+      headers = nil
+      total = file_line_count
+      rows = CSV.new(@file).to_enum
+      rows.with_index(1).each_slice(BATCH_SIZE) do |batch|
+        headers ||= validate_headers(*batch.shift)
+        errors = parse_batch(headers, batch)
+        status = {
+          errors: errors,
+          progress: (batch.last[1].to_f / total * 100).floor
+        }
+        yield status
+      end
     end
 
     def parse_batch(headers, batch)
@@ -83,6 +99,12 @@ def parse_batch(headers, batch)
 
     private
 
+    def file_line_count
+      count = @file.each.inject(0) { |c, _line| c + 1}
+      @file.rewind
+      count
+    end
+
     def check_encoding(str)
       encoded = str&.force_encoding('utf-8')
       valid = (encoded || '').valid_encoding?

lib/outcomes/import.rb

@@ -57,7 +57,7 @@ def import_group(group)
       parent = parents.first
 
       LearningOutcomeGroup.create!(
-        context: @context,
+        context: context,
         vendor_guid: group[:vendor_guid],
         title: group[:title],
         description: group[:description] || '',
@@ -70,7 +70,7 @@ def import_outcome(outcome)
       parents = find_parents(outcome)
 
       imported = LearningOutcome.new(
-        context: @context,
+        context: context,
         title: outcome[:title],
         vendor_guid: outcome[:vendor_guid],
         description: outcome[:description] || '',
@@ -94,7 +94,7 @@ def create_rubric(ratings)
     end
 
     def root_parent
-      @root ||= LearningOutcomeGroup.find_or_create_root(@context, true)
+      @root ||= LearningOutcomeGroup.find_or_create_root(context, true)
     end
 
     def find_parents(object)

spec/lib/outcomes/csv_importer_spec.rb

@@ -18,8 +18,69 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 
 describe Outcomes::CsvImporter do
-  def csv_path(name)
-    File.expand_path(File.dirname(__FILE__) + "/fixtures/#{name}.csv")
+  def csv_file(name)
+    path = File.expand_path(File.dirname(__FILE__) + "/fixtures/#{name}.csv")
+    File.open(path, 'rb')
+  end
+
+  def import_fake_csv(rows, &updates)
+    Tempfile.open do |tf|
+      CSV.open(tf.path, 'wb') do |csv|
+        rows.each { |r| csv << r }
+      end
+      tf.binmode
+      Outcomes::CsvImporter.new(import, tf).run(&updates)
+    end
+  end
+
+  def outcome_row(**changes)
+    valid = {
+      title: 'title',
+      vendor_guid: SecureRandom.uuid,
+      object_type: 'outcome',
+      parent_guids: '',
+      calculation_method: 'highest',
+      calculation_int: '',
+      workflow_state: ''
+    }
+
+    row = valid.merge(changes)
+    headers.map { |k| row[k.to_sym] }
+  end
+
+  def group_row(**changes)
+    valid = {
+      title: 'title',
+      vendor_guid: SecureRandom.uuid,
+      object_type: 'group',
+      parent_guids: '',
+      calculation_method: '',
+      calculation_int: '',
+      workflow_state: ''
+    }
+
+    row = valid.merge(changes)
+    headers.map { |k| row[k.to_sym] }
+  end
+
+  before :once do
+    account_model
+  end
+
+  let(:import) do
+    OutcomeImport.create!(context: @account)
+  end
+
+  let(:headers) do
+    %w[
+      title
+      vendor_guid
+      object_type
+      parent_guids
+      calculation_method
+      calculation_int
+      workflow_state
+    ]
   end
 
   describe 'the csv importer' do
@@ -30,18 +91,19 @@ def csv_path(name)
     end
 
     def expect_ok_import(path)
-      errors = Outcomes::CsvImporter.new(path, nil).run
-      expect(errors).to eq([])
+      Outcomes::CsvImporter.new(import, path).run do |status|
+        expect(status[:errors]).to eq([])
+      end
     end
 
     it 'can import the demo csv file' do
-      expect_ok_import(csv_path('demo'))
+      expect_ok_import(csv_file('demo'))
       expect(LearningOutcomeGroup.count).to eq(3)
       expect(LearningOutcome.count).to eq(1)
     end
 
     it 'imports group attributes correctly' do
-      expect_ok_import(csv_path('demo'))
+      expect_ok_import(csv_file('demo'))
 
       group = by_guid['b']
       expect(group.title).to eq('B')
@@ -50,7 +112,7 @@ def expect_ok_import(path)
     end
 
     it 'imports outcome attributes correctly' do
-      expect_ok_import(csv_path('demo'))
+      expect_ok_import(csv_file('demo'))
 
       outcome = by_guid['c']
       expect(outcome.title).to eq('C')
@@ -66,7 +128,7 @@ def expect_ok_import(path)
     end
 
     it 'imports ratings correctly' do
-      expect_ok_import(csv_path('scoring'))
+      expect_ok_import(csv_file('scoring'))
 
       criteria = by_guid['c'].rubric_criterion
       ratings = criteria[:ratings].sort_by { |r| r[:points] }
@@ -80,13 +142,13 @@ def expect_ok_import(path)
     end
 
     it 'works when no ratings are present' do
-      expect_ok_import(csv_path('no-ratings'))
+      expect_ok_import(csv_file('no-ratings'))
 
       expect(by_guid['c'].rubric_criterion).to eq(nil)
     end
 
     it 'properly sets scoring types' do
-      expect_ok_import(csv_path('scoring'))
+      expect_ok_import(csv_file('scoring'))
 
       by_method = LearningOutcome.all.to_a.group_by(&:calculation_method)
 
@@ -99,28 +161,32 @@ def expect_ok_import(path)
 
     it 'can import a utf-8 csv file with non-ascii characters' do
       guid = 'søren'
-      expect_ok_import(csv_path('nor'))
+      expect_ok_import(csv_file('nor'))
       expect(LearningOutcomeGroup.where(vendor_guid: guid).count).to eq(1)
     end
 
     it 'can import csv files with chinese characters' do
       guid = '作戰'
-      expect_ok_import(csv_path('chn'))
+      expect_ok_import(csv_file('chn'))
       expect(LearningOutcomeGroup.where(vendor_guid: guid).count).to eq(1)
     end
-  end
 
-  def import_fake_csv(rows)
-    Tempfile.open do |tf|
-      CSV.open(tf.path, 'wb') do |csv|
-        rows.each { |r| csv << r }
+    it 'reports import progress' do
+      stub_const('Outcomes::CsvImporter::BATCH_SIZE', 2)
+
+      increments = []
+      import_fake_csv([headers] + (1..3).map { |ix| group_row(vendor_guid: ix) }.to_a) do |status|
+        increments.push(status[:progress])
       end
-      Outcomes::CsvImporter.new(tf.path, nil).run
+      expect(increments).to eq([0, 50, 100])
     end
   end
 
   def expect_import_error(rows, expected)
-    errors = import_fake_csv(rows)
+    errors = []
+    import_fake_csv(rows) do |status|
+      errors += status[:errors]
+    end
     expect(errors).to eq(expected)
   end
 
@@ -148,47 +214,6 @@ def expect_import_error(rows, expected)
   end
 
   describe 'throws user-friendly row errors' do
-    let(:headers) do
-      %w[
-        title
-        vendor_guid
-        object_type
-        parent_guids
-        calculation_method
-        calculation_int
-        workflow_state
-      ]
-    end
-
-    def outcome_row(**changes)
-      valid = {
-        title: 'title',
-        vendor_guid: SecureRandom.uuid,
-        object_type: 'outcome',
-        parent_guids: '',
-        calculation_method: 'highest',
-        calculation_int: '',
-        workflow_state: ''
-      }
-
-      row = valid.merge(changes)
-      headers.map { |k| row[k.to_sym] }
-    end
-
-    def group_row(**changes)
-      valid = {
-        title: 'title',
-        vendor_guid: SecureRandom.uuid,
-        object_type: 'group',
-        parent_guids: '',
-        calculation_method: '',
-        calculation_int: '',
-        workflow_state: ''
-      }
-
-      row = valid.merge(changes)
-      headers.map { |k| row[k.to_sym] }
-    end
 
     it 'if rating tiers have points missing' do
       expect_import_error(