Skip to content

Commit

Permalink
Fix discarded duplicated products bug
Browse files Browse the repository at this point in the history
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.

This commit also includes a spec to catch this error in the future.
  • Loading branch information
Azeem838 committed Oct 4, 2021
1 parent cc54fee commit 3d97f44
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 deletions.
6 changes: 4 additions & 2 deletions core/lib/spree/core/product_duplicator.rb
Expand Up @@ -41,8 +41,9 @@ def duplicate_product

def duplicate_master
master = product.master
discarded_count = Spree::Product.with_discarded.where("name like ?", "COPY OF #{product.name}").count + 1
master.dup.tap do |new_master|
new_master.sku = "COPY OF #{master.sku}"
new_master.sku = "#{discarded_count.ordinalize} COPY OF #{new_master.sku}"
new_master.deleted_at = nil
new_master.images = master.images.map { |image| duplicate_image image } if @include_images
new_master.price = master.price
Expand All @@ -51,7 +52,8 @@ def duplicate_master

def duplicate_variant(variant)
new_variant = variant.dup
new_variant.sku = "COPY OF #{new_variant.sku}"
discarded_count = Spree::Product.with_discarded.where("name like ?", "COPY OF #{product.name}").count + 1
new_variant.sku = "#{discarded_count.ordinalize} COPY OF #{new_variant.sku}"
new_variant.deleted_at = nil
new_variant.option_values = variant.option_values.map { |option_value| option_value }
new_variant
Expand Down
5 changes: 5 additions & 0 deletions core/spec/models/spree/product_duplicator_spec.rb
Expand Up @@ -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
2 changes: 1 addition & 1 deletion core/spec/models/spree/product_spec.rb
Expand Up @@ -22,7 +22,7 @@ class Extension < Spree::Base
it 'duplicates product' do
clone = product.duplicate
expect(clone.name).to eq('COPY OF ' + product.name)
expect(clone.master.sku).to eq('COPY OF ' + product.master.sku)
expect(clone.master.sku).to eq('1st COPY OF ' + product.master.sku)
expect(clone.taxons).to eq(product.taxons)
expect(clone.images.size).to eq(product.images.size)
end
Expand Down

0 comments on commit 3d97f44

Please sign in to comment.