Merge pull request #9741 from mkllnk/empty-variant-param

Avoid error when sending empty variant attributes in bulk product edit

app/services/sets/product_set.rb

@@ -47,10 +47,9 @@ module Sets
     end
 
     def update_product(product, attributes)
-      original_supplier = product.supplier_id
       return false unless update_product_only_attributes(product, attributes)
 
-      ExchangeVariantDeleter.new.delete(product) if original_supplier != product.supplier_id
+      ExchangeVariantDeleter.new.delete(product) if product.saved_change_to_supplier_id?
 
       update_product_variants(product, attributes) &&
         update_product_master(product, attributes)
@@ -110,6 +109,8 @@ module Sets
     end
 
     def create_variant(product, variant_attributes)
+      return if variant_attributes.blank?
+
       on_hand = variant_attributes.delete(:on_hand)
       on_demand = variant_attributes.delete(:on_demand)
 

spec/services/sets/product_set_spec.rb

@@ -75,17 +75,9 @@ describe Sets::ProductSet do
           end
         end
 
-        context 'when a different supplier is passed' do
-          let!(:producer) { create(:enterprise) }
-          let!(:product) { create(:simple_product) }
-          let(:collection_hash) do
-            {
-              0 => {
-                id: product.id,
-                supplier_id: producer.id
-              }
-            }
-          end
+        context "when the product is in an order cycle" do
+          let(:producer) { create(:enterprise) }
+          let(:product) { create(:simple_product) }
 
           let(:distributor) { create(:distributor_enterprise) }
           let!(:order_cycle) {
@@ -94,13 +86,36 @@ describe Sets::ProductSet do
                                         distributors: [distributor])
           }
 
-          it 'updates the product and removes the product from order cycles' do
-            product_set.save
+          context 'and only the name changes' do
+            let(:collection_hash) do
+              { 0 => { id: product.id, name: "New season product" } }
+            end
 
-            expect(product.reload.attributes).to include(
-              'supplier_id' => producer.id
-            )
-            expect(order_cycle.distributed_variants).to_not include product.variants.first
+            it 'updates the product and keeps it in order cycles' do
+              expect {
+                product_set.save
+                product.reload
+              }.to change { product.name }.to("New season product").
+                and change { order_cycle.distributed_variants.count }.by(0)
+
+              expect(order_cycle.distributed_variants).to include product.variants.first
+            end
+          end
+
+          context 'and a different supplier is passed' do
+            let(:collection_hash) do
+              { 0 => { id: product.id, supplier_id: producer.id } }
+            end
+
+            it 'updates the product and removes the product from order cycles' do
+              expect {
+                product_set.save
+                product.reload
+              }.to change { product.supplier }.to(producer).
+                and change { order_cycle.distributed_variants.count }.by(-1)
+
+              expect(order_cycle.distributed_variants).to_not include product.variants.first
+            end
           end
         end