create course copy and zip migration workers

Course copying and importing .zip files will now work the
same as other content migrations.

Test Plan:
 * Using the ContentMigration api test course copy and zip imports
 * Make sure the current course copy and zip import UIs still work
   * for course copy make sure selective options work

closes CNVS-4228

Change-Id: I80a849471dffaf44d683e980cf0b73505b353d83
Reviewed-on: https://gerrit.instructure.com/19740
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: James Williams  <jamesw@instructure.com>
QA-Review: Clare Strong <clare@instructure.com>
Product-Review: Bracken Mosbacker <bracken@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>

app/controllers/content_migrations_controller.rb

@@ -108,7 +108,7 @@ class ContentMigrationsController < ApplicationController
   #
   # You can use the {api:ProgressController#show Progress API} to track the
   # progress of the migration. The migration's progress is linked to with the
-  # _progress_url_ value
+  # _progress_url_ value.
   #
   # The two general workflows are:
   #
@@ -124,12 +124,16 @@ class ContentMigrationsController < ApplicationController
   # 3. {api:ContentMigrationsController#show GET} the ContentMigration
   # 4. Use the {api:ProgressController#show Progress} specified in _progress_url_ to monitor progress
   #
-  # @argument migration_type [string] The type of the migration. Allowed values: canvas_cartridge_importer, common_cartridge_importer, qti_converter, moodle_converter
+  # @argument migration_type [string] The type of the migration. Allowed values: canvas_cartridge_importer, common_cartridge_importer, course_copy_importer, zip_file_importer, qti_converter, moodle_converter
   #
   # @argument pre_attachment[name] [string] Required if uploading a file. This is the first step in uploading a file to the content migration. See the {file:file_uploads.html File Upload Documentation} for details on the file upload workflow.
   #
   # @argument pre_attachment[*] (optional) Other file upload properties, See {file:file_uploads.html File Upload Documentation}
   #
+  # @argument settings[source_course_id] [string] (optional) The course to copy from for a course copy migration. (required if doing course copy)
+  #
+  # @argument settings[folder_id] [string] (optional) The folder to unzip the .zip file into for a zip_file_import. (required if doing .zip file upload)
+  #
   # @argument settings[overwrite_quizzes] [boolean] (optional) Whether to overwrite quizzes with the same identifiers between content packages
   #
   # @argument settings[question_bank_id] [integer] (optional) The existing question bank ID to import questions into if not specified in the content package
@@ -160,15 +164,21 @@ class ContentMigrationsController < ApplicationController
   # 
   # @returns ContentMigration
   def create
-    plugin = Canvas::Plugin.find(params[:migration_type])
-    if !plugin
+    @plugin = Canvas::Plugin.find(params[:migration_type])
+    if !@plugin
       return render(:json => { :message => t('bad_migration_type', "Invalid migration_type") }, :status => :bad_request)
     end
-    if plugin.settings && plugin.settings[:requires_file_upload]
+    settings = @plugin.settings || {}
+    if settings[:requires_file_upload]
       if !params[:pre_attachment] || params[:pre_attachment][:name].blank?
         return render(:json => { :message => t('must_upload_file', "File upload is required") }, :status => :bad_request)
       end
     end
+    if validator = settings[:required_options_validator]
+      if res = validator.has_error(params[:settings], @current_user, @context)
+        return render(:json => { :message => res.respond_to?(:call) ? res.call : res }, :status => :bad_request)
+      end
+    end
 
     @content_migration = @context.content_migrations.build(:user => @current_user, :context => @context, :migration_type => params[:migration_type])
     @content_migration.workflow_state = 'created'
@@ -186,6 +196,7 @@ class ContentMigrationsController < ApplicationController
   # @returns ContentMigration
   def update
     @content_migration = @context.content_migrations.find(params[:id])
+    @plugin = Canvas::Plugin.find(@content_migration.migration_type)
 
     update_migration
   end
@@ -200,6 +211,8 @@ class ContentMigrationsController < ApplicationController
   def update_migration
     @content_migration.update_migration_settings(params[:settings]) if params[:settings]
     @content_migration.set_date_shift_options(params[:date_shift_options])
+
+    params[:selective_import] = false if @plugin.settings && @plugin.settings[:no_selective_import]
     if Canvas::Plugin.value_to_boolean(params[:selective_import])
       #todo selective import options
     else

app/models/content_migration.rb

@@ -267,6 +267,19 @@ class ContentMigration < ActiveRecord::Base
     add_warning(t('errors.import_error', "Import Error: ") + "#{item_type} - \"#{item_name}\"", warning)
   end
 
+  def fail_with_error!(exception_or_info)
+    opts={}
+    if exception_or_info.is_a?(Exception)
+      opts[:exception] = exception_or_info
+    else
+      opts[:error_message] = exception_or_info
+    end
+    add_error(t(:unexpected_error, "There was an unexpected error, please contact support."), opts)
+    self.workflow_state = :failed
+    job_progress.fail
+    save
+  end
+
   # deprecated warning format
   def old_warnings_format
     self.migration_issues.map do |mi|
@@ -425,7 +438,7 @@ class ContentMigration < ActiveRecord::Base
   end
 
   def for_course_copy?
-    !!self.source_course
+    !!self.source_course || (self.migration_type && self.migration_type == 'course_copy_importer')
   end
 
   def set_date_shift_options(opts)
@@ -439,63 +452,8 @@ class ContentMigration < ActiveRecord::Base
   end
 
   def copy_course
-    self.workflow_state = :pre_processing
-    reset_job_progress
-    self.migration_settings[:skip_import_notification] = true
-    self.save
-
-    begin
-      ce = ContentExport.new
-      ce.content_migration = self
-      ce.selected_content = copy_options
-      ce.course = self.source_course
-      ce.export_type = ContentExport::COURSE_COPY
-      ce.user = self.user
-      ce.save!
-      self.content_export = ce
-
-      ce.export_course_without_send_later
-
-      if ce.workflow_state == 'exported_for_course_copy'
-        # use the exported attachment as the import archive
-        self.attachment = ce.attachment
-        migration_settings[:migration_ids_to_import] ||= {:copy=>{}}
-        migration_settings[:migration_ids_to_import][:copy][:everything] = true
-        # set any attachments referenced in html to be copied
-        ce.selected_content['attachments'] ||= {}
-        ce.referenced_files.values.each do |att_mig_id|
-          ce.selected_content['attachments'][att_mig_id] = true
-        end
-        ce.save
-
-        self.save
-        worker = Canvas::Migration::Worker::CCWorker.new
-        worker.migration_id = self.id
-        worker.perform
-        self.reload
-        if self.workflow_state == 'exported'
-          self.workflow_state = :pre_processed
-          self.update_import_progress(10)
-
-          self.context.copy_attachments_from_course(self.source_course, :content_export => ce, :content_migration => self)
-          self.update_import_progress(20)
-
-          self.import_content_without_send_later
-        end
-      else
-        self.workflow_state = :failed
-        migration_settings[:last_error] = "ContentExport failed to export course."
-        self.save
-      end
-    rescue => e
-      self.workflow_state = :failed
-      er = ErrorReport.log_exception(:content_migration, e)
-      migration_settings[:last_error] = "ErrorReport:#{er.id}"
-      logger.error e
-      self.save
-      raise e
-    ensure
-    end
+    worker = Canvas::Migration::Worker::CourseCopyWorker.new
+    worker.perform(self)
   end
   handle_asynchronously :copy_course, :priority => Delayed::LOW_PRIORITY, :max_attempts => 1
   
@@ -570,7 +528,13 @@ class ContentMigration < ActiveRecord::Base
 
   def fast_update_progress(val)
     reset_job_progress unless job_progress
+    if val == 100
+      job_progress.completion = 100
+      job_progress.workflow_state = 'completed'
+      job_progress.save
+    else
       job_progress.update_completion!(val)
+    end
     # Until this progress is phased out
     self.progress = val
     ContentMigration.where(:id => self).update_all(:progress=>val)

app/models/progress.rb

@@ -29,6 +29,7 @@ class Progress < ActiveRecord::Base
   workflow do
     state :queued do
       event :start, :transitions_to => :running
+      event :fail, :transitions_to => :failed
     end
     state :running do
       event(:complete, :transitions_to => :completed) { update_completion! 100 }

lib/canvas/migration/validators/course_copy_validator.rb

@@ -0,0 +1,35 @@
+#
+# Copyright (C) 2013 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+module Canvas::Migration::Validators::CourseCopyValidator
+  def self.has_error(options, user, course)
+    if !options || !options[:source_course_id]
+      return I18n.t :course_copy_argument_error, 'A course copy requires a source course.'
+    end
+    source = Course.find_by_id(options[:source_course_id])
+    if source
+      if !source.grants_rights?(user, nil, :manage)
+        return I18n.t :course_copy_not_allowed_error, 'You are not allowed to copy the source course.'
+      end
+    else
+      return I18n.t :course_copy_no_course_error, 'The source course was not found.'
+    end
+
+    false
+  end
+end

lib/canvas/migration/validators/zip_importer_validator.rb

@@ -0,0 +1,30 @@
+#
+# Copyright (C) 2013 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+module Canvas::Migration::Validators::ZipImporterValidator
+  def self.has_error(options, user, course)
+    if !options || !options[:folder_id]
+      return I18n.t :zip_argument_error, 'A .zip upload requires a folder to upload to.'
+    end
+    if !course.folders.find_by_id(options[:folder_id])
+      return I18n.t :zip_no_folder_error, "The specified folder couldn't be found in this course."
+    end
+
+    false
+  end
+end

lib/canvas/migration/worker/course_copy_worker.rb

@@ -0,0 +1,87 @@
+#
+# Copyright (C) 2011 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+class Canvas::Migration::Worker::CourseCopyWorker < Struct.new(:migration_id)
+  def perform(cm=nil)
+    cm ||= ContentMigration.find migration_id
+    
+    cm.workflow_state = :pre_processing
+    cm.reset_job_progress
+    cm.migration_settings[:skip_import_notification] = true
+    cm.save
+    cm.job_progress.start
+
+    begin
+      ce = ContentExport.new
+      ce.content_migration = cm
+      ce.selected_content = cm.copy_options
+      source = cm.source_course || Course.find(cm.migration_settings[:source_course_id])
+      ce.course = source
+      ce.export_type = ContentExport::COURSE_COPY
+      ce.user = cm.user
+      ce.save!
+      cm.content_export = ce
+
+      ce.export_course_without_send_later
+
+      if ce.workflow_state == 'exported_for_course_copy'
+        # use the exported attachment as the import archive
+        cm.attachment = ce.attachment
+        cm.migration_settings[:migration_ids_to_import] ||= {:copy=>{}}
+        cm.migration_settings[:migration_ids_to_import][:copy][:everything] = true
+        # set any attachments referenced in html to be copied
+        ce.selected_content['attachments'] ||= {}
+        ce.referenced_files.values.each do |att_mig_id|
+          ce.selected_content['attachments'][att_mig_id] = true
+        end
+        ce.save
+
+        cm.save
+        worker = Canvas::Migration::Worker::CCWorker.new
+        worker.migration_id = cm.id
+        worker.perform
+        cm.reload
+        if cm.workflow_state == 'exported'
+          cm.workflow_state = :pre_processed
+          cm.update_import_progress(10)
+
+          cm.context.copy_attachments_from_course(source, :content_export => ce, :content_migration => cm)
+          cm.update_import_progress(20)
+
+          cm.import_content_without_send_later
+          cm.workflow_state = :imported
+          cm.save
+          cm.update_import_progress(100)
+        end
+      else
+        cm.workflow_state = :failed
+        cm.migration_settings[:last_error] = "ContentExport failed to export course."
+        cm.save
+      end
+    rescue => e
+      cm.fail_with_error!(e)
+      raise e
+    end
+  end
+
+  def self.enqueue(content_migration)
+    Delayed::Job.enqueue(new(content_migration.id),
+                         :priority => Delayed::LOW_PRIORITY,
+                         :max_attempts => 1,
+                         :strand => content_migration.strand)
+  end
+end

lib/canvas/migration/worker/zip_file_worker.rb

@@ -0,0 +1,64 @@
+#
+# Copyright (C) 2011 Instructure, Inc.
+#
+# This file is part of Canvas.
+#
+# Canvas is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License as published by the Free
+# Software Foundation, version 3 of the License.
+#
+# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
+# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+# details.
+#
+# You should have received a copy of the GNU Affero General Public License along
+# with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+class Canvas::Migration::Worker::ZipFileWorker < Struct.new(:migration_id)
+  def perform(cm=nil)
+    cm ||= ContentMigration.find migration_id
+
+    cm.workflow_state = :importing
+    cm.migration_settings[:skip_import_notification] = true
+    cm.job_progress.start
+    cm.save
+
+    begin
+      zipfile = cm.attachment.open(:need_local_file => true)
+      folder = cm.context.folders.find(cm.migration_settings[:folder_id])
+
+      progress = 0.0
+      update_callback = lambda{|pct|
+        if pct - progress >= 1.0
+          cm.update_import_progress(pct)
+        end
+      }
+
+      UnzipAttachment.process(
+              :context => cm.context,
+              :root_directory => folder,
+              :filename => zipfile.path,
+              :callback => update_callback
+      )
+
+      zipfile.close
+      zipfile = nil
+
+      cm.workflow_state = :imported
+      cm.save
+      cm.update_import_progress(100)
+    rescue => e
+      cm.fail_with_error!(e)
+      raise e
+    end
+
+  end
+
+  def self.enqueue(content_migration)
+    Delayed::Job.enqueue(new(content_migration.id),
+                         :priority => Delayed::LOW_PRIORITY,
+                         :max_attempts => 1,
+                         :strand => content_migration.strand)
+  end
+end

lib/canvas/plugins/default_plugins.rb

@@ -138,6 +138,39 @@ Canvas::Plugin.register 'canvas_cartridge_importer', :export_system, {
     :provides =>{:canvas_cartridge => CC::Importer::Canvas::Converter}
   },
 }
+require_dependency 'canvas/migration/worker/course_copy_worker'
+Canvas::Plugin.register 'course_copy_importer', :export_system, {
+        :name => lambda { I18n.t :course_copy_name, 'Copy Canvas Course' },
+        :author => 'Instructure',
+        :author_website => 'http://www.instructure.com',
+        :description => lambda { I18n.t :course_copy_description, 'Migration plugin for copying canvas courses' },
+        :version => '1.0.0',
+        :select_text => lambda { I18n.t :course_copy_file_description, "Copy a Canvas Course" },
+        :hide_from_users => true, # until new UI is done
+        :settings => {
+                :worker => 'CourseCopyWorker',
+                :migration_partial => '',
+                :requires_file_upload => false,
+                :required_options_validator => Canvas::Migration::Validators::CourseCopyValidator
+        },
+}
+require_dependency 'canvas/migration/worker/zip_file_worker'
+Canvas::Plugin.register 'zip_file_importer', :export_system, {
+        :name => lambda { I18n.t :zip_file_name, 'Copy Canvas Course' },
+        :author => 'Instructure',
+        :author_website => 'http://www.instructure.com',
+        :description => lambda { I18n.t :zip_file_description, 'Migration plugin for unpacking plain .zip files into a course' },
+        :version => '1.0.0',
+        :select_text => lambda { I18n.t :zip_file_file_description, "Import plain files from a .zip" },
+        :hide_from_users => true, # until new UI is done
+        :settings => {
+                :worker => 'ZipFileWorker',
+                :migration_partial => '',
+                :requires_file_upload => true,
+                :no_selective_import => true,
+                :required_options_validator => Canvas::Migration::Validators::ZipImporterValidator
+        },
+}
 Canvas::Plugin.register 'common_cartridge_importer', :export_system, {
   :name => lambda{ I18n.t :common_cartridge_name, 'Common Cartridge Importer' },
   :author => 'Instructure',

lib/cc/importer/cc_worker.rb

@@ -18,6 +18,7 @@
 class Canvas::Migration::Worker::CCWorker < Struct.new(:migration_id)
   def perform
     cm = ContentMigration.find_by_id migration_id
+    cm.job_progress.start
     begin
       cm.update_conversion_progress(1)
       settings = cm.migration_settings.clone
@@ -41,7 +42,6 @@ class Canvas::Migration::Worker::CCWorker < Struct.new(:migration_id)
       if export_folder_path
         Canvas::Migration::Worker::upload_exported_data(export_folder_path, cm)
         Canvas::Migration::Worker::clear_exported_data(export_folder_path)
-        cm.update_conversion_progress(100)
       end
 
       cm.migration_settings[:worker_class] = converter_class.name
@@ -50,6 +50,7 @@ class Canvas::Migration::Worker::CCWorker < Struct.new(:migration_id)
       end
       cm.workflow_state = :exported
       saved = cm.save
+      cm.update_conversion_progress(100)
 
       if cm.import_immediately?
         cm.import_content_without_send_later
@@ -61,13 +62,13 @@ class Canvas::Migration::Worker::CCWorker < Struct.new(:migration_id)
       end
 
       saved
-    rescue => e
-      report = ErrorReport.log_exception(:content_migration, e)
-      if cm
+    rescue Canvas::Migration::Error
+      cm.add_error($!.message, :exception => $!)
       cm.workflow_state = :failed
-        cm.migration_settings[:last_error] = "ErrorReport:#{report.id}"
+      cm.job_progress.fail
       cm.save
-      end
+    rescue => e
+      cm.fail_with_error!(e) if cm
     end
   end
 

spec/apis/v1/content_migrations_api_spec.rb

@@ -137,6 +137,16 @@ describe ContentMigrationsController, :type => :integration do
       migration.job_progress.should be_nil
     end
     
+    it "should error if expected setting isn't set" do
+      json = api_call(:post, @migration_url, @params, {:migration_type => 'course_copy_importer'}, {}, :expected_status => 400)
+      json.should == {"message"=>'A course copy requires a source course.'}
+    end
+    
+    it "should queue if correct settings set" do
+      # implicitly tests that the response was a 200
+      api_call(:post, @migration_url, @params, {:migration_type => 'course_copy_importer', :settings => {:source_course_id => @course.id.to_param}})
+    end
+
     context "migration file upload" do
       it "should set attachment pre-flight data" do
         json = api_call(:post, @migration_url, @params, @post_params)

spec/lib/cc/importer/cc_worker_spec.rb

@@ -21,6 +21,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper.rb')
 describe Canvas::Migration::Worker::CCWorker do
   it "should set the worker_class on the migration" do
     cm = ContentMigration.create!(:migration_settings => { :no_archive_file => true })
+    cm.reset_job_progress
     Canvas::Migration::Worker.expects(:get_converter).with(anything).returns(CC::Importer::Canvas::Converter)
     CC::Importer::Canvas::Converter.any_instance.expects(:export).returns({})
     worker = Canvas::Migration::Worker::CCWorker.new(cm.id)

spec/models/content_migration_spec.rb

@@ -1753,4 +1753,29 @@ equation: <img class="equation_image" title="Log_216" src="/equation_images/Log_
     ContentMigration.migration_plugins(true).include?(ab).should be_false
   end
 
+  context "zip file import" do
+    it "should import" do
+      course_with_teacher
+      zip_path = File.join(File.dirname(__FILE__) + "/../fixtures/migration/file.zip")
+      cm = ContentMigration.new(:context => @course, :user => @user,)
+      cm.migration_type = 'zip_file_importer'
+      cm.migration_settings[:folder_id] = Folder.root_folders(@course).first.id
+      cm.save!
+
+      attachment = Attachment.new
+      attachment.context = cm
+      attachment.uploaded_data = File.open(zip_path, 'rb')
+      attachment.filename = 'file.zip'
+      attachment.save!
+
+      cm.attachment = attachment
+      cm.save!
+
+      cm.queue_migration
+      run_jobs
+      @course.reload
+      @course.attachments.count.should == 1
+    end
+  end
+
 end

vendor/plugins/academic_benchmark/lib/academic_benchmark.rb

@@ -32,7 +32,7 @@ module AcademicBenchmark
 
   def self.queue_migration_for_guid(guid, user)
     if !Account.site_admin.grants_right?(user, :manage_global_outcomes)
-      raise Canvas::Migration::Error.new("User isn't allowed to edit global outcomes")
+      raise Canvas::Migration::Error.new(I18n.t('academic_benchmark.no_permissions', "User isn't allowed to edit global outcomes"))
     end
 
     cm = ContentMigration.create(:context => Account.site_admin)

vendor/plugins/academic_benchmark/lib/academic_benchmark/converter.rb

@@ -17,7 +17,7 @@ module AcademicBenchmark
 
     def export
       if content_migration && !Account.site_admin.grants_right?(content_migration.user, :manage_global_outcomes)
-        raise Canvas::Migration::Error.new("User isn't allowed to edit global outcomes")
+        raise Canvas::Migration::Error.new(I18n.t('academic_benchmark.no_perms', "User isn't allowed to edit global outcomes"))
       end
 
       if @archive_file
@@ -31,14 +31,10 @@ module AcademicBenchmark
             convert_guids(@settings[:guids]) if @settings[:guids]
           end
         else
-          message = I18n.t('academic_benchmark.no_api_key', "An API key is required to use Academic Benchmarks")
-          add_warning(message)
-          raise Canvas::Migration::Error.new("no academic benchmarks api key")
+          raise Canvas::Migration::Error.new(I18n.t('academic_benchmark.no_api_key', "An API key is required to use Academic Benchmarks"))
         end
       else
-        message = I18n.t('academic_benchmark.no_file', "No outcome file or authority given")
-        add_warning(message)
-        raise Canvas::Migration::Error.new("No outcome file or authority given")
+        raise Canvas::Migration::Error.new(I18n.t('academic_benchmark.no_file', "No outcome file or authority given"))
       end
 
       save_to_file

vendor/plugins/academic_benchmark/spec_canvas/academic_benchmark_spec.rb

@@ -98,8 +98,7 @@ describe AcademicBenchmark::Converter do
     @cm.export_content
     run_jobs
     @cm.reload
-    @cm.old_warnings_format.should == []
-    @cm.migration_settings[:last_error].should be_nil
+    @cm.migration_issues.count.should == 0
     @cm.workflow_state.should == 'imported'
 
     verify_full_import()
@@ -112,8 +111,9 @@ describe AcademicBenchmark::Converter do
     run_jobs
     @cm.reload
 
+    @cm.migration_issues.count.should == 1
+    @cm.migration_issues.first.description.should == "User isn't allowed to edit global outcomes"
     @cm.workflow_state.should == 'failed'
-    @cm.migration_settings[:last_error].should =~ /ErrorReport:\d+/
   end
 
   it "should fail if no file or authority set" do
@@ -125,8 +125,8 @@ describe AcademicBenchmark::Converter do
     run_jobs
     @cm.reload
 
-    @cm.old_warnings_format.should == [["No outcome file or authority given", ""]]
-    @cm.migration_settings[:last_error].should_not be_nil
+    @cm.migration_issues.count.should == 1
+    @cm.migration_issues.first.description.should == "No outcome file or authority given"
     @cm.workflow_state.should == 'failed'
   end
 
@@ -144,8 +144,7 @@ describe AcademicBenchmark::Converter do
       run_jobs
       @cm.reload
 
-      @cm.old_warnings_format.should == []
-      @cm.migration_settings[:last_error].should be_nil
+      @cm.migration_issues.count.should == 0
       @cm.workflow_state.should == 'imported'
 
       @root_group = LearningOutcomeGroup.global_root_outcome_group
@@ -160,8 +159,8 @@ describe AcademicBenchmark::Converter do
       run_jobs
       @cm.reload
 
-      @cm.old_warnings_format.should == [["An API key is required to use Academic Benchmarks", ""]]
-      @cm.migration_settings[:last_error].should_not be_nil
+      @cm.migration_issues.count.should == 1
+      @cm.migration_issues.first.description.should == "An API key is required to use Academic Benchmarks"
       @cm.workflow_state.should == 'failed'
     end
 

vendor/plugins/qti_exporter/lib/workers/qti_worker.rb

@@ -44,12 +44,7 @@ module Canvas::Migration
           cm.save
           cm.update_import_progress(100)
         rescue => e
-          report = ErrorReport.log_exception(:content_migration, e)
-          if cm
-            cm.workflow_state = :failed
-            cm.migration_settings[:last_error] = "ErrorReport:#{report.id}"
-            cm.save
-          end
+          cm.fail_with_error!(e) if cm
         end
       end