Skip to content

Commit

Permalink
Add custom price_for_options method to variant
Browse files Browse the repository at this point in the history
We need to safely ensure the new price_for_options method in the price
selector will not break for those that have a custom price selector.

If the new method is defined in the price selector class this will
behave normally, and if their customization does not allow for it, we
will instead return an unpersisted Spree::Price from the Spree::Money
object that the deprecated price_for method returns.

Spree::Product can now delegate price_for_options to master variant
  • Loading branch information
Mike Conlin committed Mar 25, 2021
1 parent 260ef49 commit 9321d04
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
1 change: 1 addition & 0 deletions core/app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ def find_or_build_master
:has_default_price?,
:images,
:price_for,
:price_for_options,
:rebuild_vat_prices=,
to: :find_or_build_master

Expand Down
14 changes: 13 additions & 1 deletion core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,9 @@ def price_selector
end

# Chooses an appropriate price for the given pricing options
# This has been deprecated in favor of #price_for_options.
#
# @see Spree::Variant::PriceSelector#price_for
# @see Spree::Variant::PriceSelector#price_for_options
delegate :price_for, to: :price_selector

# Returns the difference in price from the master variant
Expand All @@ -293,6 +294,17 @@ def price_same_as_master?(pricing_options = Spree::Config.default_pricing_option
diff && diff.zero?
end

def price_for_options(price_options)
if price_selector.respond_to?(:price_for_options)
price_selector.price_for_options(price_options)
else
money = price_for(price_options)
return if money.nil?

Spree::Price.new(amount: money.to_d, variant: self, currency: price_options.currency)
end
end

# Generates a friendly name and sku string.
#
# @return [String]
Expand Down
33 changes: 33 additions & 0 deletions core/spec/models/spree/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,39 @@
expect(variant.price_selector).to receive(:price_for).with(price_options)
variant.price_for(price_options)
end

it "returns a Spree::Money object" do
expect(variant.price_for(price_options)).to eq Spree::Money.new(19.99)
end
end

context "#price_for_options(price_options)" do
subject { variant.price_for_options(price_options) }

let(:price_options) { Spree::Config.variant_price_selector_class.pricing_options_class.new }

it "delegates to the price_selector" do
expect(variant.price_selector).to receive(:price_for_options).with(price_options)
subject
end

it "returns a Spree::Price object for the given pricing_options", :aggregate_failures do
expect(subject).to be_a Spree::Price
expect(subject.amount).to eq 19.99
end

context "when the price_selector does not implement #price_for_options" do
before do
allow(variant.price_selector).to receive(:respond_to?).with(:price_for_options).and_return false
end

it "returns an unpersisted Spree::Price", :aggregate_failures do
expect(Spree::Deprecation).to receive(:warn).
with(/^price_for is deprecated and will be removed/, any_args)
expect(subject).to be_a Spree::Price
expect(subject.amount).to eq 19.99
end
end
end

context "#price_difference_from_master" do
Expand Down

0 comments on commit 9321d04

Please sign in to comment.