From c8397024e4a7f6e626983dc45d7f336441fd2f66 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 26 Jul 2018 15:55:42 +0100
Subject: [PATCH] Streamline Product Import UX flow

---
 .../import_form_controller.js.coffee          | 54 +++++++++---------
 .../stylesheets/admin/product_import.css.scss | 38 +++++++------
 .../admin/product_import_controller.rb        | 16 +++++-
 app/models/product_import/product_importer.rb |  2 +-
 .../admin/product_import/_ams_data.html.haml  |  1 +
 .../admin/product_import/import.html.haml     | 29 ++++------
 config/locales/en.yml                         |  2 +-
 spec/features/admin/product_import_spec.rb    | 56 ++-----------------
 8 files changed, 79 insertions(+), 119 deletions(-)
 create mode 100644 app/views/admin/product_import/_ams_data.html.haml

diff --git a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee
index 8f031cf4ab..75b506fc66 100644
--- a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee
+++ b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee
@@ -1,20 +1,22 @@
-angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $http, $filter, ProductImportService, $timeout) ->
+angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $http, $filter, ProductImportService, ams_data, $timeout) ->
 
   $scope.entries = {}
   $scope.update_counts = {}
   $scope.reset_counts = {}
-  $scope.importSettings = null
+  $scope.supplier_product_counts = ams_data.supplier_product_counts
 
   $scope.updates = {}
   $scope.updated_total = 0
   $scope.updated_ids = []
   $scope.update_errors = []
 
+  $scope.step = 'settings'
   $scope.chunks = 0
   $scope.completed = 0
-  $scope.percentage = "0%"
-  $scope.started = false
-  $scope.finished = false
+  $scope.percentage = {
+    import: "0%",
+    save: "0%"
+  }
 
   $scope.countResettable = () ->
     angular.forEach $scope.supplier_product_counts, (value, key) ->
@@ -25,7 +27,6 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
   $scope.resetProgress = () ->
     $scope.chunks = 0
     $scope.completed = 0
-    $scope.percentage = "0%"
     $scope.started = false
     $scope.finished = false
 
@@ -33,22 +34,23 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
 
   $scope.confirmSettings = () ->
     $scope.step = 'import'
+    $scope.start()
 
   $scope.viewResults = () ->
     $scope.countResettable()
     $scope.step = 'results'
-    $scope.resetProgress()
 
   $scope.acceptResults = () ->
+    $scope.resetProgress()
     $scope.step = 'save'
+    $scope.start()
 
   $scope.finalResults = () ->
     $scope.step = 'complete'
 
   $scope.start = () ->
     $scope.started = true
-    $scope.percentage = "1%"
-    total = $scope.item_count
+    total = ams_data.item_count
     size = 100
     $scope.chunks = Math.ceil(total / size)
 
@@ -64,15 +66,14 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
       i++
 
   $scope.processImport = (start, end) ->
-    $scope.getSettings() if $scope.importSettings == null
     $http(
-      url: $scope.import_url
+      url: ams_data.import_url
       method: 'POST'
       data:
         'start': start
         'end': end
-        'filepath': $scope.filepath
-        'settings': $scope.importSettings
+        'filepath': ams_data.filepath
+        'settings': ams_data.importSettings
     ).success((data, status) ->
       angular.merge($scope.entries, angular.fromJson(data['entries']))
       $scope.sortUpdates(data['reset_counts'])
@@ -83,12 +84,6 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
       console.error(data)
     )
 
-  $scope.getSettings = () ->
-    $scope.importSettings = {
-      reset_all_absent: document.getElementsByName('settings[reset_all_absent]')[0].value,
-      import_into: document.getElementsByName('settings[import_into]')[0].value
-    }
-
   $scope.sortUpdates = (data) ->
     angular.forEach data, (value, key) ->
       if (key in $scope.update_counts)
@@ -97,15 +92,14 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
         $scope.update_counts[key] = value['updates_count']
 
   $scope.processSave = (start, end) ->
-    $scope.getSettings() if $scope.importSettings == null
     $http(
-      url: $scope.save_url
+      url: ams_data.save_url
       method: 'POST'
       data:
         'start': start
         'end': end
-        'filepath': $scope.filepath
-        'settings': $scope.importSettings
+        'filepath': ams_data.filepath
+        'settings': ams_data.importSettings
     ).success((data, status) ->
       $scope.sortResults(data['results'])
 
@@ -131,7 +125,7 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
       $scope.updated_total += value
 
   $scope.resetAbsent = () ->
-    return unless $scope.importSettings['reset_all_absent']
+    return unless ams_data.importSettings['reset_all_absent']
     enterprises_to_reset = []
 
     angular.forEach $scope.reset_counts, (count, enterprise_id) ->
@@ -139,11 +133,11 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
 
     if enterprises_to_reset.length && $scope.updated_ids.length
       $http(
-        url: $scope.reset_url
+        url: ams_data.reset_url
         method: 'POST'
         data:
-          'filepath': $scope.filepath
-          'settings': $scope.importSettings
+          'filepath': ams_data.filepath
+          'settings': ams_data.importSettings
           'reset_absent': true,
           'updated_ids': $scope.updated_ids,
           'enterprises_to_reset': enterprises_to_reset
@@ -155,8 +149,10 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt
 
   $scope.updateProgress = () ->
     $scope.completed++
-    $scope.percentage = String(Math.round(($scope.completed / $scope.chunks) * 100)) + '%'
+    $scope.percentage[$scope.step] = String(Math.round(($scope.completed / $scope.chunks) * 100)) + '%'
 
     if $scope.completed == $scope.chunks
-      $scope.finished = true
+      $timeout($scope.viewResults, 1000) if $scope.step == 'import'
+      $timeout($scope.finalResults, 1000) if $scope.step == 'save'
+
       $scope.resetAbsent() if $scope.step == 'save'
diff --git a/app/assets/stylesheets/admin/product_import.css.scss b/app/assets/stylesheets/admin/product_import.css.scss
index 91d7e70297..d8ad019af9 100644
--- a/app/assets/stylesheets/admin/product_import.css.scss
+++ b/app/assets/stylesheets/admin/product_import.css.scss
@@ -232,29 +232,35 @@ form.product-import, div.post-save-results, div.import-wrapper {
   }
 }
 
-form.product-import, div.save-results {
-  transition: all linear 0.25s;
-}
-
-form.product-import.ng-hide, div.save-results.ng-hide {
-  opacity: 0;
-}
-
 div.import-wrapper {
+
+  .ng-hide:not(.ng-hide-animate) {
+    // We have to use !important here to override angular's display properties
+    // scss-lint:disable ImportantRule
+    display: block !important;
+    position: absolute;
+    opacity: 0;
+    top: -9999px;
+    left: -9999px;
+  }
+
+  .ng-hide-add, .ng-hide-remove, .ng-hide-animate {
+    transition: all .4s ease-in-out;
+  }
+
+  form.product-import, div.save-results {
+    transition: all .4s ease-in-out;
+  }
+
   div.progress-interface {
     text-align: center;
-    transition: all linear 0.25s;
+    transition: all .4s ease-in-out;
 
     button:disabled {
       background: #ccc !important;
     }
+  }
 
-  }
-  div.progress-interface.ng-hide {
-    position: absolute;
-    width: 100%;
-    opacity: 0;
-  }
   .post-save-results {
     a.button{
       float: left;
@@ -279,7 +285,7 @@ div.progress-bar {
     height: 100%;
     border-radius: 0.3em;
     box-shadow: inset 0 0 3px rgba(0,0,0,0.3);
-    transition: width 0.5s ease-in-out;
+    transition: width .3s ease-in-out;
   }
 }
 
diff --git a/app/controllers/admin/product_import_controller.rb b/app/controllers/admin/product_import_controller.rb
index 88d9623b3a..38ea79a97d 100644
--- a/app/controllers/admin/product_import_controller.rb
+++ b/app/controllers/admin/product_import_controller.rb
@@ -11,7 +11,6 @@ module Admin
     end
 
     def import
-      # Save uploaded file to tmp directory
       @filepath = save_uploaded_file(params[:file])
       @importer = ProductImport::ProductImporter.new(File.new(@filepath), spree_current_user, params[:settings])
       @original_filename = params[:file].try(:original_filename)
@@ -19,8 +18,7 @@ module Admin
       check_file_errors @importer
       check_spreadsheet_has_data @importer
 
-      @tax_categories = Spree::TaxCategory.order('is_default DESC, name ASC')
-      @shipping_categories = Spree::ShippingCategory.order('name ASC')
+      @ams_data = ams_data
     end
 
     def validate_data
@@ -87,6 +85,18 @@ module Admin
       end
     end
 
+    def ams_data
+      {
+        filepath: @filepath,
+        item_count: @importer.item_count,
+        supplier_product_counts: @importer.supplier_products,
+        import_url: main_app.admin_product_import_process_async_path,
+        save_url: main_app.admin_product_import_save_async_path,
+        reset_url: main_app.admin_product_import_reset_async_path,
+        importSettings: @importer.import_settings,
+      }
+    end
+
     # Define custom model class for Cancan permissions
     def model_class
       ProductImport::ProductImporter
diff --git a/app/models/product_import/product_importer.rb b/app/models/product_import/product_importer.rb
index 2dc12a9e74..6ec0695768 100644
--- a/app/models/product_import/product_importer.rb
+++ b/app/models/product_import/product_importer.rb
@@ -68,7 +68,7 @@ module ProductImport
     end
 
     def supplier_products
-      @processor.supplier_products
+      @processor.andand.supplier_products
     end
 
     def total_supplier_products
diff --git a/app/views/admin/product_import/_ams_data.html.haml b/app/views/admin/product_import/_ams_data.html.haml
new file mode 100644
index 0000000000..868538dd9a
--- /dev/null
+++ b/app/views/admin/product_import/_ams_data.html.haml
@@ -0,0 +1 @@
+= admin_inject_json "admin.productImport", "ams_data", @ams_data
\ No newline at end of file
diff --git a/app/views/admin/product_import/import.html.haml b/app/views/admin/product_import/import.html.haml
index 7a4326e3d8..a760635f6c 100644
--- a/app/views/admin/product_import/import.html.haml
+++ b/app/views/admin/product_import/import.html.haml
@@ -1,14 +1,12 @@
 - content_for :page_title do
   #{t('admin.product_import.title')}
 
+= render partial: 'ams_data'
 = render partial: 'spree/admin/shared/product_sub_menu'
 
-.import-wrapper{ng: {app: 'admin.productImport', controller: 'ImportFormCtrl', init: "supplier_product_counts = #{@importer.supplier_products.to_json}"}}
+.import-wrapper{ng: {app: 'admin.productImport', controller: 'ImportFormCtrl'}}
 
-  = hidden_field_tag "settings[reset_all_absent]", @importer.import_settings['reset_all_absent'], 'ng-model' => "importSettings[reset_all_absent]"
-  = hidden_field_tag "settings[import_into]", @importer.import_settings['import_into'], 'ng-model' => "importSettings[import_into]"
-
-  - if @importer.item_count == 0 #and @importer.invalid_count
+  - if @importer.item_count == 0
     %h5
       = t('.no_valid_entries')
     %p
@@ -19,20 +17,16 @@
       = render 'import_options' if @importer.table_headings
       %br
       %a.button.proceed{href: '', ng: {click: 'confirmSettings()'}}
-        = t('.proceed')
+        = t('.import')
       %a.button{href: main_app.admin_product_import_path} #{t('admin.cancel')}
 
   .progress-interface{ng: {show: 'step == "import"'}}
     %span.filename
       = @original_filename
       %span.percentage
-        ({{ percentage }})
+        ({{ percentage.import }})
     .progress-bar
-      %span.progress-track{class: 'ng-binding', style: "width:{{percentage}}"}
-    %button.start_import{ng: {click: 'start()', disabled: 'started', init: "item_count = #{@importer.item_count}; import_url = '#{main_app.admin_product_import_process_async_path}'; filepath = '#{@filepath}'; import_into = '#{@import_into}'"}}
-      = t('.import')
-    %button.review{ng: {click: 'viewResults()', disabled: '!finished'}}
-      = t('.review')
+      %span.progress-track{class: 'ng-binding', style: "width:{{ percentage.import }}"}
     %p.red
       {{ exception }}
 
@@ -53,7 +47,8 @@
       = hidden_field_tag :filepath, @filepath
       = hidden_field_tag "settings[import_into]", @import_into
 
-      %a.button.proceed{href: '', ng: {show: 'count((entries | entriesFilterValid:"invalid")) == 0', click: 'acceptResults()'}}= t('.proceed')
+      %a.button.proceed{href: '', ng: {show: 'count((entries | entriesFilterValid:"invalid")) == 0', click: 'acceptResults()'}}
+        = t('.save')
 
       %a.button{href: main_app.admin_product_import_path}= t('admin.cancel')
 
@@ -63,13 +58,9 @@
 
   .progress-interface{ng: {show: 'step == "save"'}}
     %span.filename
-      #{t('.save_imported')} ({{ percentage }})
+      #{t('.save_imported')} ({{ percentage.save }})
     .progress-bar{}
-      %span.progress-track{ng: {style: "{'width':percentage}"}}
-    %button.start_save{ng: {click: 'start()', disabled: 'started', init: "item_count = #{@importer.item_count}; save_url = '#{main_app.admin_product_import_save_async_path}'; reset_url = '#{main_app.admin_product_import_reset_async_path}'; filepath = '#{@filepath}'; import_into = '#{@import_into}'"}}
-      = t('.save')
-    %button.view_results{ng: {click: 'finalResults()', disabled: '!finished'}}
-      = t('.results')
+      %span.progress-track{ng: {style: "{'width': percentage.save }"}}
     %p.red
       {{ exception }}
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 9ffd7894f1..7f85aff207 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -481,7 +481,7 @@ en:
         shipping_categories: Shipping Categories
       import:
         review: Review
-        proceed: Proceed
+        import: Import
         save: Save
         results: Results
         save_imported: Save imported products
diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb
index 25a4431187..af23ddf9bf 100644
--- a/spec/features/admin/product_import_spec.rb
+++ b/spec/features/admin/product_import_spec.rb
@@ -45,9 +45,6 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
       expect(page).to have_selector '.item-count', text: "2"
@@ -55,9 +52,6 @@ feature "Product Import", js: true do
       expect(page).to have_selector '.create-count', text: "2"
       expect(page).to_not have_selector '.update-count'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       save_data
 
       expect(page).to have_selector '.created-count', text: '2'
@@ -94,9 +88,6 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
       expect(page).to have_selector '.item-count', text: "2"
@@ -120,18 +111,12 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
       expect(page).to have_selector '.item-count', text: "1"
       expect(page).to have_selector '.create-count', text: "1"
       expect(page).to_not have_selector '.update-count'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       save_data
 
       expect(page).to have_selector '.created-count', text: '1'
@@ -156,14 +141,8 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       save_data
 
       carrots = Spree::Product.find_by_name('Carrots')
@@ -210,14 +189,8 @@ feature "Product Import", js: true do
 
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       save_data
 
       expect(page).to have_selector '.created-count', text: '1'
@@ -242,9 +215,6 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
       expect(page).to have_selector '.item-count', text: "3"
@@ -254,9 +224,6 @@ feature "Product Import", js: true do
       expect(page).to have_selector '.inv-create-count', text: "2"
       expect(page).to have_selector '.inv-update-count', text: "1"
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       save_data
 
       expect(page).to_not have_selector '.created-count'
@@ -345,9 +312,6 @@ feature "Product Import", js: true do
       attach_file 'file', '/tmp/test.csv'
       click_button 'Upload'
 
-      expect(page).to have_selector 'a.button.proceed', visible: true
-      click_link 'Proceed'
-
       import_data
 
       expect(page).to have_content I18n.t('admin.product_import.import.validation_overview')
@@ -363,24 +327,16 @@ feature "Product Import", js: true do
   private
 
   def import_data
-    expect(page).to have_selector 'button.start_import', visible: true
-    expect(page).to have_selector "button.review[disabled='disabled']"
-
-    find('button.start_import').trigger 'click'
-    wait_until { page.find("button.review:not([disabled='disabled'])").present? }
-
-    find('button.review').trigger 'click'
+    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
     expect(page).to have_content I18n.t('admin.product_import.import.validation_overview')
   end
 
   def save_data
-    expect(page).to have_selector 'button.start_save', visible: true
-    expect(page).to have_selector "button.view_results[disabled='disabled']"
-
-    find('button.start_save').trigger 'click'
-    wait_until { page.find("button.view_results:not([disabled='disabled'])").present? }
-
-    find('button.view_results').trigger 'click'
+    expect(page).to have_selector 'a.button.proceed', visible: true
+    click_link I18n.t('admin.product_import.import.save')
+    expect(page).to have_selector 'div.save-results', visible: true
     expect(page).to have_content I18n.t('admin.product_import.save_results.final_results')
   end
 end