create multi_term_batch_mode

fixes CNVS-39621

test plan
 - run multi_term_batch_mode sis imports
 - it should remove excluded items from all
   included terms
 - run rake doc:api
 - api docs and sis_csv doc should generate and be
   clear

Change-Id: I315b8bf4a78422a8b9f96cf330da6aa1553cacb8
Reviewed-on: https://gerrit.instructure.com/127686
Tested-by: Jenkins
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>

app/controllers/sis_imports_api_controller.rb

@@ -420,22 +420,28 @@ class SisImportsApiController < ApplicationController
           batch_mode_term = api_find(@account.enrollment_terms.active,
                                      params[:batch_mode_term_id])
         end
-        unless batch_mode_term
+        unless batch_mode_term || params[:multi_term_batch_mode]
           return render :json => {:message => "Batch mode specified, but the given batch_mode_term_id cannot be found."}, :status => :bad_request
         end
       end
 
       batch = SisBatch.create_with_attachment(@account, params[:import_type], file_obj, @current_user) do |batch|
         batch.change_threshold = params[:change_threshold]
+        batch.options ||= {}
         if batch_mode_term
           batch.batch_mode = true
           batch.batch_mode_term = batch_mode_term
+        elsif params[:multi_term_batch_mode]
+          batch.batch_mode=true
+          batch.options[:multi_term_batch_mode] = value_to_boolean(params[:multi_term_batch_mode])
+          unless batch.change_threshold
+            return render json: {message: "change_threshold is required to use multi term_batch mode."}, status: :bad_request
+          end
         elsif params[:diffing_data_set_identifier].present?
           batch.enable_diffing(params[:diffing_data_set_identifier],
                                remaster: value_to_boolean(params[:diffing_remaster_data_set]))
         end
 
-        batch.options ||= {}
         if value_to_boolean(params[:override_sis_stickiness])
           batch.options[:override_sis_stickiness] = true
           [:add_sis_stickiness, :clear_sis_stickiness].each do |option|

app/models/sis_batch.rb

@@ -191,7 +191,7 @@ class SisBatch < ActiveRecord::Base
   end
 
   def batch_aborted(message)
-    add_errors([message])
+    add_errors([[message]])
     self.save!
     raise SisBatch::Aborted
   end
@@ -308,10 +308,23 @@ class SisBatch < ActiveRecord::Base
     end
   end
 
+  def batch_mode_terms
+    if self.options[:multi_term_batch_mode]
+      @terms ||= EnrollmentTerm.where(sis_batch_id: self)
+      unless @terms.exists?
+        abort_batch
+        batch_aborted(t('Terms not found. Terms must be included with multi_term_batch_mode'))
+      end
+      @terms
+    else
+      self.batch_mode_term
+    end
+  end
+
   def term_course_scope
     if data[:supplied_batches].include?(:course)
       scope = account.all_courses.active.where.not(sis_batch_id: nil, sis_source_id: nil)
-      scope.where(enrollment_term_id: self.batch_mode_term)
+      scope.where(enrollment_term_id: batch_mode_terms)
     end
   end
 
@@ -340,7 +353,7 @@ class SisBatch < ActiveRecord::Base
 
   def term_sections_scope
     if data[:supplied_batches].include?(:section)
-      scope = self.account.course_sections.active.where(courses: {enrollment_term_id: self.batch_mode_term})
+      scope = self.account.course_sections.active.where(courses: {enrollment_term_id: batch_mode_terms})
       scope = scope.where.not(sis_batch_id: nil, sis_source_id: nil)
       scope.joins("INNER JOIN #{Course.quoted_table_name} ON courses.id=COALESCE(nonxlist_course_id, course_id)").readonly(false)
     end
@@ -369,7 +382,7 @@ class SisBatch < ActiveRecord::Base
   def term_enrollments_scope
     if data[:supplied_batches].include?(:enrollment)
       scope = self.account.enrollments.active.joins(:course).readonly(false).where.not(sis_batch_id: nil)
-      scope.where(courses: {enrollment_term_id: self.batch_mode_term})
+      scope.where(courses: {enrollment_term_id: batch_mode_terms})
     end
   end
 
@@ -394,17 +407,18 @@ class SisBatch < ActiveRecord::Base
 
   def remove_previous_imports
     # we shouldn't be able to get here without a term, but if we do, skip
-    return unless self.batch_mode_term
+    return unless self.batch_mode_term || options[:multi_term_batch_mode]
     supplied_batches = data[:supplied_batches].dup.keep_if { |i| [:course, :section, :enrollment].include? i }
     return unless supplied_batches.present?
-    SisBatch.where(id: self).update_all(workflow_state: 'cleanup_batch')
-
-    count = 0
-    courses = non_batch_courses_scope
-    sections = non_batch_sections_scope
-    enrollments = non_batch_enrollments_scope
-
     begin
+      batch_mode_terms if options[:multi_term_batch_mode]
+      SisBatch.where(id: self).update_all(workflow_state: 'cleanup_batch')
+
+      count = 0
+      courses = non_batch_courses_scope
+      sections = non_batch_sections_scope
+      enrollments = non_batch_enrollments_scope
+
       count = detect_changes(count, courses, enrollments, sections)
       row = remove_non_batch_courses(courses, count) if courses
       row = remove_non_batch_sections(sections, count, row) if sections
@@ -449,8 +463,8 @@ class SisBatch < ActiveRecord::Base
   end
 
   def change_detected_message(count, type)
-    [t("%{count} %{type} would be deleted and exceeds the set threshold of %{change_threshold}%",
-       count: count, type: type, change_threshold: change_threshold)]
+    t("%{count} %{type} would be deleted and exceeds the set threshold of %{change_threshold}%",
+      count: count, type: type, change_threshold: change_threshold)
   end
 
   def as_json(options={})

doc/api/sis_csv.md

@@ -48,6 +48,15 @@ The change_threshold can be set to any integer between 1 and 100.
 
 change_threshold also impacts diffing mode.
 
+Multi Term Batch Mode
+---------------------
+
+Multi term batch mode is just like batch mode except against multiple terms.
+Multi term batch mode is run against all terms included in the same import for
+the batch. To use multi term batch mode you must also set a change_threshold. If
+you intend to remove all items with multi term batch mode, you can set the
+change_threshold to 100.
+
 Diffing Mode
 ------------
 

spec/apis/v1/sis_imports_api_spec.rb

@@ -248,6 +248,31 @@ describe SisImportsApiController, type: :request do
     expect(batch.change_threshold).to eq 7
   end
 
+  it "should requre change threshold for multi_term_batch_mode" do
+    json = api_call(:post,
+          "/api/v1/accounts/#{@account.id}/sis_imports.json",
+          { controller: 'sis_imports_api', action: 'create',
+            format: 'json', account_id: @account.id.to_s },
+          { import_type: 'instructure_csv',
+            attachment: fixture_file_upload("files/sis/test_user_1.csv", 'text/csv'),
+            multi_term_batch_mode: '1'})
+    expect(json['message']).to eq 'change_threshold is required to use multi term_batch mode.'
+  end
+
+  it "should use multi_term_batch_mode" do
+    json = api_call(:post,
+          "/api/v1/accounts/#{@account.id}/sis_imports.json",
+          { controller: 'sis_imports_api', action: 'create',
+            format: 'json', account_id: @account.id.to_s },
+          { import_type: 'instructure_csv',
+            attachment: fixture_file_upload("files/sis/test_user_1.csv", 'text/csv'),
+            batch_mode: '1',
+            multi_term_batch_mode: '1',
+            change_threshold: 7,})
+    batch = SisBatch.find(json["id"])
+    expect(batch.options[:multi_term_batch_mode]).to be_truthy
+  end
+
   it "should enable batch with sis stickyness" do
     json = api_call(:post,
       "/api/v1/accounts/#{@account.id}/sis_imports.json",

spec/models/sis_batch_spec.rb

@@ -736,6 +736,55 @@ test_1,u1,student,active}
         expect(b1.processing_errors.size).to eq 0
       end
 
+      describe 'multi_term_batch_mode' do
+        before :once do
+          @term2 = @account.enrollment_terms.first
+          @term2.update_attribute(:sis_source_id, 'term2')
+
+          @c2 = factory_with_protected_attributes(@account.courses, name: "delete me", enrollment_term: @term2,
+                                                  sis_source_id: 'test_2', sis_batch_id: @old_batch.id)
+        end
+
+        it 'should use multi_term_batch_mode' do
+          batch = create_csv_data([
+                                    %{term_id,name,status
+                                      term1,term1,active
+                                      term2,term2,active},
+                                    %{course_id,short_name,long_name,account_id,term_id,status},
+                                    %{course_id,user_id,role,status},
+                                  ]) do |batch|
+            batch.options = {}
+            batch.batch_mode = true
+            batch.options[:multi_term_batch_mode] = true
+            batch.save!
+            batch.process_without_send_later
+          end
+          expect(@e1.reload).to be_deleted
+          expect(@e2.reload).to be_deleted
+          expect(@c1.reload).to be_deleted
+          expect(@c2.reload).to be_deleted
+          expect(batch.workflow_state).to eq 'imported'
+        end
+
+        it 'should not use multi_term_batch_mode if no terms are passed' do
+          batch = create_csv_data([
+                                    %{course_id,short_name,long_name,account_id,term_id,status},
+                                    %{course_id,user_id,role,status},
+                                  ]) do |batch|
+            batch.options = {}
+            batch.batch_mode = true
+            batch.options[:multi_term_batch_mode] = true
+            batch.save!
+            batch.process_without_send_later
+          end
+          expect(@e1.reload).to be_active
+          expect(@e2.reload).to be_active
+          expect(@c1.reload.workflow_state).to eq 'created'
+          expect(@c2.reload.workflow_state).to eq 'created'
+          expect(batch.workflow_state).to eq 'aborted'
+        end
+      end
+
     end
   end
 end