From 634e0295c83a72aac82408447e27ea990c26a0f2 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 22 Nov 2017 00:36:07 -0800 Subject: [PATCH 1/6] Add discard gem --- core/solidus_core.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index c836247df20..1da5a16df98 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -38,4 +38,5 @@ Gem::Specification.new do |s| s.add_dependency 'paranoia', '~> 2.4' s.add_dependency 'ransack', '~> 1.8' s.add_dependency 'state_machines-activerecord', '~> 0.4' + s.add_dependency 'discard' end From b7e756e7af556baba0a14743241b8e70744b43dd Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 23 Nov 2017 16:03:11 -0800 Subject: [PATCH 2/6] Call discard from Admin::ResrouceController --- backend/app/controllers/spree/admin/resource_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/app/controllers/spree/admin/resource_controller.rb b/backend/app/controllers/spree/admin/resource_controller.rb index 694766bf11e..6dbb02d1a72 100644 --- a/backend/app/controllers/spree/admin/resource_controller.rb +++ b/backend/app/controllers/spree/admin/resource_controller.rb @@ -87,7 +87,9 @@ def destroy invoke_callbacks(:destroy, :before) destroy_result = - if @object.respond_to?(:paranoia_destroy) + if @object.respond_to?(:discard) + @object.discard + elsif @object.respond_to?(:paranoia_destroy) @object.paranoia_destroy else @object.destroy From 42816ae55b9afe24bf45bb83c136ae89b2471de3 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 23 Nov 2017 17:11:30 -0800 Subject: [PATCH 3/6] Add Spree::ParanoiaDeprecations --- core/lib/spree/core.rb | 1 + core/lib/spree/paranoia_deprecations.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 core/lib/spree/paranoia_deprecations.rb diff --git a/core/lib/spree/core.rb b/core/lib/spree/core.rb index 6aa121a5482..6d148f10e2e 100644 --- a/core/lib/spree/core.rb +++ b/core/lib/spree/core.rb @@ -12,6 +12,7 @@ require 'state_machines-activerecord' require 'spree/deprecation' +require 'spree/paranoia_deprecations' # This is required because ActiveModel::Validations#invalid? conflicts with the # invalid state of a Payment. In the future this should be removed. diff --git a/core/lib/spree/paranoia_deprecations.rb b/core/lib/spree/paranoia_deprecations.rb new file mode 100644 index 00000000000..e2b8898d051 --- /dev/null +++ b/core/lib/spree/paranoia_deprecations.rb @@ -0,0 +1,19 @@ +module Spree + module ParanoiaDeprecations + def paranoia_destroy + Spree::Deprecation.warn <<-WARN.strip_heredoc, caller + Calling #destroy (or #paranoia_destroy) on a #{self.class} currently performs a soft-destroy using the paranoia gem. + In Solidus 3.0, paranoia will be removed, and this will perform a HARD destroy instead. To continue soft-deleting, use #discard instead. + WARN + super + end + + def paranoia_delete + Spree::Deprecation.warn <<-WARN.strip_heredoc, caller + Calling #delete (or #paranoia_delete) on a #{self.class} currently performs a soft-destroy using the paranoia gem. + In Solidus 3.0, paranoia will be removed, and this will perform a HARD destroy instead. To continue soft-deleting, use #discard instead. + WARN + super + end + end +end From 2d415ac627f9308e37845430f643412a21689a7f Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 22 Nov 2017 00:36:36 -0800 Subject: [PATCH 4/6] Convert product, variant, stock item to discard This deprecates the #paranoia_destroy (and therefore also #destroy) method on these models in favour of using #discard. after_discard callbacks have been added to these models to duplicate the current behaviour of calling #destroy (before_destroy callbacks and destroying of dependent: :destroy associations). --- .../controllers/spree/api/products_controller.rb | 2 +- .../spree/api/stock_items_controller.rb | 2 +- .../controllers/spree/api/variants_controller.rb | 2 +- .../spree/api/products_controller_spec.rb | 2 +- .../spree/api/shipments_controller_spec.rb | 2 +- .../spree/admin/products_controller.rb | 2 +- .../spree/admin/products_controller_spec.rb | 2 +- .../spree/admin/variants_controller_spec.rb | 6 +++--- core/app/models/spree/order.rb | 2 +- core/app/models/spree/product.rb | 15 +++++++++++++++ core/app/models/spree/stock_item.rb | 6 ++++++ core/app/models/spree/variant.rb | 16 +++++++++++++++- core/spec/lib/spree/core/importer/order_spec.rb | 2 +- core/spec/models/spree/customer_return_spec.rb | 2 +- core/spec/models/spree/inventory_unit_spec.rb | 6 +++--- core/spec/models/spree/line_item_spec.rb | 8 ++++---- core/spec/models/spree/order_spec.rb | 2 +- core/spec/models/spree/product_spec.rb | 16 ++++++++-------- core/spec/models/spree/shipment_spec.rb | 2 +- .../spec/models/spree/stock/availability_spec.rb | 4 ++-- core/spec/models/spree/stock_item_spec.rb | 8 ++++---- core/spec/models/spree/stock_location_spec.rb | 2 +- core/spec/models/spree/variant_spec.rb | 4 ++-- frontend/spec/features/caching/products_spec.rb | 8 ++++---- 24 files changed, 79 insertions(+), 44 deletions(-) diff --git a/api/app/controllers/spree/api/products_controller.rb b/api/app/controllers/spree/api/products_controller.rb index 0d21d4c65e5..b98db3f8ae5 100644 --- a/api/app/controllers/spree/api/products_controller.rb +++ b/api/app/controllers/spree/api/products_controller.rb @@ -91,7 +91,7 @@ def update def destroy @product = find_product(params[:id]) authorize! :destroy, @product - @product.paranoia_destroy + @product.discard respond_with(@product, status: 204) end diff --git a/api/app/controllers/spree/api/stock_items_controller.rb b/api/app/controllers/spree/api/stock_items_controller.rb index 7c838d8c870..06337dd7305 100644 --- a/api/app/controllers/spree/api/stock_items_controller.rb +++ b/api/app/controllers/spree/api/stock_items_controller.rb @@ -49,7 +49,7 @@ def update def destroy @stock_item = Spree::StockItem.accessible_by(current_ability, :destroy).find(params[:id]) - @stock_item.paranoia_destroy + @stock_item.discard respond_with(@stock_item, status: 204) end diff --git a/api/app/controllers/spree/api/variants_controller.rb b/api/app/controllers/spree/api/variants_controller.rb index 45c96a3004c..6a2e5c5b370 100644 --- a/api/app/controllers/spree/api/variants_controller.rb +++ b/api/app/controllers/spree/api/variants_controller.rb @@ -15,7 +15,7 @@ def create def destroy @variant = scope.accessible_by(current_ability, :destroy).find(params[:id]) - @variant.paranoia_destroy + @variant.discard respond_with(@variant, status: 204) end diff --git a/api/spec/requests/spree/api/products_controller_spec.rb b/api/spec/requests/spree/api/products_controller_spec.rb index b0a37155d9c..3317eb8fdaa 100644 --- a/api/spec/requests/spree/api/products_controller_spec.rb +++ b/api/spec/requests/spree/api/products_controller_spec.rb @@ -178,7 +178,7 @@ module Spree specify do get spree.api_product_path(product) expect(json_response["slug"]).to match(/and-1-ways/) - product.paranoia_destroy + product.discard get spree.api_product_path(other_product) expect(json_response["slug"]).to match(/droids/) diff --git a/api/spec/requests/spree/api/shipments_controller_spec.rb b/api/spec/requests/spree/api/shipments_controller_spec.rb index 09e4ae1ed48..4a36a6070d4 100644 --- a/api/spec/requests/spree/api/shipments_controller_spec.rb +++ b/api/spec/requests/spree/api/shipments_controller_spec.rb @@ -128,7 +128,7 @@ it 'removes a destroyed variant from a shipment' do order.contents.add(variant, 2) - variant.paranoia_destroy + variant.discard put spree.remove_api_shipment_path(shipment), params: { variant_id: variant.to_param, quantity: 1 } expect(response.status).to eq(200) diff --git a/backend/app/controllers/spree/admin/products_controller.rb b/backend/app/controllers/spree/admin/products_controller.rb index 49935b35ad8..5b02f822a66 100644 --- a/backend/app/controllers/spree/admin/products_controller.rb +++ b/backend/app/controllers/spree/admin/products_controller.rb @@ -47,7 +47,7 @@ def update def destroy @product = Spree::Product.friendly.find(params[:id]) - @product.paranoia_destroy! + @product.discard flash[:success] = t('spree.notice_messages.product_deleted') diff --git a/backend/spec/controllers/spree/admin/products_controller_spec.rb b/backend/spec/controllers/spree/admin/products_controller_spec.rb index c547114920f..8f2685c9d9c 100644 --- a/backend/spec/controllers/spree/admin/products_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/products_controller_spec.rb @@ -17,7 +17,7 @@ # Regression test for https://github.com/spree/spree/issues/1903 context 'when soft deleted products exist' do let!(:soft_deleted_product) { create(:product, sku: "ABC123") } - before { soft_deleted_product.paranoia_destroy } + before { soft_deleted_product.discard } context 'when params[:q][:with_deleted] is not set' do let(:params) { { q: {} } } diff --git a/backend/spec/controllers/spree/admin/variants_controller_spec.rb b/backend/spec/controllers/spree/admin/variants_controller_spec.rb index 7db6d6ebc81..61a95d9370c 100644 --- a/backend/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/variants_controller_spec.rb @@ -18,7 +18,7 @@ module Admin end context "with a deleted product" do - before { product.paranoia_destroy! } + before { product.discard } it "is the product" do subject @@ -31,8 +31,8 @@ module Admin let!(:variant) { create(:variant, product: product) } let!(:deleted_variant) { create(:variant, product: product) } - context "with deleted variants" do - before { deleted_variant.paranoia_destroy! } + context "with soft-deleted variants" do + before { deleted_variant.discard } context "when deleted is not requested" do it "excludes deleted variants" do diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 96486c522d7..cdc873ff1b9 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -467,7 +467,7 @@ def insufficient_stock_lines # Check to see if any line item variants are soft, deleted. # If so add error and restart checkout. def ensure_line_item_variants_are_not_deleted - if line_items.any? { |li| li.variant.paranoia_destroyed? } + if line_items.any? { |li| li.variant.discarded? } errors.add(:base, I18n.t('spree.deleted_variants_present')) restart_checkout_flow false diff --git a/core/app/models/spree/product.rb b/core/app/models/spree/product.rb index dd7b964d4b5..8fcedc991c5 100644 --- a/core/app/models/spree/product.rb +++ b/core/app/models/spree/product.rb @@ -1,3 +1,5 @@ +require 'discard' + module Spree # Products represent an entity for sale in a store. Products can have # variations, called variants. Product properties include description, @@ -12,6 +14,18 @@ class Product < Spree::Base friendly_id :slug_candidates, use: :history acts_as_paranoid + include Spree::ParanoiaDeprecations + + include Discard::Model + self.discard_column = :deleted_at + + after_discard do + variants_including_master.discard_all + self.product_option_types = [] + self.product_properties = [] + self.classifications = [] + self.product_promotion_rules = [] + end has_many :product_option_types, dependent: :destroy, inverse_of: :product has_many :option_types, through: :product_option_types @@ -90,6 +104,7 @@ def find_or_build_master after_create :build_variants_from_option_values_hash, if: :option_values_hash after_destroy :punch_slug + after_discard :punch_slug after_initialize :ensure_master diff --git a/core/app/models/spree/stock_item.rb b/core/app/models/spree/stock_item.rb index 1ceb7a543d1..3629f451182 100644 --- a/core/app/models/spree/stock_item.rb +++ b/core/app/models/spree/stock_item.rb @@ -1,6 +1,12 @@ +require 'discard' + module Spree class StockItem < Spree::Base acts_as_paranoid + include Spree::ParanoiaDeprecations + + include Discard::Model + self.discard_column = :deleted_at belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :stock_items belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant', inverse_of: :stock_items diff --git a/core/app/models/spree/variant.rb b/core/app/models/spree/variant.rb index 4454d8457be..3b8bd17f717 100644 --- a/core/app/models/spree/variant.rb +++ b/core/app/models/spree/variant.rb @@ -1,3 +1,5 @@ +require 'discard' + module Spree # == Master Variant # @@ -14,9 +16,21 @@ module Spree # option values and may have inventory units. Sum of on_hand each variant's # inventory level determine "on_hand" level for the product. class Variant < Spree::Base - acts_as_paranoid acts_as_list scope: :product + acts_as_paranoid + include Spree::ParanoiaDeprecations + + include Discard::Model + self.discard_column = :deleted_at + + after_discard do + stock_items.discard_all + images.destroy_all + prices.destroy_all + currently_valid_prices.destroy_all + end + attr_writer :rebuild_vat_prices include Spree::DefaultPrice diff --git a/core/spec/lib/spree/core/importer/order_spec.rb b/core/spec/lib/spree/core/importer/order_spec.rb index 6939c39be2c..6c87a61f112 100644 --- a/core/spec/lib/spree/core/importer/order_spec.rb +++ b/core/spec/lib/spree/core/importer/order_spec.rb @@ -242,7 +242,7 @@ module Core context 'variant was soft-deleted' do it 'raise error as variant shouldnt be found' do - variant.product.paranoia_destroy + variant.product.discard hash = { sku: variant.sku } expect { Importer::Order.ensure_variant_id_from_params(hash) diff --git a/core/spec/models/spree/customer_return_spec.rb b/core/spec/models/spree/customer_return_spec.rb index d46a1463537..24650ad52dc 100644 --- a/core/spec/models/spree/customer_return_spec.rb +++ b/core/spec/models/spree/customer_return_spec.rb @@ -218,7 +218,7 @@ end it "should NOT raise an error when a soft-deleted stock item exists in the stock location" do - inventory_unit.find_stock_item.paranoia_destroy + inventory_unit.find_stock_item.discard create(:customer_return_without_return_items, return_items: [return_item], stock_location_id: new_stock_location.id) end diff --git a/core/spec/models/spree/inventory_unit_spec.rb b/core/spec/models/spree/inventory_unit_spec.rb index de41ed67915..69e42f61e97 100644 --- a/core/spec/models/spree/inventory_unit_spec.rb +++ b/core/spec/models/spree/inventory_unit_spec.rb @@ -114,17 +114,17 @@ end end - context "variants deleted" do + context "variants discarded" do let!(:unit) { create(:inventory_unit) } it "can still fetch variant" do - unit.variant.destroy + unit.variant.discard expect(unit.reload.variant).to be_a Spree::Variant end it "can still fetch variants by eager loading (remove default_scope)" do skip "find a way to remove default scope when eager loading associations" - unit.variant.destroy + unit.variant.discard expect(Spree::InventoryUnit.joins(:variant).includes(:variant).first.variant).to be_a Spree::Variant end end diff --git a/core/spec/models/spree/line_item_spec.rb b/core/spec/models/spree/line_item_spec.rb index 46b112af601..5dae6722576 100644 --- a/core/spec/models/spree/line_item_spec.rb +++ b/core/spec/models/spree/line_item_spec.rb @@ -5,13 +5,13 @@ let(:line_item) { order.line_items.first } context '#destroy' do - it "fetches deleted products" do - line_item.product.paranoia_destroy + it "fetches soft-deleted products" do + line_item.product.discard expect(line_item.reload.product).to be_a Spree::Product end - it "fetches deleted variants" do - line_item.variant.paranoia_destroy + it "fetches soft-deleted variants" do + line_item.variant.discard expect(line_item.reload.variant).to be_a Spree::Variant end diff --git a/core/spec/models/spree/order_spec.rb b/core/spec/models/spree/order_spec.rb index a92e74d2f22..a7c54d7e428 100644 --- a/core/spec/models/spree/order_spec.rb +++ b/core/spec/models/spree/order_spec.rb @@ -157,7 +157,7 @@ context 'when variant is destroyed' do before do allow(order).to receive(:restart_checkout_flow) - order.line_items.first.variant.paranoia_destroy + order.line_items.first.variant.discard end it 'should restart checkout flow' do diff --git a/core/spec/models/spree/product_spec.rb b/core/spec/models/spree/product_spec.rb index 4a35dfbc7f3..47770d902a7 100644 --- a/core/spec/models/spree/product_spec.rb +++ b/core/spec/models/spree/product_spec.rb @@ -111,9 +111,9 @@ class Extension < Spree::Base end context "product has no variants" do - context "#destroy" do + context "#discard" do it "should set deleted_at value" do - product.paranoia_destroy + product.discard expect(product.deleted_at).not_to be_nil expect(product.master.reload.deleted_at).not_to be_nil end @@ -125,11 +125,11 @@ class Extension < Spree::Base create(:variant, product: product) end - context "#destroy" do + context "#discard" do it "should set deleted_at value" do - product.paranoia_destroy + product.discard expect(product.deleted_at).not_to be_nil - expect(product.variants_including_master).to all(be_paranoia_destroyed) + expect(product.variants_including_master).to all(be_discarded) end end end @@ -177,7 +177,7 @@ class Extension < Spree::Base end it "should not be available if soft-destroyed" do - product.paranoia_destroy + product.discard expect(product).not_to be_available end end @@ -292,7 +292,7 @@ class Extension < Spree::Base it "doesnt raise ReadOnlyRecord error" do Spree::StockMovement.create!(stock_item: stock_item, quantity: 1) - product.paranoia_destroy + product.discard end end @@ -314,7 +314,7 @@ class Extension < Spree::Base it "renames slug on destroy" do old_slug = product.slug - product.paranoia_destroy + product.discard expect(old_slug).to_not eq product.slug end diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index 472cfb06221..7704772b369 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -196,7 +196,7 @@ end context "variant was removed" do - before { variant.paranoia_destroy } + before { variant.discard } it "still returns variant expected" do expect(shipment.manifest.first.variant).to eq variant diff --git a/core/spec/models/spree/stock/availability_spec.rb b/core/spec/models/spree/stock/availability_spec.rb index a1057846f5b..c3ccef84908 100644 --- a/core/spec/models/spree/stock/availability_spec.rb +++ b/core/spec/models/spree/stock/availability_spec.rb @@ -59,7 +59,7 @@ module Spree::Stock end context 'with soft-deleted stock_item' do - before { stock_item.paranoia_destroy! } + before { stock_item.discard } it "returns empty hash" do expect(subject).to eq({}) @@ -125,7 +125,7 @@ module Spree::Stock end context 'with soft-deleted stock_item' do - before { stock_item.paranoia_destroy! } + before { stock_item.discard } it { is_expected.to eq({}) } end diff --git a/core/spec/models/spree/stock_item_spec.rb b/core/spec/models/spree/stock_item_spec.rb index a2144d47566..0ffeea7b3b2 100644 --- a/core/spec/models/spree/stock_item_spec.rb +++ b/core/spec/models/spree/stock_item_spec.rb @@ -149,12 +149,12 @@ before { Spree::StockMovement.create(stock_item: subject, quantity: 1) } it "doesnt raise ReadOnlyRecord error" do - subject.paranoia_destroy + subject.discard end end context "destroyed" do - before { subject.paranoia_destroy } + before { subject.discard } it "recreates stock item just fine" do stock_location.stock_items.create!(variant: subject.variant) @@ -277,8 +277,8 @@ # Regression test for https://github.com/spree/spree/issues/4651 context "variant" do - it "can be found even if the variant is deleted" do - subject.variant.paranoia_destroy + it "can be found even if the variant is soft-deleted" do + subject.variant.discard expect(subject.reload.variant).not_to be_nil end end diff --git a/core/spec/models/spree/stock_location_spec.rb b/core/spec/models/spree/stock_location_spec.rb index 60dc75d6ccb..1c94f79a114 100644 --- a/core/spec/models/spree/stock_location_spec.rb +++ b/core/spec/models/spree/stock_location_spec.rb @@ -226,7 +226,7 @@ module Spree it 'zero on_hand and backordered' do subject - variant.stock_items.each(&:paranoia_destroy!) + variant.stock_items.discard_all on_hand, backordered = subject.fill_status(variant, 1) expect(on_hand).to eq 0 expect(backordered).to eq 0 diff --git a/core/spec/models/spree/variant_spec.rb b/core/spec/models/spree/variant_spec.rb index 728c595cb5b..c599057d803 100644 --- a/core/spec/models/spree/variant_spec.rb +++ b/core/spec/models/spree/variant_spec.rb @@ -145,7 +145,7 @@ context "and a variant is soft-deleted" do let!(:old_options_text) { variant.options_text } - before { variant.paranoia_destroy! } + before { variant.discard } it "still keeps the option values for that variant" do expect(variant.reload.options_text).to eq(old_options_text) @@ -657,7 +657,7 @@ describe "deleted_at scope" do let!(:previous_variant_price) { variant.display_price } - before { variant.paranoia_destroy } + before { variant.discard } it "should keep its price if deleted" do expect(variant.display_price).to eq(previous_variant_price) diff --git a/frontend/spec/features/caching/products_spec.rb b/frontend/spec/features/caching/products_spec.rb index feaaf70fbf3..4ea5dfd41ee 100644 --- a/frontend/spec/features/caching/products_spec.rb +++ b/frontend/spec/features/caching/products_spec.rb @@ -31,22 +31,22 @@ end it "busts the cache when all products are soft-deleted" do - product.paranoia_destroy! - product2.paranoia_destroy! + product.discard + product2.discard visit spree.root_path assert_written_to_cache("views/en/USD/spree/products/all--#{Date.today.to_s(:number)}-0") expect(cache_writes.count).to eq(1) end it "busts the cache when the newest product is soft-deleted" do - product.paranoia_destroy! + product.discard visit spree.root_path assert_written_to_cache("views/en/USD/spree/products/all--#{product2.updated_at.utc.to_s(:number)}") expect(cache_writes.count).to eq(1) end it "busts the cache when an older product is soft-deleted" do - product2.paranoia_destroy! + product2.discard visit spree.root_path assert_written_to_cache("views/en/USD/spree/products/all--#{product.updated_at.utc.to_s(:number)}") expect(cache_writes.count).to eq(1) From b4d36e3e944ee75f343e162d65b56fd1f1de8576 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 23 Nov 2017 15:15:10 -0800 Subject: [PATCH 5/6] Migrate Prices to discard --- core/app/models/spree/price.rb | 6 ++++++ core/app/models/spree/variant.rb | 4 ++-- core/spec/models/spree/product/scopes_spec.rb | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/core/app/models/spree/price.rb b/core/app/models/spree/price.rb index c91104910d4..ba0d3a8ff94 100644 --- a/core/app/models/spree/price.rb +++ b/core/app/models/spree/price.rb @@ -1,6 +1,12 @@ +require 'discard' + module Spree class Price < Spree::Base acts_as_paranoid + include Spree::ParanoiaDeprecations + + include Discard::Model + self.discard_column = :deleted_at MAXIMUM_AMOUNT = BigDecimal('99_999_999.99') diff --git a/core/app/models/spree/variant.rb b/core/app/models/spree/variant.rb index 3b8bd17f717..5a10249d66a 100644 --- a/core/app/models/spree/variant.rb +++ b/core/app/models/spree/variant.rb @@ -27,8 +27,8 @@ class Variant < Spree::Base after_discard do stock_items.discard_all images.destroy_all - prices.destroy_all - currently_valid_prices.destroy_all + prices.discard_all + currently_valid_prices.discard_all end attr_writer :rebuild_vat_prices diff --git a/core/spec/models/spree/product/scopes_spec.rb b/core/spec/models/spree/product/scopes_spec.rb index 0380e4f1e40..9e0c0f63fbd 100644 --- a/core/spec/models/spree/product/scopes_spec.rb +++ b/core/spec/models/spree/product/scopes_spec.rb @@ -131,7 +131,7 @@ end context "with soft-deleted master price" do - before { product.master.prices.each(&:paranoia_destroy!) } + before { product.master.prices.discard_all } it "doesn't include the product" do expect(Spree::Product.available).to match_array([]) From b6e8236dd934436471030b1d33e1ca25a7b185f9 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 23 Nov 2017 16:22:29 -0800 Subject: [PATCH 6/6] Add specs for discarding variants and products --- core/spec/models/spree/product_spec.rb | 40 ++++++++++++++++++++++++ core/spec/models/spree/variant_spec.rb | 42 ++++++++++++++++++++------ 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/core/spec/models/spree/product_spec.rb b/core/spec/models/spree/product_spec.rb index 47770d902a7..c53ce133abf 100644 --- a/core/spec/models/spree/product_spec.rb +++ b/core/spec/models/spree/product_spec.rb @@ -341,6 +341,46 @@ class Extension < Spree::Base end end + describe "#discard" do + let(:product) { create(:product, slug: 'my-awesome-product') } + + it "destroys related associations" do + create(:variant, product: product) + product.option_types = [create(:option_type)] + product.master.images = [create(:image)] + product.taxons = [create(:taxon)] + product.properties = [create(:property)] + + product.discard + + product.reload + expect(product.option_types).to be_empty + expect(product.images).to be_empty + expect(product.taxons).to be_empty + expect(product.properties).to be_empty + end + + it "removes from product promotion rules" do + promotion = create(:promotion) + rule = promotion.rules.create!(type: 'Spree::Promotion::Rules::Product', products: [product]) + + product.discard + + rule.reload + expect(rule.products).to be_empty + end + + it "replaces the slug" do + product.discard + + expect(product.slug).to match /\A\d+_my-awesome-product\z/ + + # Ensure a new product can be created with the slug + new_product = create(:product, slug: 'my-awesome-product') + expect(new_product.slug).to eq('my-awesome-product') + end + end + context "associations" do describe "product_option_types" do context "with no existing option types" do diff --git a/core/spec/models/spree/variant_spec.rb b/core/spec/models/spree/variant_spec.rb index c599057d803..43c0db71557 100644 --- a/core/spec/models/spree/variant_spec.rb +++ b/core/spec/models/spree/variant_spec.rb @@ -654,19 +654,43 @@ end end - describe "deleted_at scope" do - let!(:previous_variant_price) { variant.display_price } + describe "#discard" do + it "discards related associations" do + variant.images = [create(:image)] - before { variant.discard } + expect(variant.stock_items).not_to be_empty + expect(variant.prices).not_to be_empty + expect(variant.currently_valid_prices).not_to be_empty - it "should keep its price if deleted" do - expect(variant.display_price).to eq(previous_variant_price) + variant.discard + + expect(variant.images).to be_empty + expect(variant.stock_items).to be_empty + expect(variant.prices).to be_empty + expect(variant.currently_valid_prices).to be_empty end - context 'when loading with pre-fetching of default_price' do - it 'also keeps the previous price' do - reloaded_variant = Spree::Variant.with_deleted.includes(:default_price).find_by(id: variant.id) - expect(reloaded_variant.display_price).to eq(previous_variant_price) + describe 'default_price' do + let!(:previous_variant_price) { variant.display_price } + + it "should discard default_price" do + variant.discard + variant.reload + expect(variant.default_price).to be_discarded + end + + it "should keep its price if deleted" do + variant.discard + variant.reload + expect(variant.display_price).to eq(previous_variant_price) + end + + context 'when loading with pre-fetching of default_price' do + it 'also keeps the previous price' do + variant.discard + reloaded_variant = Spree::Variant.with_deleted.includes(:default_price).find_by(id: variant.id) + expect(reloaded_variant.display_price).to eq(previous_variant_price) + end end end end