From 4fd115897a76c1801d15b4f80ed111a04e12dee2 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 2 Jul 2024 13:32:20 +1000 Subject: [PATCH] Refactor ProductImport::EntryValidator Move comparaison function to ProductImport::SpreadsheetEntry --- app/models/product_import/entry_validator.rb | 21 +---- .../product_import/spreadsheet_entry.rb | 14 +++ .../product_import/spreadsheet_entry_spec.rb | 91 +++++++++++++++++++ 3 files changed, 107 insertions(+), 19 deletions(-) create mode 100644 spec/models/product_import/spreadsheet_entry_spec.rb diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 2623316400..532f0281bf 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -305,7 +305,7 @@ module ProductImport unscaled_units = entry.unscaled_units.to_f || 0 entry.unit_value = unscaled_units * unit_scale unless unit_scale.nil? - if inventory_entry_matches_existing_variant?(entry, existing_variant) + if entry.match_inventory_variant?(existing_variant) variant_override = create_inventory_item(entry, existing_variant) return validate_inventory_item(entry, variant_override) end @@ -315,23 +315,6 @@ module ProductImport error: I18n.t('admin.product_import.model.not_found')) 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 - - def display_name_are_the_same?(entry, existing_variant) - return true if entry.display_name.blank? && existing_variant.display_name.blank? - - existing_variant.display_name == entry.display_name - end - def category_validation(entry) category_name = entry.category @@ -377,7 +360,7 @@ module ProductImport products.each { |product| product_field_errors(entry, product) } products.flat_map(&:variants).each do |existing_variant| - next unless entry_matches_existing_variant?(entry, existing_variant) && + next unless entry.match_variant?(existing_variant) && existing_variant.deleted_at.nil? variant_field_errors(entry, existing_variant) diff --git a/app/models/product_import/spreadsheet_entry.rb b/app/models/product_import/spreadsheet_entry.rb index a46d061ee7..23743e6f90 100644 --- a/app/models/product_import/spreadsheet_entry.rb +++ b/app/models/product_import/spreadsheet_entry.rb @@ -84,6 +84,14 @@ module ProductImport invalid_attrs.except(* NON_PRODUCT_ATTRIBUTES, *NON_DISPLAY_ATTRIBUTES) end + def match_variant?(variant) + match_display_name?(variant) && variant.unit_value.to_d == unscaled_units.to_d + end + + def match_inventory_variant?(variant) + match_display_name?(variant) && variant.unit_value.to_d == unit_value.to_d + end + private def remove_empty_skus(attrs) @@ -99,5 +107,11 @@ module ProductImport end end end + + def match_display_name?(variant) + return true if display_name.blank? && variant.display_name.blank? + + variant.display_name == display_name + end end end diff --git a/spec/models/product_import/spreadsheet_entry_spec.rb b/spec/models/product_import/spreadsheet_entry_spec.rb new file mode 100644 index 0000000000..f2e98c86c0 --- /dev/null +++ b/spec/models/product_import/spreadsheet_entry_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: false + +require 'spec_helper' +RSpec.describe ProductImport::SpreadsheetEntry do + let(:enterprise) { create(:enterprise) } + let(:entry) { + ProductImport::SpreadsheetEntry.new( + "units" => "500", + "unit_type" => "kg", + "name" => "Tomato", + "enterprise" => enterprise, + "enterprise_id" => enterprise.id, + "producer" => enterprise, + "producer_id" => enterprise.id, + "distributor" => enterprise, + "price" => "1.0", + "on_hand" => "1", + "display_name" => display_name, + ) + } + let(:display_name) { "" } + + # TODO test match on display_name + describe "#match_variant?" do + it "returns true if matching" do + variant = create(:variant, unit_value: 500) + + expect(entry.match_variant?(variant)).to be(true) + end + + it "returns false if not machting" do + variant = create(:variant, unit_value: 250) + + expect(entry.match_variant?(variant)).to be(false) + end + + context "with same display_name" do + let(:display_name) { "Good" } + + it "returns true" do + variant = create(:variant, unit_value: 500, display_name: "Good") + + expect(entry.match_variant?(variant)).to be(true) + end + end + + context "with different display_name" do + let(:display_name) { "Bad" } + + it "returns false" do + variant = create(:variant, unit_value: 500, display_name: "Good") + + expect(entry.match_variant?(variant)).to be(false) + end + end + end + + describe "#match_inventory_variant?" do + it "returns true if matching" do + variant = create(:variant, unit_value: 500_000) + + expect(entry.match_inventory_variant?(variant)).to be(true) + end + + it "returns false if not machting" do + variant = create(:variant, unit_value: 500) + + expect(entry.match_inventory_variant?(variant)).to be(false) + end + + context "with same display_name" do + let(:display_name) { "Good" } + + it "returns true" do + variant = create(:variant, unit_value: 500_000, display_name: "Good") + + expect(entry.match_inventory_variant?(variant)).to be(true) + end + end + + context "with different display_name" do + let(:display_name) { "Bad" } + + it "returns false" do + variant = create(:variant, unit_value: 500_000, display_name: "Good") + + expect(entry.match_inventory_variant?(variant)).to be(false) + end + end + end +end