Skip to content

Commit

Permalink
Maintain prices for previously soft-deleted variants
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mamhoff committed Oct 19, 2022
1 parent faeba54 commit a920d3f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
1 change: 1 addition & 0 deletions core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion core/app/models/spree/variant/price_selector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ def price_for_options(price_options)
#
# @return [Array<Spree::Price>]
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,
Expand Down
16 changes: 16 additions & 0 deletions core/spec/models/spree/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a920d3f

Please sign in to comment.