From 3ab288f4355aa801004ec26b68bde6278fb85c0b Mon Sep 17 00:00:00 2001
From: Mohamed ABDELLANI <abdellani@gmail.com>
Date: Sun, 20 Aug 2023 09:33:28 +0100
Subject: [PATCH] fix product's unit value validation

---
 app/models/spree/product.rb       | 20 ++++++++++++++++++--
 spec/models/spree/product_spec.rb | 21 +++++++++++++++++++--
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb
index f3932dac96..76284f532c 100755
--- a/app/models/spree/product.rb
+++ b/app/models/spree/product.rb
@@ -53,8 +53,7 @@ module Spree
     validates :name, presence: true
 
     validates :variant_unit, presence: true
-    validates :unit_value, numericality: { greater_than: 0 }, allow_nil: true, presence:
-      { if: ->(p) { %w(weight volume).include?(p.variant_unit) && new_record? } }
+    validate :validate_unit_value
     validates :variant_unit_scale,
               presence: { if: ->(p) { %w(weight volume).include? p.variant_unit } }
     validates :variant_unit_name,
@@ -207,6 +206,23 @@ module Spree
       }.inject(:or)
     end
 
+    def validate_unit_value
+      return unless %w(weight volume).include?(variant_unit) && new_record?
+
+      return errors.add(:unit_value, I18n.t('errors.messages.blank')) if unit_value.blank?
+
+      value = Float(unit_value, exception: false)
+
+      return if value.is_a?(Numeric) && value > 0
+
+      error = if value.nil?
+                I18n.t('errors.messages.not_a_number')
+              else
+                I18n.t('errors.messages.greater_than', count: 0)
+              end
+      errors.add(:unit_value, error)
+    end
+
     def property(property_name)
       return nil unless prop = properties.find_by(name: property_name)
 
diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb
index e646c95ec1..b006d10fec 100644
--- a/spec/models/spree/product_spec.rb
+++ b/spec/models/spree/product_spec.rb
@@ -167,8 +167,25 @@ module Spree
         expect(build(:simple_product, primary_taxon: nil)).not_to be_valid
       end
 
-      it "requires a unit value" do
-        expect(build(:simple_product, unit_value: nil)).to be_valid
+      context "unit value" do
+        it "requires a unit value when variant unit is weight" do
+          expect(build(:simple_product, variant_unit: 'weight', variant_unit_name: 'name',
+                                        unit_value: nil)).not_to be_valid
+          expect(build(:simple_product, variant_unit: 'weight', variant_unit_name: 'name',
+                                        unit_value: 0)).not_to be_valid
+        end
+
+        it "requires a unit value when variant unit is volume" do
+          expect(build(:simple_product, variant_unit: 'volume', variant_unit_name: 'name',
+                                        unit_value: nil)).not_to be_valid
+          expect(build(:simple_product, variant_unit: 'volume', variant_unit_name: 'name',
+                                        unit_value: 0)).not_to be_valid
+        end
+
+        it "does not require a unit value when variant unit is items" do
+          expect(build(:simple_product, variant_unit: 'items', variant_unit_name: 'name',
+                                        unit_value: nil)).to be_valid
+        end
       end
 
       it "requires a supplier" do