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 75b506fc66..d9b73cdabf 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 @@ -51,7 +51,7 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt $scope.start = () -> $scope.started = true total = ams_data.item_count - size = 100 + size = 50 $scope.chunks = Math.ceil(total / size) i = 0 diff --git a/app/assets/javascripts/admin/product_import/filters/filter_entries.js.coffee b/app/assets/javascripts/admin/product_import/filters/filter_entries.js.coffee index 4a1007b3a9..efbcf5652b 100644 --- a/app/assets/javascripts/admin/product_import/filters/filter_entries.js.coffee +++ b/app/assets/javascripts/admin/product_import/filters/filter_entries.js.coffee @@ -10,7 +10,7 @@ angular.module("admin.productImport").filter 'entriesFilterValid', -> if type == 'valid' and validates_as != '' \ or type == 'invalid' and validates_as == '' \ - or type == 'create_product' and validates_as == 'new_product' or validates_as == 'new_variant' \ + or type == 'create_product' and (validates_as == 'new_product' or validates_as == 'new_variant') \ or type == 'update_product' and validates_as == 'existing_variant' \ or type == 'create_inventory' and validates_as == 'new_inventory_item' \ or type == 'update_inventory' and validates_as == 'existing_inventory_item' diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index 97f9cbe55c..14cfd67f2d 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -115,7 +115,13 @@ module ProductImport return unless entry.validates_as? 'existing_variant' - save_variant entry + begin + save_variant entry + rescue ActiveRecord::StaleObjectError + entry.product_object.reload + save_variant entry + end + @variants_updated += 1 end diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 4f48c7745c..33a7932f4f 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -124,16 +124,15 @@ module ProductImport end def inventory_validation(entry) - # Checks a potential inventory item corresponds to a valid variant - match = Spree::Product.where(supplier_id: entry.producer_id, name: entry.name, deleted_at: nil).first + products = Spree::Product.where(supplier_id: entry.producer_id, name: entry.name, deleted_at: nil) - if match.nil? + if products.empty? mark_as_invalid(entry, attribute: 'name', error: I18n.t('admin.product_import.model.no_product')) return end - match.variants.each do |existing_variant| - unit_scale = match.variant_unit_scale + products.flat_map(&:variants).each do |existing_variant| + unit_scale = existing_variant.product.variant_unit_scale unscaled_units = entry.unscaled_units || 0 entry.unit_value = unscaled_units * unit_scale @@ -176,24 +175,20 @@ module ProductImport end def product_validation(entry) - # Find product with matching supplier and name - match = Spree::Product.where(supplier_id: entry.supplier_id, name: entry.name, deleted_at: nil).first + products = Spree::Product.where(supplier_id: entry.supplier_id, name: entry.name, deleted_at: nil) - # If no matching product was found, create a new product - if match.nil? + if products.empty? mark_as_new_product(entry) return end - # Otherwise, if a variant exists with matching display_name and unit_value, update it - match.variants.each do |existing_variant| + products.flat_map(&:variants).each do |existing_variant| if entry_matches_existing_variant?(entry, existing_variant) && existing_variant.deleted_at.nil? return mark_as_existing_variant(entry, existing_variant) end end - # Otherwise, a variant with sufficiently matching attributes doesn't exist; create a new one - mark_as_new_variant(entry, match.id) + mark_as_new_variant(entry, products.first.id) end def mark_as_new_product(entry) diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index 6644512a00..5d98cadb5e 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -201,6 +201,44 @@ feature "Product Import", js: true do expect(Spree::Product.find_by_name('Beans').on_hand).to eq 0 end + it "can save a new product and variant of that product at the same time, add variant to existing product" do + csv_data = CSV.generate do |csv| + csv << ["name", "supplier", "category", "on_hand", "price", "units", "unit_type", "display_name"] + csv << ["Potatoes", "User Enterprise", "Vegetables", "5", "3.50", "500", "g", "Small Bag"] + csv << ["Potatoes", "User Enterprise", "Vegetables", "6", "5.50", "2", "kg", "Big Bag"] + csv << ["Beans", "User Enterprise", "Vegetables", "7", "2.50", "250", "g", nil] + end + File.write('/tmp/test.csv', csv_data) + + visit main_app.admin_product_import_path + attach_file 'file', '/tmp/test.csv' + click_button 'Upload' + + import_data + + expect(page).to have_selector '.item-count', text: "3" + expect(page).to_not have_selector '.invalid-count' + expect(page).to have_selector '.create-count', text: "3" + expect(page).to_not have_selector '.update-count' + expect(page).to_not have_selector '.inv-create-count' + expect(page).to_not have_selector '.inv-update-count' + + save_data + + small_bag = Spree::Variant.find_by_display_name('Small Bag') + expect(small_bag.product.name).to eq 'Potatoes' + expect(small_bag.price).to eq 3.50 + expect(small_bag.on_hand).to eq 5 + + big_bag = Spree::Variant.find_by_display_name('Big Bag') + expect(big_bag.product.name).to eq 'Potatoes' + expect(big_bag.price).to eq 5.50 + expect(big_bag.on_hand).to eq 6 + + expect(big_bag.product.id).to eq small_bag.product.id + end + + it "can import items into inventory" do csv_data = CSV.generate do |csv| csv << ["name", "supplier", "producer", "category", "on_hand", "price", "units"] diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index c0cbc6718f..b587ad8d76 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -16,6 +16,7 @@ describe ProductImport::ProductImporter do let!(:category) { create(:taxon, name: 'Vegetables') } let!(:category2) { create(:taxon, name: 'Cake') } + let!(:category3) { create(:taxon, name: 'Cereal') } let!(:tax_category) { create(:tax_category) } let!(:tax_category2) { create(:tax_category) } let!(:shipping_category) { create(:shipping_category) } @@ -28,6 +29,13 @@ describe ProductImport::ProductImporter do let!(:product5) { create(:simple_product, supplier: enterprise2, on_hand: '100', name: 'Lettuce', unit_value: '500') } let!(:product6) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Beetroot', unit_value: '500', on_demand: true, variant_unit_scale: 1, variant_unit: 'weight') } let!(:product7) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Tomato', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight') } + + let!(:product8) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category3.id) } + let!(:product9) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category3.id) } + let!(:variant2) { create(:variant, product_id: product8.id, price: '4.50', on_hand: '100', unit_value: '500', display_name: 'Porridge Oats') } + let!(:variant3) { create(:variant, product_id: product8.id, price: '5.50', on_hand: '100', unit_value: '500', display_name: 'Rolled Oats') } + let!(:variant4) { create(:variant, product_id: product9.id, price: '6.50', on_hand: '100', unit_value: '500', display_name: 'Flaked Oats') } + let!(:variant_override) { create(:variant_override, variant_id: product4.variants.first.id, hub: enterprise2, count_on_hand: 42) } let!(:variant_override2) { create(:variant_override, variant_id: product5.variants.first.id, hub: enterprise, count_on_hand: 96) } @@ -196,7 +204,7 @@ describe ProductImport::ProductImporter do end File.write('/tmp/test-m.csv', csv_data) file = File.new('/tmp/test-m.csv') - settings = {enterprise2.id.to_s => {'import_into' => 'product_list'}} + settings = {'import_into' => 'product_list'} @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) end after { File.delete('/tmp/test-m.csv') } @@ -242,7 +250,7 @@ describe ProductImport::ProductImporter do end File.write('/tmp/test-m.csv', csv_data) file = File.new('/tmp/test-m.csv') - settings = {enterprise.id.to_s => {'import_into' => 'product_list'}} + settings = {'import_into' => 'product_list'} @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) end after { File.delete('/tmp/test-m.csv') } @@ -272,6 +280,8 @@ describe ProductImport::ProductImporter do expect(big_bag.product.name).to eq 'Potatoes' expect(big_bag.price).to eq 5.50 expect(big_bag.on_hand).to eq 6 + + expect(big_bag.product.id).to eq small_bag.product.id end end @@ -284,7 +294,7 @@ describe ProductImport::ProductImporter do end File.write('/tmp/test-m.csv', csv_data) file = File.new('/tmp/test-m.csv') - settings = {enterprise3.id.to_s => {'import_into' => 'product_list'}} + settings = {'import_into' => 'product_list'} @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) end after { File.delete('/tmp/test-m.csv') } @@ -317,6 +327,115 @@ describe ProductImport::ProductImporter do end end + describe "when more than one product of the same name already exists with multiple variants each" do + before do + csv_data = CSV.generate do |csv| + csv << ["name", "supplier", "category", "on_hand", "price", "units", "unit_type", "display_name"] + csv << ["Oats", "User Enterprise", "Cereal", "50", "3.50", "500", "g", "Rolled Oats"] # Update + csv << ["Oats", "User Enterprise", "Cereal", "80", "3.75", "500", "g", "Flaked Oats"] # Update + csv << ["Oats", "User Enterprise", "Cereal", "60", "5.50", "500", "g", "Magic Oats"] # Add + csv << ["Oats", "User Enterprise", "Cereal", "70", "8.50", "500", "g", "French Oats"] # Add + csv << ["Oats", "User Enterprise", "Cereal", "70", "8.50", "500", "g", "Scottish Oats"] # Add + end + File.write('/tmp/test-m.csv', csv_data) + file = File.new('/tmp/test-m.csv') + settings = {'import_into' => 'product_list'} + @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) + end + after { File.delete('/tmp/test-m.csv') } + + it "validates entries" do + @importer.validate_entries + entries = JSON.parse(@importer.entries_json) + + expect(filter('valid', entries)).to eq 5 + expect(filter('invalid', entries)).to eq 0 + expect(filter('create_product', entries)).to eq 3 + expect(filter('update_product', entries)).to eq 2 + expect(filter('create_inventory', entries)).to eq 0 + expect(filter('update_inventory', entries)).to eq 0 + end + + it "saves and updates" do + @importer.save_entries + + expect(@importer.products_created_count).to eq 3 + expect(@importer.products_updated_count).to eq 2 + expect(@importer.inventory_created_count).to eq 0 + expect(@importer.inventory_updated_count).to eq 0 + expect(@importer.updated_ids.count).to eq 5 + end + end + + describe "when importer processes create and update across multiple stages" do + before do + csv_data = CSV.generate do |csv| + csv << ["name", "supplier", "category", "on_hand", "price", "units", "unit_type", "display_name"] + csv << ["Bag of Oats", "User Enterprise", "Cereal", "60", "5.50", "500", "g", "Magic Oats"] # Add + csv << ["Bag of Oats", "User Enterprise", "Cereal", "70", "8.50", "500", "g", "French Oats"] # Add + csv << ["Bag of Oats", "User Enterprise", "Cereal", "80", "9.50", "500", "g", "Organic Oats"] # Add + csv << ["Bag of Oats", "User Enterprise", "Cereal", "90", "7.50", "500", "g", "Scottish Oats"] # Add + csv << ["Bag of Oats", "User Enterprise", "Cereal", "30", "6.50", "500", "g", "Breakfast Oats"] # Add + end + File.write('/tmp/test-m.csv', csv_data) + @file = File.new('/tmp/test-m.csv') + @settings = {'import_into' => 'product_list'} + end + after { File.delete('/tmp/test-m.csv') } + + it "processes the validation in stages" do + # Using settings of start: 1, end: 2 to simulate import over multiple stages + @importer = ProductImport::ProductImporter.new(@file, admin, start: 1, end: 3, settings: @settings) + + @importer.validate_entries + entries = JSON.parse(@importer.entries_json) + + expect(filter('valid', entries)).to eq 3 + expect(filter('invalid', entries)).to eq 0 + expect(filter('create_product', entries)).to eq 3 + expect(filter('update_product', entries)).to eq 0 + expect(filter('create_inventory', entries)).to eq 0 + expect(filter('update_inventory', entries)).to eq 0 + + @importer = ProductImport::ProductImporter.new(@file, admin, start: 4, end: 6, settings: @settings) + + @importer.validate_entries + entries = JSON.parse(@importer.entries_json) + + expect(filter('valid', entries)).to eq 2 + expect(filter('invalid', entries)).to eq 0 + expect(filter('create_product', entries)).to eq 2 + expect(filter('update_product', entries)).to eq 0 + expect(filter('create_inventory', entries)).to eq 0 + expect(filter('update_inventory', entries)).to eq 0 + end + + it "processes saving in stages" do + @importer = ProductImport::ProductImporter.new(@file, admin, start: 1, end: 3, settings: @settings) + @importer.save_entries + + expect(@importer.products_created_count).to eq 3 + expect(@importer.products_updated_count).to eq 0 + expect(@importer.inventory_created_count).to eq 0 + expect(@importer.inventory_updated_count).to eq 0 + expect(@importer.updated_ids.count).to eq 3 + + @importer = ProductImport::ProductImporter.new(@file, admin, start: 4, end: 6, settings: @settings) + @importer.save_entries + + expect(@importer.products_created_count).to eq 2 + expect(@importer.products_updated_count).to eq 0 + expect(@importer.inventory_created_count).to eq 0 + expect(@importer.inventory_updated_count).to eq 0 + expect(@importer.updated_ids.count).to eq 2 + + products = Spree::Product.find_all_by_name('Bag of Oats') + + expect(products.count).to eq 1 + expect(products.first.variants.count).to eq 5 + end + end + describe "importing items into inventory" do before do csv_data = CSV.generate do |csv| @@ -482,7 +601,7 @@ describe ProductImport::ProductImporter do @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, updated_ids: updated_ids, enterprises_to_reset: [enterprise.id], settings: settings) @importer.reset_absent(updated_ids) - expect(@importer.products_reset_count).to eq 2 + expect(@importer.products_reset_count).to eq 7 expect(Spree::Product.find_by_name('Carrots').on_hand).to eq 5 # Present in file, added expect(Spree::Product.find_by_name('Beans').on_hand).to eq 6 # Present in file, updated