fix product's unit value validation

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)
 

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