From 16e4b469a86697dc393311e4b48fc365757d04c3 Mon Sep 17 00:00:00 2001 From: Martin Meyerhoff Date: Thu, 6 Oct 2022 09:43:49 +0200 Subject: [PATCH] Maintain prices for previously soft-deleted variants Previous Solidus behavior included soft-deleted prices when looking at the default price. The intention of this behavior was that a a variant that is soft-deleted, and has its prices soft-deleted as a consequence, still has a `default_price` record. However, for a `kept` variant, we do not want to see a discarded `default_price` - otherwise what sense does it make to even add the delete action to the price? Especially given that deletion touches the variant and will then move it forward in the array of sorted prices to be considered for a given set of pricing attributes. This commit changes the `prices` association to include discarded prices, but then filters out discarded prices for non-discarded variants in the price selector. (cherry picked from commit 3f4578aba47fbfdebcba7cbdcb84507f142ae93d) --- core/app/models/spree/variant.rb | 1 + core/app/models/spree/variant/price_selector.rb | 4 +++- core/spec/models/spree/variant_spec.rb | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/core/app/models/spree/variant.rb b/core/app/models/spree/variant.rb index f15baebd2f9..c0dfd03dc6e 100644 --- a/core/app/models/spree/variant.rb +++ b/core/app/models/spree/variant.rb @@ -51,6 +51,7 @@ class Variant < Spree::Base has_many :images, -> { order(:position) }, as: :viewable, dependent: :destroy, class_name: "Spree::Image" has_many :prices, + -> { with_discarded }, class_name: 'Spree::Price', dependent: :destroy, inverse_of: :variant, diff --git a/core/app/models/spree/variant/price_selector.rb b/core/app/models/spree/variant/price_selector.rb index 2cc76dad715..cb9566f2e86 100644 --- a/core/app/models/spree/variant/price_selector.rb +++ b/core/app/models/spree/variant/price_selector.rb @@ -52,7 +52,9 @@ def price_for_options(price_options) # # @return [Array] def sorted_prices_for(variant) - variant.prices.sort_by do |price| + variant.prices.select do |price| + variant.discarded? || price.kept? + end.sort_by do |price| [ price.country_iso.nil? ? 0 : 1, price.updated_at || Time.zone.now, diff --git a/core/spec/models/spree/variant_spec.rb b/core/spec/models/spree/variant_spec.rb index 36fe926d2df..5517da5839d 100644 --- a/core/spec/models/spree/variant_spec.rb +++ b/core/spec/models/spree/variant_spec.rb @@ -261,6 +261,22 @@ expect(variant.default_price.attributes).to eq(price.attributes) end + + context "when the variant and the price are both soft-deleted" do + it "will use a deleted price as the default price" do + variant = create(:variant, deleted_at: 1.day.ago) + variant.prices.each { |price| price.discard } + expect(variant.reload.price).to be_present + end + end + + context "when the variant is not soft-deleted, but its price is" do + it "will not use a deleted price as the default price" do + variant = create(:variant) + variant.prices.each { |price| price.discard } + expect(variant.reload.price).not_to be_present + end + end end describe '#default_price_or_build' do