Product Import update

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

@@ -0,0 +1,5 @@
+angular.module("ofn.admin").controller "FeedbackPanelsCtrl", ($scope) ->
+  $scope.active = false
+
+  $scope.togglePanel = ->
+    $scope.active = !$scope.active

app/assets/stylesheets/admin/product_import.css.scss

@@ -0,0 +1,104 @@
+div.feedback-section {
+
+  div.feedback-header {
+    width: 100%;
+    //font-size: 1.5em;
+    clear: both;
+    //border: 1px solid #ccc;
+    float: left;
+    padding: 0.5em;
+
+    div {
+      font-size: 1.25em;
+      float: left;
+    }
+
+    div.header-caret {
+      width: 2em;
+      text-align: center;
+      min-height: 0.1em; //Empty div fix
+    }
+
+    div.header-icon {
+      width: 2.5em;
+      text-align: center;
+      padding-top: 0.1em;
+
+      .fa {
+        font-size: 1.5em;
+        line-height: 0.9em;
+      }
+      .fa-warning {
+        color: #ee4728;
+      }
+      .fa-check-circle {
+        color: #86d83a;
+      }
+    }
+
+    div.header-count {
+      min-width: 2em;
+      text-align: right;
+      padding-right: 0.5em;
+    }
+
+    div.header-description {
+      width: auto;
+    }
+
+  }
+
+  div.feedback-header:hover {
+    cursor: pointer;
+    background-color: #f7f7f7;
+  }
+
+  div.feedback-header.active {
+    background-color: #efefef;
+    text-shadow: 1px 1px 0px rgba(255,255,255,0.75);
+  }
+
+  div.feedback-panel {
+    width: 100%;
+    clear: both;
+    //border: 1px solid #ccc;
+    margin-bottom: 0.5em;
+    background-color: #f9f9f9;
+    padding: 1.5em;
+
+    div.table-wrap {
+      width: 100%;
+      overflow: auto;
+      border-right: 1px solid #ceede3;
+      max-height: 23em;
+    }
+
+    table {
+      background-color: white;
+      margin-bottom: 0;
+      td, th {
+        white-space: nowrap;
+      }
+    }
+
+    div.import-errors {
+      margin-bottom: 0.85em;
+
+      p.line {
+        font-size: 1.15em;
+        margin-bottom: 0.2em;
+        color: #577084;
+      }
+      p.error {
+        color: #cb1b1b;
+        margin-bottom: 0.2em;
+      }
+    }
+
+  }
+
+}
+
+br.panels.clearfix {
+  clear: both;
+}

app/controllers/admin/product_import_controller.rb

@@ -2,44 +2,54 @@ require 'roo'
 
 class Admin::ProductImportController < Spree::Admin::BaseController
 
-  before_filter :check_upload, except: :index
+  before_filter :validate_upload_presence, except: :index
 
   def import
     # Save uploaded file to tmp directory
-    @filepath = save_upload(params[:file])
+    @filepath = save_uploaded_file(params[:file])
     @importer = ProductImporter.new(File.new(@filepath), editable_enterprises)
 
-    if @importer.errors.present?
-      flash[:notice] = @importer.errors.full_messages.to_sentence
-    end
+    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')
   end
 
   def save
-    file = File.new(params[:filepath])
-    @importer = ProductImporter.new(file, editable_enterprises)
-    @importer.save_all_valid
-
-    if @importer.updated_count && @importer.updated_count > 0
-      File.delete(file)
-      flash[:success] = "#{@importer.updated_count} records updated successfully"
-    else
-      flash[:notice] = @importer.errors.full_messages.to_sentence
-    end
+    @importer = ProductImporter.new(File.new(params[:filepath]), editable_enterprises)
+    @importer.save_all if @importer.has_valid_entries?
   end
 
   private
 
-  def check_upload
+  def validate_upload_presence
     unless params[:file] || (params[:filepath] && File.exist?(params[:filepath]))
-      redirect_to '/admin/product_import', :notice => 'File not found or could not be opened'
+      redirect_to '/admin/product_import', notice: 'File not found or could not be opened'
       return
     end
   end
 
-  def save_upload(upload)
-    filename = Time.now.strftime('%d-%m-%Y-%H-%M-%S')
+  def check_file_errors(importer)
+    if importer.errors.present?
+      redirect_to '/admin/product_import', notice: @importer.errors.full_messages.to_sentence
+      return
+    end
+  end
+
+  def check_spreadsheet_has_data(importer)
+    unless importer.item_count
+      redirect_to '/admin/product_import', notice: 'No data found in spreadsheet'
+      return
+    end
+  end
+
+  def save_uploaded_file(upload)
+    filename = 'import' + Time.now.strftime('%d-%m-%Y-%H-%M-%S')
     extension = '.' + upload.original_filename.split('.').last
-    File.open(Rails.root.join('tmp', filename+extension), 'wb') do |f|
+    directory = 'tmp/product_import'
+    Dir.mkdir(directory) unless File.exists?(directory)
+    File.open(Rails.root.join(directory, filename+extension), 'wb') do |f|
       f.write(upload.read)
       f.path
     end

app/models/product_importer.rb

@@ -5,8 +5,6 @@ class ProductImporter
   include ActiveModel::Conversion
   include ActiveModel::Validations
 
-  attr_reader :valid_entries, :invalid_entries
-
   def initialize(file, editable_enterprises, options={})
     if file.is_a?(File)
       @file = file
@@ -15,9 +13,9 @@ class ProductImporter
       @valid_entries = {}
       @invalid_entries = {}
 
-      @products_to_create = []
-      @variants_to_create = []
-      @variants_to_update = []
+      @products_to_create = {}
+      @variants_to_create = {}
+      @variants_to_update = {}
       
       @products_created = 0
       @variants_created = 0
@@ -26,9 +24,11 @@ class ProductImporter
       @editable_enterprises = {}
       editable_enterprises.map { |e| @editable_enterprises[e.name] = e.id }
 
-      validate_all
+      @non_display_attributes = 'id', 'product_id', 'variant_id', 'supplier_id', 'primary_taxon_id', 'category_id', 'shipping_category_id', 'tax_category_id'
+
+      validate_all if @sheet
     else
-      self.errors.add(:importer, "error: no file uploaded")
+      self.errors.add(:importer, 'error: no file uploaded')
     end
   end
 
@@ -36,30 +36,89 @@ class ProductImporter
     false #ActiveModel, not ActiveRecord
   end
 
-  # Private methods below which only work with a valid spreadsheet object can
-  # be called publicly via here if the spreadsheet was successfully loaded,
-  # otherwise they return nil (without error).
-  def method_missing(method, *args, &block)
-    if self.respond_to?(method, include_private=true)
-      @sheet ? self.send(method, *args, &block) : nil
-    else
-      super
+  def has_valid_entries?
+    valid_count and valid_count > 0
+  end
+
+  def item_count
+    @sheet ? @sheet.last_row - 1 : 0
+  end
+
+  def valid_count
+    @valid_entries.count
+  end
+
+  def invalid_count
+    @invalid_entries.count
+  end
+
+  def products_create_count
+    @products_to_create.count + @variants_to_create.count
+  end
+
+  def products_update_count
+    @variants_to_update.count
+  end
+
+  def suppliers_index
+    @suppliers_index || get_suppliers_index
+  end
+
+  def invalid_entries
+    entries = {}
+    @invalid_entries.each do |line_number, data|
+      entries[line_number] = {entry: data[:entry].except(*@non_display_attributes), errors: data[:errors]}
     end
+    entries
+  end
+
+  def products_to_create
+    entries = {}
+    @products_to_create.merge(@variants_to_create).each do |line_number, data|
+      entries[line_number] = {entry: data[:entry].except(*@non_display_attributes)}
+    end
+    entries
+  end
+
+  def products_to_update
+    entries = {}
+    @variants_to_update.each do |line_number, data|
+      entries[line_number] = {entry: data[:entry].except(*@non_display_attributes)}
+    end
+    entries
+  end
+
+  def products_created_count
+    @products_created + @variants_created
+  end
+
+  def products_updated_count
+    @variants_updated
+  end
+
+  def total_saved_count
+    @products_created + @variants_created + @variants_updated
+  end
+
+  def save_all
+    save_all_valid
+    delete_uploaded_file
   end
 
   private
 
   def open_spreadsheet
     if accepted_mimetype
-      @sheet = Roo::Spreadsheet.open(@file, extension: accepted_mimetype)
+      Roo::Spreadsheet.open(@file, extension: accepted_mimetype)
     else
-      self.errors.add(:importer, "could not proccess file: invalid filetype")
+      self.errors.add(:importer, 'could not process file: invalid filetype')
+      delete_uploaded_file
       nil
     end
   end
 
   def accepted_mimetype
-    File.extname(@file.path).in?(['.csv', '.xls', '.xlsx', '.ods']) ? @file.path.split('.').last.to_sym : false
+    File.extname(@file.path).in?('.csv', '.xls', '.xlsx', '.ods') ? @file.path.split('.').last.to_sym : false
   end
 
   def headers
@@ -67,6 +126,7 @@ class ProductImporter
   end
 
   def rows
+    return [] unless @sheet and @sheet.last_row
     (2..@sheet.last_row).map do |i|
       @sheet.row(i)
     end
@@ -82,62 +142,80 @@ class ProductImporter
     entries.each_with_index do |entry, i|
       line_number = i+2
 
-      supplier_validation(entry, line_number)
-      category_validation(entry, line_number)
+      supplier_validation(line_number, entry)
+      category_validation(line_number, entry)
 
       # Ensure on_hand isn't nil because Spree::Product and
-      # Spree::Variant each validate this differently
+      # Spree::Variant each validate it differently
       entry['on_hand'] = 0 if entry['on_hand'].nil?
 
-      set_update_status(entry, line_number)
-      mark_as_valid(entry, line_number) unless entry_invalid?(line_number)
+      set_update_status(line_number, entry)
+      mark_as_valid(line_number, entry) unless entry_invalid?(line_number)
     end
+
+    delete_uploaded_file if item_count.zero? or valid_count.zero?
   end
 
   def entry_invalid?(line_number)
     !!@invalid_entries[line_number]
   end
 
-  def supplier_validation(entry, line_number)
-    # Fetch/assign and validate supplier id
+  def supplier_validation(line_number, entry)
     suppliers_index = @suppliers_index || get_suppliers_index
     supplier_name = entry['supplier']
-    if supplier_name.blank?
-      mark_as_invalid(entry, line_number, "Supplier name field is empty")
-    else
-      if suppliers_index[supplier_name]
-        entry['supplier_id'] = suppliers_index[supplier_name]
-      else
-        mark_as_invalid(entry, line_number, "Supplier \"#{supplier_name}\" not found in database")
-      end
 
-      # Check enterprise permissions
-      unless @editable_enterprises[supplier_name]
-        mark_as_invalid(entry, line_number, "You do not have permission to manage products for \"#{supplier_name}\"")
-      end
+    if supplier_name.blank?
+      mark_as_invalid(line_number, entry, "Supplier name field is empty")
+      return
     end
+
+    unless supplier_exists?(supplier_name)
+      mark_as_invalid(line_number, entry, "Supplier \"#{supplier_name}\" not found in database")
+      return
+    end
+
+    unless permission_to_manage?(supplier_name)
+      mark_as_invalid(line_number, entry, "You do not have permission to manage products for \"#{supplier_name}\"")
+      return
+    end
+
+    entry['supplier_id'] = suppliers_index[supplier_name]
   end
 
-  def category_validation(entry, line_number)
-    # Fetch/assign and validate category id
+  def supplier_exists?(supplier_name)
+    @suppliers_index[supplier_name]
+  end
+
+  def permission_to_manage?(supplier_name)
+    @editable_enterprises.has_key?(supplier_name)
+  end
+
+  def category_validation(line_number, entry)
     categories_index = @categories_index || get_categories_index
     category_name = entry['category']
+
     if category_name.blank?
-      mark_as_invalid(entry, line_number, "Category field is empty")
+      mark_as_invalid(line_number, entry, "Category field is empty")
+      entry['primary_taxon_id'] = Spree::Taxon.first.id # Removes a duplicate validation message
+      return
+    end
+
+    if category_exists?(category_name)
+      entry['primary_taxon_id'] = categories_index[category_name]
     else
-      if categories_index[category_name]
-        entry['primary_taxon_id'] = categories_index[category_name]
-      else
-        mark_as_invalid(entry, line_number, "Category \"#{category_name}\" not found in database")
-      end
+      mark_as_invalid(line_number, entry, "Category \"#{category_name}\" not found in database")
     end
   end
 
-  def mark_as_valid(entry, line_number)
+  def category_exists?(category_name)
+    @categories_index[category_name]
+  end
+
+  def mark_as_valid(line_number, entry)
     @valid_entries[line_number] = {entry: entry}
   end
 
-  def mark_as_invalid(entry, line_number, errors)
+  def mark_as_invalid(line_number, entry, errors)
     errors = [errors] if errors.is_a? String
 
     if entry_invalid?(line_number)
@@ -173,15 +251,14 @@ class ProductImporter
 
   def save_all_valid
     updated = {}
-    @products_to_create.each do |entry|
-      # If we've already added a new product with these attributes from
-      # this spreadsheet, pass this entry to @variants_to_create with
+    @products_to_create.each do |line_number, data|
+      entry = data[:entry]
+      # If we've already added a new product with these attributes
+      # from this spreadsheet, mark this entry as a new variant with
       # the new product id, as this is a now variant of that product...
       if updated[entry['supplier_id']] && updated[entry['supplier_id']][entry['name']]
         product_id = updated[entry['supplier_id']][entry['name']]
-        new_variant = Spree::Variant.new(entry.except('id', 'product_id'))
-        new_variant.product_id = product_id
-        @variants_to_create.push(new_variant)
+        mark_as_new_variant(line_number, entry, product_id)
         next
       end
 
@@ -197,117 +274,90 @@ class ProductImporter
         end
         @products_created += 1
       else
-        self.errors.add(:importer, product.errors.full_messages)
+        self.errors.add("Line #{line_number}:", product.errors.full_messages)
       end
 
       updated[entry['supplier_id']] = {entry['name'] => product.id}
     end
 
-    @variants_to_update.each do |variant|
-      if variant.save
+    @variants_to_update.each do |line_number, data|
+      variant = data[:variant]
+      if variant.valid? and variant.save
         @variants_updated += 1
       else
-        self.errors.add(:importer, variant.errors.full_messages)
+        self.errors.add("Line #{line_number}:", variant.errors.full_messages)
       end
     end
 
-    @variants_to_create.each do |new_variant|
-      if new_variant.save
+    @variants_to_create.each do |line_number, data|
+      new_variant = data[:variant]
+      if new_variant.valid? and new_variant.save
         @variants_created += 1
       else
-        self.errors.add(:importer, new_variant.errors.full_messages)
+        self.errors.add("Line #{line_number}:", new_variant.errors.full_messages)
       end
     end
 
-    self.errors.add(:importer, "did not save any products successfully") if updated_count == 0
+    self.errors.add(:importer, "did not save any products successfully") if total_saved_count == 0
 
-    updated_count
+    total_saved_count
   end
 
-  def set_update_status(entry, line_number)
+  def set_update_status(line_number, entry)
     # Find product with matching supplier and name
     match = Spree::Product.where(supplier_id: entry['supplier_id'], name: entry['name'], deleted_at: nil).first
 
     # If no matching product was found, create a new product
     if match.nil?
-      mark_as_new_product(entry, line_number)
+      mark_as_new_product(line_number, entry)
       return
     end
 
     # Otherwise, if a variant exists with matching display_name and unit_value, update it
     match.variants.each do |existing_variant|
       if existing_variant.display_name == entry['display_name'] && existing_variant.unit_value == Float(entry['unit_value'])
-        mark_as_existing_variant(entry, line_number, existing_variant)
+        mark_as_existing_variant(line_number, entry, existing_variant)
         return
       end
     end
 
     # Otherwise, a variant with sufficiently matching attributes doesn't exist; create a new one
-    mark_as_new_variant(entry, line_number)
+    mark_as_new_variant(line_number, entry, match.id)
   end
 
-  def mark_as_new_product(entry, line_number)
+  def mark_as_new_product(line_number, entry)
     new_product = Spree::Product.new()
     new_product.assign_attributes(entry.except('id'))
     if new_product.valid?
-      @products_to_create.push(entry) unless entry_invalid?(line_number)
+      @products_to_create[line_number] = {entry: entry} unless entry_invalid?(line_number)
     else
-      mark_as_invalid(entry, line_number, new_product.errors.full_messages)
+      mark_as_invalid(line_number, entry, new_product.errors.full_messages)
     end
   end
 
-  def mark_as_existing_variant(entry, line_number, existing_variant)
+  def mark_as_existing_variant(line_number, entry, existing_variant)
     existing_variant.assign_attributes(entry.except('id', 'product_id'))
     if existing_variant.valid?
-      @variants_to_update.push(existing_variant) unless entry_invalid?(line_number)
+      @variants_to_update[line_number] = {entry: entry, variant: existing_variant} unless entry_invalid?(line_number)
     else
-      mark_as_invalid(entry, line_number, existing_variant.errors.full_messages)
+      mark_as_invalid(line_number, entry, existing_variant.errors.full_messages)
     end
   end
 
-  def mark_as_new_variant(entry, line_number)
+  def mark_as_new_variant(line_number, entry, product_id)
     new_variant = Spree::Variant.new(entry.except('id', 'product_id'))
-    new_variant.product_id = match.id
+    new_variant.product_id = product_id
     if new_variant.valid?
-      @variants_to_create.push(new_variant) unless entry_invalid?(line_number)
+      @variants_to_create[line_number] = {entry: entry, variant: new_variant} unless entry_invalid?(line_number)
     else
-      mark_as_invalid(entry, line_number, new_variant.errors.full_messages)
+      mark_as_invalid(line_number, entry, new_variant.errors.full_messages)
     end
   end
 
-  def has_valid_entries?
-    valid_count > 0
-  end
-
-  def item_count
-    @sheet.last_row - 1
-  end
-
-  def valid_count
-    @valid_entries.count
-  end
-
-  def invalid_count
-    @invalid_entries.count
-  end
-
-  def products_create_count
-    @products_to_create.count + @variants_to_create.count
-  end
-
-  def products_update_count
-    @variants_to_update.count
-  end
-
-  def products_created
-    @products_created + @variants_created
-  end
-
-  def products_updated
-    @variants_updated
-  end
-
-  def updated_count
-    @products_created + @variants_created + @variants_updated
+  def delete_uploaded_file
+    # Only delete if file is in '/tmp/product_import' directory
+    if @file.path == Rails.root.join('tmp', 'product_import').to_s
+      File.delete(@file)
+    end
   end
 end

app/views/admin/product_import/_entries_table.html.haml

@@ -0,0 +1,12 @@
+- if entries && entries.count > 0 #&& entries.values.first && entries.values.first[:entry]
+  %div.table-wrap
+    %table
+      %thead
+        %th Line
+        - entries.values.first[:entry].each do |key, value|
+          %th= key
+      - entries.each do |line, item|
+        %tr
+          %td= line
+          - item[:entry].each do |key, value|
+            %td= value

app/views/admin/product_import/_errors_list.html.haml

@@ -0,0 +1,11 @@
+- @importer.invalid_entries.each do |line, item|
+  %div.import-errors
+    %p.line
+      %strong
+        Item line #{line}:
+      %span= item[:entry]['name']
+      - if item[:entry]['display_name']
+        ( #{item[:entry]['display_name']} )
+    - item[:errors].each do |error|
+      %p.error
+         -  #{error}

app/views/admin/product_import/_import_review.html.haml

@@ -0,0 +1,62 @@
+%h5 Import validation overview
+%br
+
+%div.feedback-section
+  %div.feedback-header
+    %div.header-caret
+      -#%i.icon-chevron-right{ng: {hide: 'active'}}
+      -#%i.icon-chevron-down{ng: {hide: '!active'}}
+    %div.header-icon
+      %i.fa.fa-check-circle
+    %div.header-count
+      %strong.item-count= @importer.item_count
+    %div.header-description
+      Entries found in imported file
+  -#%div.feedback-panel{ng: {hide: '!active'}}
+  -#  Content goes here
+
+%div.feedback-section{ng: {controller: 'FeedbackPanelsCtrl', init: "count = #{@importer.invalid_count}"}}
+  %div.feedback-header{ng: {click: 'togglePanel()', class: '{active: active && count}'}}
+    %div.header-caret
+      %i.icon-chevron-right{ng: {hide: 'active || count == 0'}}
+      %i.icon-chevron-down{ng: {hide: '!active || count == 0'}}
+    %div.header-icon
+      %i.fa.fa-warning
+    %div.header-count
+      %strong.invalid-count= @importer.invalid_count
+    %div.header-description
+      Items contain errors and will not be imported
+  %div.feedback-panel{ng: {hide: '!active || count == 0'}}
+    = render 'errors_list'
+    %br
+    = render 'entries_table', entries: @importer.invalid_entries
+
+%div.feedback-section{ng: {controller: 'FeedbackPanelsCtrl', init: "count = #{@importer.products_create_count}"}}
+  %div.feedback-header{ng: {click: 'togglePanel()', class: '{active: active && count}'}}
+    %div.header-caret
+      %i.icon-chevron-right{ng: {hide: 'active || count == 0'}}
+      %i.icon-chevron-down{ng: {hide: '!active || count == 0'}}
+    %div.header-icon
+      %i.fa.fa-check-circle
+    %div.header-count
+      %strong.create-count= @importer.products_create_count
+    %div.header-description
+      Products will be created
+  %div.feedback-panel{ng: {hide: '!active || count == 0'}}
+    = render 'entries_table', entries: @importer.products_to_create
+
+%div.feedback-section{ng: {controller: 'FeedbackPanelsCtrl', init: "count = #{@importer.products_update_count}"}}
+  %div.feedback-header{ng: {click: 'togglePanel()', class: '{active: active && count}'}}
+    %div.header-caret
+      %i.icon-chevron-right{ng: {hide: 'active || count == 0'}}
+      %i.icon-chevron-down{ng: {hide: '!active || count == 0'}}
+    %div.header-icon
+      %i.fa.fa-check-circle
+    %div.header-count
+      %strong.update-count= @importer.products_update_count
+    %div.header-description
+      Products will be updated
+  %div.feedback-panel{ng: {hide: '!active || count == 0'}}
+    = render 'entries_table', entries: @importer.products_to_update
+
+%br.panels.clearfix

app/views/admin/product_import/_upload_form.html.haml

@@ -0,0 +1,9 @@
+%h5 Select a spreadsheet to upload
+%br
+= form_tag main_app.admin_product_import_path, multipart: true do
+  = file_field_tag :file
+  %br
+  %br
+  = submit_tag "Import"
+  %br
+  %br

app/views/admin/product_import/import.html.haml

@@ -1,67 +1,32 @@
 - content_for :page_title do
   Product Import
 
-= render :partial => 'spree/admin/shared/product_sub_menu'
+= render partial: 'spree/admin/shared/product_sub_menu'
 
-- if @importer.valid_count && @importer.invalid_count
-  %h5 Products imported from spreadsheet:
-  %br
-  %p{style: "font-size: 1.15em"}
-    Valid entries found:
-    = @importer.valid_count
-  %p{style: "font-size: 1.15em"}
-    Invalid entries found:
-    = @importer.invalid_count
-  %br
-  %p{style: "font-size: 1.15em"}
-    Products to be created:
-    = @importer.products_create_count
-  %p{style: "font-size: 1.15em"}
-    Products to be updated:
-    = @importer.products_update_count
+= form_tag main_app.admin_product_import_save_path, {'ng-app' => 'ofn.admin'} do
 
-- if @importer.invalid_entries && @importer.invalid_entries.count > 0
-  %br
-  %h5 Import errors:
-  %br
-  - @importer.invalid_entries.each do |line, item|
-    %p{style: "font-size: 1.15em"}
-      %strong
-        Item line #{line}:
-      %span= item[:entry]['name']
-      - if item[:entry]['display_name']
-        ( #{item[:entry]['display_name']} )
-    - item[:errors].each do |error|
-      %p{class: "red"}
-         -  #{error}
-  %br
-  %h5 Review invalid entries:
-  %div{style: 'width: 100%; overflow: auto;'}
-    %table
-      %thead
-        %th Line
-        - @importer.invalid_entries.values.first[:entry].each do |key, value|
-          %th{style: 'white-space: nowrap;'}= key
-      - @importer.invalid_entries.each do |line, item|
-        %tr
-          %td= line
-          - item[:entry].each do |key, value|
-            %td{style: 'white-space: nowrap;'}= value
+  - if @importer.invalid_count && !@importer.has_valid_entries?
+    %h5 No valid entries found
+    %p There are no entries that can be saved
+    %br
 
-- if @importer.has_valid_entries?
-  %br
-  - if @importer.invalid_count > 0
-    %h5 Invalid entries detected.
-    %p Save valid products for now and discard the others?
-  - else
-    %h5 No errors detected!
-    %p Save all imported products?
-  %br
-  = form_tag main_app.admin_product_import_save_path, multipart: true do
+  = render 'import_review'
+
+  - if @importer.has_valid_entries?
+    - if @importer.invalid_count > 0
+      %br
+      %h5 Imported file contains some invalid entries
+      %p Save valid entries for now and discard the others?
+    - else
+      %h5 No errors detected!
+      %p Save all imported products?
+    %br
     = hidden_field_tag :filepath, @filepath
     = submit_tag "Save"
     %a.button{href: main_app.admin_product_import_path} Cancel
 
-- if @importer.invalid_count && !@importer.has_valid_entries?
-  %br
-  %a.button{href: main_app.admin_product_import_path} Cancel
+  - else
+    %br
+    %a.button{href: main_app.admin_product_import_path} Cancel
+
+

app/views/admin/product_import/index.html.haml

@@ -3,12 +3,4 @@
 
 = render :partial => 'spree/admin/shared/product_sub_menu'
 
-%h5 Select a spreadsheet to upload
-%br
-= form_tag main_app.admin_product_import_path, multipart: true do
-  = file_field_tag :file
-  %br
-  %br
-  = submit_tag "Import"
-  %br
-  %br
+= render 'upload_form'

app/views/admin/product_import/save.html.haml

@@ -8,17 +8,20 @@
 
 %p{style: 'font-size: 1.15em'}
   Products created:
-  = @importer.products_created
+  = @importer.products_created_count
 %p{style: 'font-size: 1.15em'}
   Products updated:
-  = @importer.products_updated
+  = @importer.products_updated_count
 
-- if @importer.errors.full_messages && !@importer.errors.full_messages.blank?
-  %br
-  %h6 Errors
+%br
+
+- if !@importer.errors.full_messages
+  %h5 All #{importer.total_saved_count} items saved successfully
+- else
+  %h5 Errors
   - @importer.errors.full_messages.each do |error|
     %p{class: "red"}
        -  #{error}
 
 %br
-  %a.button{href: main_app.admin_product_import_path} Back
+%a.button{href: main_app.admin_product_import_path} Back

spec/features/admin/product_import_spec.rb

@@ -7,11 +7,14 @@ feature "Product Import", js: true do
 
   let!(:admin) { create(:admin_user) }
   let!(:user) { create_enterprise_user }
-  let!(:enterprise) { create(:enterprise, owner: user, name: "Test Enterprise") }
+  let!(:enterprise) { create(:supplier_enterprise, owner: user, name: "Test Enterprise") }
   let!(:category) { create(:taxon, name: 'Vegetables') }
+  let!(:category2) { create(:taxon, name: 'Cake') }
+  let!(:product) { create(:simple_product, supplier: enterprise, name: 'Hypothetical Cake') }
+  let!(:variant) { create(:variant, product_id: product.id, price: '8.50', count_on_hand: '100', unit_value: '500', display_name: 'Preexisting Banana') }
   let(:permissions) { OpenFoodNetwork::Permissions.new(user) }
 
-  describe "product import" do
+  describe "when importing products from uploaded file" do
     before { quick_login_as_admin }
     after { File.delete('/tmp/test.csv') }
 
@@ -26,14 +29,16 @@ feature "Product Import", js: true do
       visit main_app.admin_product_import_path
 
       expect(page).to have_content "Select a spreadsheet to upload"
-      attach_file('file', '/tmp/test.csv')
-      click_button('Import')
+      attach_file 'file', '/tmp/test.csv'
+      click_button 'Import'
 
-      expect(page).to have_content("Valid entries found: 2")
-      expect(page).to have_content("Invalid entries found: 0")
-      click_button('Save')
+      expect(page).to have_selector '.item-count', text: "2"
+      expect(page).to have_selector '.invalid-count', text: "0"
+      expect(page).to have_selector '.create-count', text: "2"
+      expect(page).to have_selector '.update-count', text: "0"
 
-      expect(page).to have_content("Products created: 2")
+      click_button 'Save'
+      expect(page).to have_content "Products created: 2"
 
       potatoes = Spree::Product.find_by_name('Potatoes')
       potatoes.supplier.should == enterprise
@@ -53,15 +58,16 @@ feature "Product Import", js: true do
 
       expect(page).to have_content "Select a spreadsheet to upload"
       attach_file('file', '/tmp/test.csv')
-      click_button('Import')
+      click_button 'Import'
 
-      expect(page).to have_content("Valid entries found: 1")
-      expect(page).to have_content("Invalid entries found: 1")
-      expect(page).to have_content("errors")
+      expect(page).to have_selector '.item-count', text: "2"
+      expect(page).to have_selector '.invalid-count', text: "1"
+      expect(page).to have_selector '.create-count', text: "1"
+      expect(page).to have_selector '.update-count', text: "0"
 
-      click_button('Save')
-
-      expect(page).to have_content("Products created: 1")
+      expect(page).to have_selector 'input[type=submit][value="Save"]'
+      click_button 'Save'
+      expect(page).to have_content "Products created: 1"
 
       Spree::Product.find_by_name('Bad Potatoes').should == nil
       carrots = Spree::Product.find_by_name('Good Carrots')
@@ -81,14 +87,84 @@ feature "Product Import", js: true do
       visit main_app.admin_product_import_path
 
       expect(page).to have_content "Select a spreadsheet to upload"
-      attach_file('file', '/tmp/test.csv')
-      click_button('Import')
+      attach_file 'file', '/tmp/test.csv'
+      click_button 'Import'
 
-      expect(page).to have_content("Valid entries found: 0")
-      expect(page).to have_content("Invalid entries found: 2")
-      expect(page).to have_content("errors")
+      expect(page).to have_selector '.item-count', text: "2"
+      expect(page).to have_selector '.invalid-count', text: "2"
+      expect(page).to have_selector '.create-count', text: "0"
+      expect(page).to have_selector '.update-count', text: "0"
 
-      expect(page).to_not have_selector('input[type=submit]', text: "Save")
+      expect(page).to_not have_selector 'input[type=submit][value="Save"]'
+    end
+
+    it "can add new variants to existing products and update price and stock level of existing products" do
+      csv_data = CSV.generate do |csv|
+        csv << ["name", "supplier", "category", "on_hand", "price", "unit_value", "variant_unit", "variant_unit_scale", "display_name"]
+        csv << ["Hypothetical Cake", "Test Enterprise", "Cake", "5", "5.50", "500", "weight", "1", "Preexisting Banana"]
+        csv << ["Hypothetical Cake", "Test Enterprise", "Cake", "6", "3.50", "500", "weight", "1", "Emergent Coffee"]
+      end
+      File.write('/tmp/test.csv', csv_data)
+
+      visit main_app.admin_product_import_path
+      attach_file 'file', '/tmp/test.csv'
+      click_button 'Import'
+
+      expect(page).to have_selector '.item-count', text: "2"
+      expect(page).to have_selector '.invalid-count', text: "0"
+      expect(page).to have_selector '.create-count', text: "1"
+      expect(page).to have_selector '.update-count', text: "1"
+
+      click_button 'Save'
+      expect(page).to have_content "Products created: 1"
+      expect(page).to have_content "Products updated: 1"
+
+      added_coffee = Spree::Variant.find_by_display_name('Emergent Coffee')
+      added_coffee.product.name.should == 'Hypothetical Cake'
+      added_coffee.price.should == 3.50
+      added_coffee.on_hand.should == 6
+
+      updated_banana = Spree::Variant.find_by_display_name('Preexisting Banana')
+      updated_banana.product.name.should == 'Hypothetical Cake'
+      updated_banana.price.should == 5.50
+      updated_banana.on_hand.should == 5
+    end
+  end
+
+  describe "when dealing with uploaded files" do
+    before { quick_login_as_admin }
+
+    it "checks filetype on upload" do
+      File.write('/tmp/test.txt', "Wrong filetype!")
+
+      visit main_app.admin_product_import_path
+      attach_file 'file', '/tmp/test.txt'
+      click_button 'Import'
+
+      expect(page).to have_content "Importer could not process file: invalid filetype"
+      expect(page).to_not have_selector 'input[type=submit][value="Save"]'
+      expect(page).to have_content "Select a spreadsheet to upload"
+      File.delete('/tmp/test.txt')
+    end
+
+    it "returns and error if nothing was uploaded" do
+      visit main_app.admin_product_import_path
+      click_button 'Import'
+
+      expect(page).to have_content "File not found or could not be opened"
+    end
+
+    it "handles cases where no meaningful data can be read from the file" do
+      File.write('/tmp/test.csv', "A22££S\\\\\n**VA,,,AF..D")
+
+      visit main_app.admin_product_import_path
+      attach_file 'file', '/tmp/test.csv'
+      click_button 'Import'
+
+      expect(page).to have_selector '.create-count', text: "0"
+      expect(page).to have_selector '.update-count', text: "0"
+      expect(page).to_not have_selector 'input[type=submit][value="Save"]'
+      File.delete('/tmp/test.csv')
     end
   end