From 163c65849ef90d69e25feb0a1ed5f2e25dbf5005 Mon Sep 17 00:00:00 2001
From: luisramos0 <luisramos0@gmail.com>
Date: Thu, 15 Aug 2019 18:52:16 +0100
Subject: [PATCH] Make product set a bit more robust by not failing to update
 on_hand when variant is not valid. This will make the overall set update work

---
 app/models/spree/product_set.rb       |  1 +
 spec/models/spree/product_set_spec.rb | 29 ++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/app/models/spree/product_set.rb b/app/models/spree/product_set.rb
index 8e23741775..84f1460621 100644
--- a/app/models/spree/product_set.rb
+++ b/app/models/spree/product_set.rb
@@ -89,6 +89,7 @@ class Spree::ProductSet < ModelSet
     on_demand = variant_attributes.delete(:on_demand)
 
     variant = product.variants.create(variant_attributes)
+    return unless variant.valid?
 
     variant.on_demand = on_demand if on_demand.present?
     variant.on_hand = on_hand.to_i if on_hand.present?
diff --git a/spec/models/spree/product_set_spec.rb b/spec/models/spree/product_set_spec.rb
index 2b4dd083ac..99aa494793 100644
--- a/spec/models/spree/product_set_spec.rb
+++ b/spec/models/spree/product_set_spec.rb
@@ -96,13 +96,32 @@ describe Spree::ProductSet do
           end
 
           context 'and the variant does not exist' do
-            let(:master_attributes) do
-              attributes_for(:variant).merge(sku: '123')
+            context 'and attributes provided are valid' do
+              let(:master_attributes) do
+                attributes_for(:variant).merge(sku: '123')
+              end
+
+              it 'creates it with the specified attributes' do
+                number_of_variants = Spree::Variant.all.size
+                product_set.save
+                expect(Spree::Variant.last.sku).to eq('123')
+                expect(Spree::Variant.all.size).to eq number_of_variants + 1
+              end
             end
 
-            it 'creates it with the specified attributes' do
-              product_set.save
-              expect(Spree::Variant.last.sku).to eq('123')
+            context 'and attributes provided are not valid' do
+              let(:master_attributes) do
+                # unit_value nil makes the variant invalid
+                # on_hand with a value would make on_hand be updated and fail with exception (no stock item)
+                attributes_for(:variant).merge(unit_value: nil, on_hand: 1)
+              end
+
+              it 'does not create variant and does not raise exception' do
+                number_of_variants = Spree::Variant.all.size
+                expect { product_set.save }
+                  .not_to raise_error(StandardError)
+                expect(Spree::Variant.all.size).to eq number_of_variants
+              end
             end
           end
         end