From de915e8bd7accf340157dd145a5730a7eb394f66 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 2 Nov 2023 15:51:51 +1100
Subject: [PATCH] Capture variant errors when updating

---
 app/services/sets/product_set.rb              | 17 +++++++++++------
 spec/services/sets/product_set_spec.rb        |  4 +++-
 spec/system/admin/bulk_product_update_spec.rb |  4 ++--
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/app/services/sets/product_set.rb b/app/services/sets/product_set.rb
index 23be044239..5c3298e473 100644
--- a/app/services/sets/product_set.rb
+++ b/app/services/sets/product_set.rb
@@ -105,8 +105,15 @@ module Sets
       if variant.present?
         variant.update(variant_attributes.except(:id))
       else
-        create_variant(product, variant_attributes)
+        variant = create_variant(product, variant_attributes)
       end
+
+      # Copy any variant errors to product
+      variant&.errors&.each do |error|
+        # The name is namespaced to avoid confusion with product attrs of same name.
+        product.errors.add("variant_#{error.attribute}".to_sym, error.message)
+      end
+      variant&.errors.blank?
     end
 
     def create_variant(product, variant_attributes)
@@ -117,11 +124,7 @@ module Sets
       on_demand = variant_attributes.delete(:on_demand)
 
       variant = product.variants.create(variant_attributes)
-
-      if variant.errors.present?
-        product.errors.merge!(variant.errors)
-        return false
-      end
+      return variant if variant.errors.present?
 
       begin
         variant.on_demand = on_demand if on_demand.present?
@@ -130,6 +133,8 @@ module Sets
         notify_bugsnag(e, product, variant, variant_attributes)
         raise e
       end
+
+      variant
     end
 
     def notify_bugsnag(error, product, variant, variant_attributes)
diff --git a/spec/services/sets/product_set_spec.rb b/spec/services/sets/product_set_spec.rb
index cbb71f6271..5a57b8e472 100644
--- a/spec/services/sets/product_set_spec.rb
+++ b/spec/services/sets/product_set_spec.rb
@@ -154,6 +154,8 @@ describe Sets::ProductSet do
             product_set.save
             product.reload
           }.to_not change { product.sku }
+
+          expect(product_set.invalid.count).to be_positive
         end
 
         it "doesn't update variant" do
@@ -171,7 +173,7 @@ describe Sets::ProductSet do
         end
       end
 
-      xcontext "variant has error" do
+      context "variant has error" do
         let(:variant_attributes) { { sku: "var_sku", display_name: "A" * 256 } } # maximum length
 
         include_examples "nothing saved"
diff --git a/spec/system/admin/bulk_product_update_spec.rb b/spec/system/admin/bulk_product_update_spec.rb
index 9444cc87ab..199c8e218f 100644
--- a/spec/system/admin/bulk_product_update_spec.rb
+++ b/spec/system/admin/bulk_product_update_spec.rb
@@ -307,7 +307,7 @@ describe '
 
         click_button 'Save Changes', match: :first
         expect(page.find("#status-message"))
-          .to have_content "Unit value can't be blank Unit value is not a number"
+          .to have_content "Variant unit value can't be blank Variant unit value is not a number"
       end
 
       it "creating a variant with unit value is: '120g' and 'on_demand' checked" do
@@ -323,7 +323,7 @@ describe '
 
         click_button 'Save Changes', match: :first
         expect(page.find("#status-message"))
-          .to have_content "Unit value can't be blank Unit value is not a number"
+          .to have_content "Variant unit value can't be blank Variant unit value is not a number"
       end
     end
   end