Fix variant data inconsistencies

app/models/spree/variant.rb

@@ -65,6 +65,7 @@ class Variant < ActiveRecord::Base
 
     before_validation :set_cost_currency
     before_validation :update_weight_from_unit_value, if: ->(v) { v.product.present? }
+    before_validation :ensure_unit_value
 
     after_save :save_default_price
     after_save :update_units
@@ -297,5 +298,11 @@ def destruction
       exchange_variants(:reload).destroy_all
       yield
     end
+
+    def ensure_unit_value
+      return unless product&.variant_unit == "items" && unit_value.nil?
+
+      self.unit_value = 1.0
+    end
   end
 end

spec/controllers/spree/admin/products_controller_spec.rb

@@ -40,7 +40,7 @@
 
       before { controller_login_as_enterprise_user([producer]) }
 
-      it 'fails' do
+      it 'succeeds' do
         spree_post :bulk_update,
                    "products" => [
                      {
@@ -50,7 +50,7 @@
                      }
                    ]
 
-        expect(response).to have_http_status(400)
+        expect(response).to have_http_status(302)
       end
 
       it 'does not redirect to bulk_products' do
@@ -63,8 +63,8 @@
                      }
                    ]
 
-        expect(response).not_to redirect_to(
-          '/api/products/bulk_products?page=1;per_page=500;'
+        expect(response).to redirect_to(
+          '/api/products/bulk_products'
         )
       end
     end

spec/models/spree/product_set_spec.rb

@@ -32,7 +32,7 @@
       end
 
       context 'when the product does exist' do
-        context 'when a different varian_unit is passed' do
+        context 'when a different variant_unit is passed' do
           let!(:product) do
             create(
               :simple_product,
@@ -54,22 +54,22 @@
             }
           end
 
-          it 'does not update the product' do
+          it 'updates the product' do
             product_set.save
 
             expect(product.reload.attributes).to include(
-              'variant_unit' => 'items'
+              'variant_unit' => 'weight'
             )
           end
 
-          it 'adds an error' do
+          it 'does not add an error' do
             product_set.save
-            expect(product_set.errors.get(:base))
-              .to include("Unit value can't be blank")
+            expect(product_set.errors)
+              .to be_empty
           end
 
-          it 'returns false' do
-            expect(product_set.save).to eq(false)
+          it 'returns true' do
+            expect(product_set.save).to eq(true)
           end
         end
 

spec/models/spree/variant_spec.rb

@@ -535,6 +535,7 @@ module Spree
         before do
           product.update_attribute :variant_unit, 'items'
           product.reload
+          variant.reload
         end
 
         it "is valid with only unit value set" do
@@ -549,10 +550,11 @@ module Spree
           expect(variant).to be_valid
         end
 
-        it "is invalid when neither unit value nor unit description are set" do
+        it "sets unit_value to 1.0 before validation if it's nil" do
           variant.unit_value = nil
           variant.unit_description = nil
-          expect(variant).not_to be_valid
+          expect(variant).to be_valid
+          expect(variant.unit_value).to eq 1.0
         end
 
         it "has a valid master variant" do
@@ -783,4 +785,17 @@ module Spree
       expect(e.reload.variant_ids).to be_empty
     end
   end
+
+  describe "#ensure_unit_value" do
+    let(:product) { create(:product, variant_unit: "weight") }
+    let(:variant) { create(:variant, product_id: product.id) }
+
+    context "when a product's variant_unit value is changed from weight to items" do
+      it "sets the variant's unit_value to 1" do
+        product.update(variant_unit: "items")
+
+        expect(variant.unit_value).to eq 1
+      end
+    end
+  end
 end