From e22804712e60945f92ec12797d4821c29ecb5f1f Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 2 Jul 2024 10:48:34 +1000 Subject: [PATCH] Fix product importer --- app/models/product_import/entry_processor.rb | 3 ++ app/models/product_import/entry_validator.rb | 45 ++++++++++++++------ spec/models/product_importer_spec.rb | 29 +++++++------ 3 files changed, 52 insertions(+), 25 deletions(-) diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index c7596cc1ae..6393f14578 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -224,6 +224,9 @@ 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.variant_unit = entry.variant_unit if entry.variant_unit + variant.variant_unit_name = entry.variant_unit_name if entry.variant_unit_name + variant.variant_unit_scale = entry.variant_unit_scale if entry.variant_unit_scale variant.import_date = @import_time variant.supplier_id = entry.producer_id variant.save diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 4db5a60d2c..2623316400 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -22,9 +22,14 @@ module ProductImport end # rubocop:enable Metrics/ParameterLists - def self.non_updatable_fields + def self.non_updatable_product_fields { description: :description, + } + end + + def self.non_updatable_variant_fields + { unit_type: :variant_unit_scale, variant_unit_name: :variant_unit_name, } @@ -67,8 +72,7 @@ module ProductImport def mark_as_new_variant(entry, product_id) variant_attributes = entry.assignable_attributes.except( - 'id', 'product_id', 'on_hand', 'on_demand', 'variant_unit', 'variant_unit_name', - 'variant_unit_scale' + 'id', 'product_id', 'on_hand', 'on_demand' ) # Variant needs a product. Product needs to be assigned first in order for # delegate to work. name= will fail otherwise. @@ -297,11 +301,11 @@ module ProductImport end products.flat_map(&:variants).each do |existing_variant| - unit_scale = existing_variant.product.variant_unit_scale + unit_scale = existing_variant.variant_unit_scale unscaled_units = entry.unscaled_units.to_f || 0 entry.unit_value = unscaled_units * unit_scale unless unit_scale.nil? - if entry_matches_existing_variant?(entry, existing_variant) + if inventory_entry_matches_existing_variant?(entry, existing_variant) variant_override = create_inventory_item(entry, existing_variant) return validate_inventory_item(entry, variant_override) end @@ -312,6 +316,12 @@ module ProductImport end def entry_matches_existing_variant?(entry, existing_variant) + # matching on the unscaled unit so we know if we are trying to update an existing variant + display_name_are_the_same?(entry, existing_variant) && + existing_variant.unit_value == entry.unscaled_units.to_f + end + + def inventory_entry_matches_existing_variant?(entry, existing_variant) display_name_are_the_same?(entry, existing_variant) && existing_variant.unit_value == entry.unit_value.to_f end @@ -367,10 +377,12 @@ module ProductImport products.each { |product| product_field_errors(entry, product) } 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 + next unless entry_matches_existing_variant?(entry, existing_variant) && + existing_variant.deleted_at.nil? + + variant_field_errors(entry, existing_variant) + + return mark_as_existing_variant(entry, existing_variant) end mark_as_new_variant(entry, products.first.id) @@ -392,8 +404,7 @@ module ProductImport def mark_as_existing_variant(entry, existing_variant) existing_variant.assign_attributes( - entry.assignable_attributes.except('id', 'product_id', 'variant_unit', 'variant_unit_name', - 'variant_unit_scale') + entry.assignable_attributes.except('id', 'product_id') ) check_on_hand_nil(entry, existing_variant) @@ -406,8 +417,18 @@ module ProductImport end end + def variant_field_errors(entry, existing_variant) + EntryValidator.non_updatable_variant_fields.each do |display_name, attribute| + next if attributes_match?(attribute, existing_variant, entry) || + attributes_blank?(attribute, existing_variant, entry) + + mark_as_invalid(entry, attribute: display_name, + error: I18n.t('admin.product_import.model.not_updatable')) + end + end + def product_field_errors(entry, existing_product) - EntryValidator.non_updatable_fields.each do |display_name, attribute| + EntryValidator.non_updatable_product_fields.each do |display_name, attribute| next if attributes_match?(attribute, existing_product, entry) || attributes_blank?(attribute, existing_product, entry) next if ignore_when_updating_product?(attribute) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index 4dc3df2199..d514193bbf 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -160,60 +160,60 @@ RSpec.describe ProductImport::ProductImporter do carrots = Spree::Product.find_by(name: 'Carrots') carrots_variant = carrots.variants.first expect(carrots.on_hand).to eq 5 - expect(carrots.variant_unit).to eq 'weight' - expect(carrots.variant_unit_scale).to eq 1 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_unit).to eq 'weight' + expect(carrots_variant_unit_scale).to eq 1 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') potatoes_variant = potatoes.variants.first expect(potatoes.on_hand).to eq 6 - expect(potatoes.variant_unit).to eq 'weight' - expect(potatoes.variant_unit_scale).to eq 1000 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_unit).to eq 'weight' + expect(potatoes_variant_unit_scale).to eq 1000 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') pea_soup_variant = pea_soup.variants.first expect(pea_soup.on_hand).to eq 8 - expect(pea_soup.variant_unit).to eq 'volume' - expect(pea_soup.variant_unit_scale).to eq 0.001 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_unit).to eq 'volume' + expect(pea_soup_variant_unit_scale).to eq 0.001 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') salad_variant = salad.variants.first expect(salad.on_hand).to eq 7 - expect(salad.variant_unit).to eq 'items' - expect(salad.variant_unit_scale).to eq nil 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_unit).to eq 'items' + expect(salad_variant_unit_scale).to eq nil 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') buns_variant = buns.variants.first expect(buns.on_hand).to eq 7 - expect(buns.variant_unit).to eq 'items' - expect(buns.variant_unit_scale).to eq nil 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_unit).to eq 'items' + expect(buns_variant_unit_scale).to eq nil expect(buns_variant.on_demand).to eq true expect(buns_variant.import_date).to be_within(1.minute).of Time.zone.now end @@ -578,9 +578,12 @@ RSpec.describe ProductImport::ProductImporter do describe "updating non-updatable fields on existing products" do let(:csv_data) { CSV.generate do |csv| - csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type"] - csv << ["Beetroot", enterprise3.name, "Vegetables", "5", "3.50", "500", "Kg"] - csv << ["Tomato", enterprise3.name, "Vegetables", "6", "5.50", "500", "Kg"] + csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", + "shipping_category"] + csv << ["Beetroot", enterprise3.name, "Vegetables", "5", "3.50", "500", "Kg", + shipping_category.name] + csv << ["Tomato", enterprise3.name, "Vegetables", "6", "5.50", "500", "Kg", + shipping_category.name] end } let(:importer) { import_data csv_data }