From 5f248b2961a2f36ae32c7db859aaee167508660b Mon Sep 17 00:00:00 2001 From: Azeem Ahmed Date: Mon, 4 Oct 2021 12:44:13 +0200 Subject: [PATCH] Fix discarded duplicated products bug When a product is cloned and then discarded without changing the cloned variants SKU, if the product is ever cloned again then the cloning process will fail with a variant and master variant SKU validation error. This issue occurs because products and their variants are soft deleted and therefore the copied SKU will not be unique during the second cloning. The previous variant SKU uniqueness scope did not work for this specific case so this conditional was added on top of it. This commit also includes a validation spec for the variant SKU and a spec to catch the deleted duplication error. --- core/app/models/spree/variant.rb | 2 +- core/spec/models/spree/product_duplicator_spec.rb | 5 +++++ core/spec/models/spree/variant_spec.rb | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/core/app/models/spree/variant.rb b/core/app/models/spree/variant.rb index 17456a2061d..97408a1a1c8 100644 --- a/core/app/models/spree/variant.rb +++ b/core/app/models/spree/variant.rb @@ -66,7 +66,7 @@ class Variant < Spree::Base validates :cost_price, numericality: { greater_than_or_equal_to: 0, allow_nil: true } validates :price, numericality: { greater_than_or_equal_to: 0, allow_nil: true } - validates_uniqueness_of :sku, allow_blank: true, case_sensitive: true, if: :enforce_unique_sku? + validates_uniqueness_of :sku, allow_blank: true, case_sensitive: true, conditions: -> { where(deleted_at: nil) }, if: :enforce_unique_sku? after_create :create_stock_items after_create :set_position diff --git a/core/spec/models/spree/product_duplicator_spec.rb b/core/spec/models/spree/product_duplicator_spec.rb index 74d8d5605a0..7965b556b3c 100644 --- a/core/spec/models/spree/product_duplicator_spec.rb +++ b/core/spec/models/spree/product_duplicator_spec.rb @@ -87,6 +87,11 @@ module Spree it "will not duplicate the option values" do expect{ duplicator.duplicate }.to change{ Spree::OptionValue.count }.by(0) end + + it "will duplicate the variants after initial duplicate is discarded" do + duplicator.duplicate.discard + expect { duplicator.duplicate }.to change { Spree::Variant.count }.by(3) + end end end end diff --git a/core/spec/models/spree/variant_spec.rb b/core/spec/models/spree/variant_spec.rb index 49706375286..0885fc9d51b 100644 --- a/core/spec/models/spree/variant_spec.rb +++ b/core/spec/models/spree/variant_spec.rb @@ -40,6 +40,11 @@ variant.price = nil expect(variant).to be_invalid end + + it "should have a unique sku" do + variant_with_same_sku = build(:variant, sku: variant.sku) + expect(variant_with_same_sku).to be_invalid + end end context "after create" do