From d5cc60fd3a1052a9373f7ab2b29981bf8946902e Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 4 Mar 2024 16:22:42 +1100 Subject: [PATCH] Fix ProductImporter and related Class --- app/models/product_import/entry_processor.rb | 2 +- app/models/product_import/entry_validator.rb | 6 +- .../product_import/products_reset_strategy.rb | 7 +- .../products_reset_strategy_spec.rb | 2 +- spec/models/product_importer_spec.rb | 130 ++++++++++-------- 5 files changed, 78 insertions(+), 69 deletions(-) diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index 05bc462eff..6308df275c 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -169,7 +169,6 @@ module ProductImport product.assign_attributes( entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name') ) - product.supplier_id = entry.producer_id if product.save ensure_variant_updated(product, entry) @@ -231,6 +230,7 @@ module ProductImport 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 end end diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 5e7172948b..2705ff1126 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -73,6 +73,7 @@ module ProductImport # Variant needs a product. Product needs to be assigned first in order for # delegate to work. name= will fail otherwise. new_variant = Spree::Variant.new(product_id:, **variant_attributes) + new_variant.supplier_id = entry.producer_id new_variant.save if new_variant.persisted? @@ -356,9 +357,7 @@ module ProductImport end def product_validation(entry) - products = Spree::Product.where(supplier_id: entry.enterprise_id, - name: entry.name, - deleted_at: nil) + products = Spree::Product.in_supplier(entry.enterprise_id).where(name: entry.name) if products.empty? mark_as_new_product(entry) @@ -382,7 +381,6 @@ module ProductImport new_product.assign_attributes( entry.assignable_attributes.except('id', 'on_hand', 'on_demand', 'display_name') ) - new_product.supplier_id = entry.producer_id entry.on_hand = 0 if entry.on_hand.nil? if new_product.valid? diff --git a/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb b/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb index ac7dac8de4..f4a8b9a2dd 100644 --- a/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb +++ b/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb @@ -20,12 +20,7 @@ module Catalog attr_reader :excluded_items_ids, :enterprise_ids def enterprise_variants_relation - relation = Spree::Variant - .joins(:product) - .where( - spree_products: { supplier_id: enterprise_ids }, - spree_variants: { deleted_at: nil } - ) + relation = Spree::Variant.where(supplier_id: enterprise_ids) return relation if excluded_items_ids.blank? diff --git a/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb b/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb index c6d43be45f..d407537e26 100644 --- a/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb +++ b/engines/catalog/spec/services/catalog/product_import/products_reset_strategy_spec.rb @@ -10,8 +10,8 @@ module Catalog describe '#reset' do let(:supplier_ids) { enterprise.id } let(:product) { create(:product) } - let(:enterprise) { product.supplier } let(:variant) { product.variants.first } + let(:enterprise) { variant.supplier } before { variant.on_hand = 2 } diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index 39b6508575..9ce0dbf83e 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -36,67 +36,70 @@ RSpec.describe ProductImport::ProductImporter do let!(:shipping_category) { create(:shipping_category) } let!(:product) { - create(:simple_product, supplier: enterprise2, name: 'Hypothetical Cake', description: nil, - primary_taxon_id: category2.id) + create(:simple_product, name: 'Hypothetical Cake', description: nil, + primary_taxon_id: category2.id, supplier_id: enterprise2.id, + variants: []) } 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!(:variant_with_empty_display_name) { create(:variant, product_id: product.id, price: '8.50', on_hand: '100', unit_value: '500', - display_name: '') + display_name: '', supplier: enterprise2) } let!(:product2) { - create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Beans', unit_value: '500', - primary_taxon_id: category.id, description: nil) + create(:simple_product, on_hand: '100', name: 'Beans', unit_value: '500', + primary_taxon_id: category.id, description: nil, variants: [] ) } let!(:product3) { - create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Sprouts', - unit_value: '500', primary_taxon_id: category.id) + create(:simple_product, on_hand: '100', name: 'Sprouts', unit_value: '500', + primary_taxon_id: category.id, supplier_id: enterprise.id) } let!(:product4) { - create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Cabbage', unit_value: '1', + create(:simple_product, on_hand: '100', name: 'Cabbage', unit_value: '1', variant_unit_scale: nil, variant_unit: "items", - variant_unit_name: "Whole", primary_taxon_id: category.id) + variant_unit_name: "Whole", primary_taxon_id: category.id, + supplier_id: enterprise.id) } let!(:product5) { - create(:simple_product, supplier: enterprise2, on_hand: '100', name: 'Lettuce', - unit_value: '500', primary_taxon_id: category.id) + create(:simple_product, on_hand: '100', name: 'Lettuce', unit_value: '500', + primary_taxon_id: category.id, supplier_id: enterprise2.id) } let!(:product6) { - create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Beetroot', + create(:simple_product, on_hand: '100', name: 'Beetroot', unit_value: '500', on_demand: true, variant_unit_scale: 1, - variant_unit: 'weight', primary_taxon_id: category.id, description: nil) + variant_unit: 'weight', primary_taxon_id: category.id, description: nil, + supplier_id: enterprise3.id) } let!(:product7) { - create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Tomato', - unit_value: '500', variant_unit_scale: 1, - variant_unit: 'weight', primary_taxon_id: category.id, - description: nil) + create(:simple_product, on_hand: '100', name: 'Tomato', unit_value: '500', + variant_unit_scale: 1, variant_unit: 'weight', + primary_taxon_id: category.id, description: nil, + supplier_id: enterprise3.id) } let!(:product8) { - create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', description: "", + create(:simple_product, on_hand: '100', name: 'Oats', description: "", unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category4.id) } let!(:product9) { - create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Oats', description: "", + create(:simple_product, on_hand: '100', name: 'Oats', description: "", unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category4.id) } let!(:variant2) { create(:variant, product_id: product8.id, price: '4.50', on_hand: '100', unit_value: '500', - display_name: 'Porridge Oats') + display_name: 'Porridge Oats', supplier: enterprise) } let!(:variant3) { create(:variant, product_id: product8.id, price: '5.50', on_hand: '100', unit_value: '500', - display_name: 'Rolled Oats') + display_name: 'Rolled Oats', supplier: enterprise) } let!(:variant4) { create(:variant, product_id: product9.id, price: '6.50', on_hand: '100', unit_value: '500', - display_name: 'Flaked Oats') + display_name: 'Flaked Oats', supplier: enterprise) } let!(:variant_override) { @@ -155,54 +158,64 @@ RSpec.describe ProductImport::ProductImporter do expect(importer.updated_ids.count).to eq 5 carrots = Spree::Product.find_by(name: 'Carrots') - expect(carrots.supplier).to eq enterprise + carrots_variant = carrots.variants.first expect(carrots.on_hand).to eq 5 - expect(carrots.variants.first.price).to eq 3.20 - expect(carrots.variants.first.unit_value).to eq 500 expect(carrots.variant_unit).to eq 'weight' expect(carrots.variant_unit_scale).to eq 1 - expect(carrots.variants.first.on_demand).not_to eq true - expect(carrots.variants.first.import_date).to be_within(1.minute).of Time.zone.now + + 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.import_date).to be_within(1.minute).of Time.zone.now potatoes = Spree::Product.find_by(name: 'Potatoes') - expect(potatoes.supplier).to eq enterprise + potatoes_variant = potatoes.variants.first expect(potatoes.on_hand).to eq 6 - expect(potatoes.variants.first.price).to eq 6.50 - expect(potatoes.variants.first.unit_value).to eq 2000 expect(potatoes.variant_unit).to eq 'weight' expect(potatoes.variant_unit_scale).to eq 1000 - expect(potatoes.variants.first.on_demand).not_to eq true - expect(potatoes.variants.first.import_date).to be_within(1.minute).of Time.zone.now + + 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.import_date).to be_within(1.minute).of Time.zone.now pea_soup = Spree::Product.find_by(name: 'Pea Soup') - expect(pea_soup.supplier).to eq enterprise + pea_soup_variant = pea_soup.variants.first expect(pea_soup.on_hand).to eq 8 - expect(pea_soup.variants.first.price).to eq 5.50 - expect(pea_soup.variants.first.unit_value).to eq 0.75 expect(pea_soup.variant_unit).to eq 'volume' expect(pea_soup.variant_unit_scale).to eq 0.001 - expect(pea_soup.variants.first.on_demand).not_to eq true - expect(pea_soup.variants.first.import_date).to be_within(1.minute).of Time.zone.now + + 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.import_date).to be_within(1.minute).of Time.zone.now salad = Spree::Product.find_by(name: 'Salad') - expect(salad.supplier).to eq enterprise + salad_variant = salad.variants.first expect(salad.on_hand).to eq 7 - expect(salad.variants.first.price).to eq 4.50 - expect(salad.variants.first.unit_value).to eq 1 expect(salad.variant_unit).to eq 'items' expect(salad.variant_unit_scale).to eq nil - expect(salad.variants.first.on_demand).not_to eq true - expect(salad.variants.first.import_date).to be_within(1.minute).of Time.zone.now + + 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.import_date).to be_within(1.minute).of Time.zone.now buns = Spree::Product.find_by(name: 'Hot Cross Buns') - expect(buns.supplier).to eq enterprise + buns_variant = buns.variants.first expect(buns.on_hand).to eq 7 - expect(buns.variants.first.price).to eq 3.50 - expect(buns.variants.first.unit_value).to eq 1 expect(buns.variant_unit).to eq 'items' expect(buns.variant_unit_scale).to eq nil - expect(buns.variants.first.on_demand).to eq true - expect(buns.variants.first.import_date).to be_within(1.minute).of Time.zone.now + + expect(buns_variant.supplier).to eq enterprise + expect(buns_variant.price).to eq 3.50 + expect(buns_variant.unit_value).to eq 1 + expect(buns_variant.on_demand).to eq true + expect(buns_variant.import_date).to be_within(1.minute).of Time.zone.now end end @@ -236,10 +249,11 @@ RSpec.describe ProductImport::ProductImporter do expect(importer.updated_ids.count).to eq 1 carrots = Spree::Product.find_by(name: 'Good Carrots') - expect(carrots.supplier).to eq enterprise + carrots_variant = carrots.variants.first expect(carrots.on_hand).to eq 5 - expect(carrots.variants.first.price).to eq 3.20 - expect(carrots.variants.first.import_date).to be_within(1.minute).of Time.zone.now + expect(carrots_variant.supplier).to eq enterprise + expect(carrots_variant.price).to eq 3.20 + expect(carrots_variant.import_date).to be_within(1.minute).of Time.zone.now expect(Spree::Product.find_by(name: 'Bad Potatoes')).to eq nil end @@ -282,12 +296,14 @@ RSpec.describe ProductImport::ProductImporter do expect(importer.products_created_count).to eq 1 carrots = Spree::Product.find_by(name: 'Good Carrots') + carrots_variant = carrots.variants.first + expect(carrots.on_hand).to eq 5 - expect(carrots.variants.first.price).to eq 3.20 - expect(carrots.variants.first.primary_taxon.name).to eq "Vegetables" - expect(carrots.variants.first.shipping_category).to eq shipping_category - expect(carrots.supplier).to eq enterprise - expect(carrots.variants.first.unit_presentation).to eq "500g" + expect(carrots.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 + expect(carrots_variant.unit_presentation).to eq "500g" end end @@ -446,7 +462,7 @@ RSpec.describe ProductImport::ProductImporter do CSV.generate do |csv| csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", "display_name", "shipping_category"] - csv << ["Hypothetical Cake", enterprise2.name, "Cake", "5", "5.50", "1", "g", + csv << ["Hypothetical Cake", enterprise2.name, "Cake", "5", "5.50", "500", "g", "", shipping_category.name] end }