Refactor ProductImport::EntryValidator

Move comparaison function to ProductImport::SpreadsheetEntry

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)

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

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