Merge pull request #3387 from kristinalim/fix/3384-product_import_timeout

3384 [Product Import] Process only one batch of rows at a time

app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee

@@ -10,6 +10,7 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
   $scope.updated_ids = []
   $scope.update_errors = []
 
+  $scope.batchSize = 50
   $scope.step = 'settings'
   $scope.chunks = 0
   $scope.completed = 0
@@ -51,19 +52,28 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
   $scope.start = () ->
     $scope.started = true
     total = ams_data.item_count
-    size = 50
-    $scope.chunks = Math.ceil(total / size)
+    $scope.chunks = Math.ceil(total / $scope.batchSize)
 
-    i = 0
+    # Process only the first batch.
+    $scope.processBatch($scope.step, 0, $scope.chunks)
 
-    while i < $scope.chunks
-      start = (i*size)+1
-      end = (i+1)*size
-      if $scope.step == 'import'
-        $scope.processImport(start, end)
-      if $scope.step == 'save'
-        $scope.processSave(start, end)
-      i++
+  $scope.processBatch = (step, batchIndex, batchCount) ->
+    start = (batchIndex * $scope.batchSize) + 1
+    end = (batchIndex + 1) * $scope.batchSize
+    isLastBatch = batchCount == batchIndex + 1
+
+    promise = if step == 'import'
+      $scope.processImport(start, end)
+    else if step == 'save'
+      $scope.processSave(start, end)
+
+    return if isLastBatch
+
+    processNextBatch = ->
+      $scope.processBatch(step, batchIndex + 1, batchCount)
+
+    # Process next batch whether or not processing of the current batch succeeds.
+    promise.then(processNextBatch, processNextBatch)
 
   $scope.processImport = (start, end) ->
     $http(

spec/features/admin/product_import_spec.rb

@@ -46,7 +46,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       expect(page).to have_selector '.item-count', text: "2"
       expect(page).to have_no_selector '.invalid-count'
@@ -89,7 +89,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       expect(page).to have_selector '.item-count', text: "2"
       expect(page).to have_selector '.invalid-count', text: "2"
@@ -112,7 +112,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       expect(page).to have_selector '.item-count', text: "1"
       expect(page).to have_selector '.create-count', text: "1"
@@ -142,7 +142,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       save_data
 
@@ -188,7 +188,7 @@ feature "Product Import", js: true do
 
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       save_data
 
@@ -213,7 +213,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       expect(page).to have_selector '.item-count', text: "3"
       expect(page).to_not have_selector '.invalid-count'
@@ -252,7 +252,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       expect(page).to have_selector '.item-count', text: "3"
       expect(page).to have_no_selector '.invalid-count'
@@ -349,7 +349,7 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      import_data
+      proceed_to_validation
 
       expect(page).to have_content I18n.t('admin.product_import.import.validation_overview')
       expect(page).to have_selector '.item-count', text: "2"
@@ -361,9 +361,61 @@ feature "Product Import", js: true do
     end
   end
 
+  describe "handling a large file (120 data rows)" do
+    let!(:producer) { enterprise }
+
+    let(:tmp_csv_path) { "/tmp/test.csv" }
+
+    before do
+      quick_login_as admin
+      visit main_app.admin_product_import_path
+    end
+
+    context "when importing to product list" do
+      def write_tmp_csv_file
+        CSV.open(tmp_csv_path, "w") do |csv|
+          csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type",
+                  "tax_category", "shipping_category"]
+          120.times do |i|
+            csv << ["Imported Product #{i + 1}", producer.name, category.name, 1, "1.00", "500",
+                    "g", tax_category.name, shipping_category.name]
+          end
+        end
+      end
+
+      before { write_tmp_csv_file }
+
+      it "validates and saves all batches" do
+        # Upload and validate file.
+        attach_file "file", tmp_csv_path
+        click_button I18n.t("admin.product_import.index.upload")
+        proceed_to_validation
+
+        # Check that all rows are validated.
+        heading = "120 #{I18n.t("admin.product_import.import.products_to_create")}"
+        find(".panel-header", text: heading).click
+        expect(page).to have_content "Imported Product 10"
+        expect(page).to have_content "Imported Product 60"
+        expect(page).to have_content "Imported Product 110"
+
+        # Save file.
+        proceed_with_save
+
+        # Be extra patient.
+        expect_progress_percentages "33%", "67%", "100%"
+        expect_import_completed
+
+        # Check that all rows are saved.
+        expect(producer.supplied_products.find_by_name("Imported Product 10")).to be_present
+        expect(producer.supplied_products.find_by_name("Imported Product 60")).to be_present
+        expect(producer.supplied_products.find_by_name("Imported Product 110")).to be_present
+      end
+    end
+  end
+
   private
 
-  def import_data
+  def proceed_to_validation
     expect(page).to have_selector 'a.button.proceed', visible: true
     click_link I18n.t('admin.product_import.import.import')
     expect(page).to have_selector 'form.product-import', visible: true
@@ -372,8 +424,22 @@ feature "Product Import", js: true do
 
   def save_data
     expect(page).to have_selector 'a.button.proceed', visible: true
-    click_link I18n.t('admin.product_import.import.save')
+    proceed_with_save
     expect(page).to have_selector 'div.save-results', visible: true
+    expect_import_completed
+  end
+
+  def expect_progress_percentages(*percentages)
+    percentages.each do |percentage|
+      expect(page).to have_selector ".progress-interface", text: percentage
+    end
+  end
+
+  def proceed_with_save
+    click_link I18n.t("admin.product_import.import.save")
+  end
+
+  def expect_import_completed
     expect(page).to have_content I18n.t('admin.product_import.save_results.final_results')
   end
 end