From 5884edaa1bda83e8a43ba64d8ccc78e266a33453 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 15 May 2024 21:43:54 +1000 Subject: [PATCH] Fix product import system spec --- .../spree/admin/products_controller.rb | 8 +++---- app/models/product_import/entry_processor.rb | 6 +++-- app/models/spree/product.rb | 10 ++++++-- spec/models/product_importer_spec.rb | 11 +++++---- spec/system/admin/product_import_spec.rb | 23 +++++++++++-------- 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 9071f4e20b..3ae92a38bd 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -173,12 +173,10 @@ module Spree def product_import_dates_query Spree::Variant. - select('DISTINCT spree_variants.import_date'). - joins(:product). - where(spree_products: { supplier_id: editable_enterprises.collect(&:id) }). + select('import_date').distinct. + where(supplier_id: editable_enterprises.collect(&:id)). where.not(spree_variants: { import_date: nil }). - where(spree_variants: { deleted_at: nil }). - order('spree_variants.import_date DESC') + order('import_date DESC') end def strip_new_properties diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index 6308df275c..4c585a4b32 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -227,11 +227,13 @@ module ProductImport # Ensure attributes are correctly copied to a new product's variant variant = product.variants.first variant.display_name = entry.display_name if entry.display_name - variant.on_demand = entry.on_demand if entry.on_demand - variant.on_hand = entry.on_hand if entry.on_hand variant.import_date = @import_time variant.supplier_id = entry.producer_id variant.save + + # on_demand and on_hand require a stock level, which is created after the variant is created + variant.on_demand = entry.on_demand if entry.on_demand + variant.on_hand = entry.on_hand if entry.on_hand end end end diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index f897c4553d..8cb1cfa041 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -74,9 +74,9 @@ module Spree :shipping_category_id, :primary_taxon_id, :supplier_id after_create :ensure_standard_variant + after_update :touch_supplier, if: :saved_change_to_primary_taxon_id? around_destroy :destruction after_save :update_units - after_update :touch_supplier, if: :saved_change_to_primary_taxon_id? # -- Scopes scope :with_properties, ->(*property_ids) { @@ -319,7 +319,13 @@ module Spree def update_units return unless saved_change_to_variant_unit? || saved_change_to_variant_unit_name? - variants.each(&:update_units) + variants.each do |v| + if v.persisted? + v.update_units + else + v.assign_units + end + end end def touch_supplier diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index 9ce0dbf83e..4dc3df2199 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -166,7 +166,7 @@ RSpec.describe ProductImport::ProductImporter do expect(carrots_variant.supplier).to eq enterprise expect(carrots_variant.price).to eq 3.20 expect(carrots_variant.unit_value).to eq 500 - expect(carrots_variant.on_demand).to_not eq true + expect(carrots_variant.on_demand).not_to eq true expect(carrots_variant.import_date).to be_within(1.minute).of Time.zone.now potatoes = Spree::Product.find_by(name: 'Potatoes') @@ -178,7 +178,7 @@ RSpec.describe ProductImport::ProductImporter do expect(potatoes_variant.supplier).to eq enterprise expect(potatoes_variant.price).to eq 6.50 expect(potatoes_variant.unit_value).to eq 2000 - expect(potatoes_variant.on_demand).to_not eq true + expect(potatoes_variant.on_demand).not_to eq true expect(potatoes_variant.import_date).to be_within(1.minute).of Time.zone.now pea_soup = Spree::Product.find_by(name: 'Pea Soup') @@ -190,7 +190,7 @@ RSpec.describe ProductImport::ProductImporter do expect(pea_soup_variant.supplier).to eq enterprise expect(pea_soup_variant.price).to eq 5.50 expect(pea_soup_variant.unit_value).to eq 0.75 - expect(pea_soup_variant.on_demand).to_not eq true + expect(pea_soup_variant.on_demand).not_to eq true expect(pea_soup_variant.import_date).to be_within(1.minute).of Time.zone.now salad = Spree::Product.find_by(name: 'Salad') @@ -202,7 +202,7 @@ RSpec.describe ProductImport::ProductImporter do expect(salad_variant.supplier).to eq enterprise expect(salad_variant.price).to eq 4.50 expect(salad_variant.unit_value).to eq 1 - expect(salad_variant.on_demand).to_not eq true + expect(salad_variant.on_demand).not_to eq true expect(salad_variant.import_date).to be_within(1.minute).of Time.zone.now buns = Spree::Product.find_by(name: 'Hot Cross Buns') @@ -299,7 +299,8 @@ RSpec.describe ProductImport::ProductImporter do carrots_variant = carrots.variants.first expect(carrots.on_hand).to eq 5 - expect(carrots.primary_taxon.name).to eq "Vegetables" + + expect(carrots_variant.primary_taxon.name).to eq "Vegetables" expect(carrots_variant.supplier).to eq enterprise expect(carrots_variant.price).to eq 3.20 expect(carrots_variant.shipping_category).to eq shipping_category diff --git a/spec/system/admin/product_import_spec.rb b/spec/system/admin/product_import_spec.rb index f7f37d87c3..85555f96bb 100644 --- a/spec/system/admin/product_import_spec.rb +++ b/spec/system/admin/product_import_spec.rb @@ -24,23 +24,26 @@ RSpec.describe "Product Import" do let!(:tax_category2) { create(:tax_category) } let!(:shipping_category) { create(:shipping_category) } - let!(:product) { create(:simple_product, supplier: enterprise2, name: 'Hypothetical Cake') } + let!(:product) { create(:simple_product, supplier_id: enterprise2.id, name: 'Hypothetical Cake') } let!(:variant) { create(:variant, product_id: product.id, price: '8.50', on_hand: 100, unit_value: '500', - display_name: 'Preexisting Banana') + display_name: 'Preexisting Banana', supplier: enterprise2) } let!(:product2) { - create(:simple_product, supplier: enterprise, on_hand: 100, name: 'Beans', unit_value: '500', - description: '', primary_taxon_id: category.id) + create(:simple_product, supplier_id: enterprise.id, on_hand: 100, name: 'Beans', + unit_value: '500', description: '', primary_taxon_id: category.id) } let!(:product3) { - create(:simple_product, supplier: enterprise, on_hand: 100, name: 'Sprouts', unit_value: '500') + create(:simple_product, supplier_id: enterprise.id, on_hand: 100, name: 'Sprouts', + unit_value: '500') } let!(:product4) { - create(:simple_product, supplier: enterprise, on_hand: 100, name: 'Cabbage', unit_value: '500') + create(:simple_product, supplier_id: enterprise.id, on_hand: 100, name: 'Cabbage', + unit_value: '500') } let!(:product5) { - create(:simple_product, supplier: enterprise2, on_hand: 100, name: 'Lettuce', unit_value: '500') + create(:simple_product, supplier_id: enterprise2.id, on_hand: 100, name: 'Lettuce', + unit_value: '500') } let!(:variant_override) { create(:variant_override, variant_id: product4.variants.first.id, hub: enterprise2, @@ -89,7 +92,7 @@ RSpec.describe "Product Import" do carrots = Spree::Product.find_by(name: 'Carrots') potatoes = Spree::Product.find_by(name: 'Potatoes') - expect(potatoes.supplier).to eq enterprise + expect(potatoes.variants.first.supplier).to eq enterprise expect(potatoes.on_hand).to eq 6 expect(potatoes.variants.first.price).to eq 6.50 expect(potatoes.variants.first.import_date).to be_within(1.minute).of Time.zone.now @@ -362,7 +365,7 @@ RSpec.describe "Product Import" do end it "handles a unit of kg for inventory import" do - product = create(:simple_product, supplier: enterprise, on_hand: 100, name: 'Beets', + product = create(:simple_product, supplier_id: enterprise.id, on_hand: 100, name: 'Beets', unit_value: '1000', variant_unit_scale: 1000) csv_data = <<~CSV name, distributor, producer, category, on_hand, price, unit_type, units, on_demand @@ -400,7 +403,7 @@ RSpec.describe "Product Import" do describe "Item type products" do let!(:product) { - create(:simple_product, supplier: enterprise, on_hand: nil, name: 'Aubergine', + create(:simple_product, supplier_id: enterprise.id, on_hand: nil, name: 'Aubergine', unit_value: '1', variant_unit_scale: nil, variant_unit: "items", variant_unit_name: "Bag") }