Make price_selector return configurable

The current price_for method ignores its implied return value and serves
up a Spree::Money object rather than a Spree::Price.

The Spree::Price model is useful in its ability to be extended to
support sales_prices, partial prices, etc. Having this method return a
Spree::Money object is limiting, and the .money call is easy enough to
be left up to the end user.

This modifies the api of the #price_for method to optionally recieve a
return_price flag. This allows us to deprecate the current behaviour and
let users modify their code to call .money explicitly.

If desired, this can be set to always return a Spree::Price globally via
the Spree::Config.price_selector_returns_price flag.

core/app/models/spree/variant/price_selector.rb

@@ -24,13 +24,28 @@ def initialize(variant)
 
       # The variant's price, given a set of pricing options
       # @param [Spree::Variant::PricingOptions] price_options Pricing Options to abide by
-      # @return [Spree::Money, nil] The most specific price for this set of pricing options.
-      def price_for(price_options)
-        variant.currently_valid_prices.detect do |price|
+      # @param [Boolean] return_price Flag to return a Spree::Price instead of
+      #   a Spree::Money
+      # @return [Spree::Price, Spree::Money, nil] The most specific price for
+      #   this set of pricing options in the desired format.
+      def price_for(price_options, return_price: false)
+        spree_price = variant.currently_valid_prices.detect do |price|
           ( price.country_iso == price_options.desired_attributes[:country_iso] ||
             price.country_iso.nil?
           ) && price.currency == price_options.desired_attributes[:currency]
-        end.try!(:money)
+        end
+
+        return_price = Spree::Config.price_selector_returns_price || return_price
+        return spree_price if return_price
+
+        Spree::Deprecation.warn(
+          'price_for will stop returning a Spree::Money, please change your
+          code so that it accepts a Spree::Price instead. When done, you can
+          suppress this message by passing `return_price: true` to this method
+          or setting `Spree::Config.price_selector_returns_price = true`
+          globally.'
+        )
+        spree_price&.money
       end
     end
   end

core/spec/models/spree/variant/price_selector_spec.rb

@@ -19,71 +19,101 @@
   describe "#price_for(options)" do
     let(:variant) { create(:variant, price: 12.34) }
 
-    context "with the default currency" do
-      let(:pricing_options) { described_class.pricing_options_class.new(currency: "USD") }
-
-      it "returns the correct (default) price as a Spree::Money object" do
-        expect(subject.price_for(pricing_options)).to eq(Spree::Money.new(12.34, currency: "USD"))
-      end
+    context "when the return_price flag is not passed in" do
+      subject { described_class.new(variant).price_for(pricing_options) }
 
-      context "with the another country iso" do
-        let(:country) { create(:country, iso: "DE") }
+      context "with the default currency" do
+        let(:pricing_options) { described_class.pricing_options_class.new(currency: "USD") }
 
-        let(:pricing_options) do
-          described_class.pricing_options_class.new(currency: "USD", country_iso: "DE")
+        it "returns the correct (default) price as a Spree::Money object" do
+          expect(subject).to eq(Spree::Money.new(12.34, currency: "USD"))
         end
 
-        context "with a price for that country present" do
-          before do
-            variant.prices.create(amount: 44.44, country: country, currency: Spree::Config.currency)
-          end
+        context "with the another country iso" do
+          let(:country) { create(:country, iso: "DE") }
 
-          it "returns the correct price" do
-            expect(subject.price_for(pricing_options)).to eq(Spree::Money.new(44.44, currency: "USD"))
+          let(:pricing_options) do
+            described_class.pricing_options_class.new(currency: "USD", country_iso: "DE")
           end
-        end
 
-        context "with no price for that country present" do
-          context "and no fallback price for the variant present" do
+          context "with a price for that country present" do
             before do
-              variant.prices.delete_all
+              variant.prices.create(amount: 44.44, country: country, currency: Spree::Config.currency)
             end
 
-            it "returns nil" do
-              expect(subject.price_for(pricing_options)).to be_nil
+            it "returns the correct price" do
+              expect(subject).to eq(Spree::Money.new(44.44, currency: "USD"))
             end
           end
 
-          context "and a fallback price for the variant present" do
-            before do
-              variant.prices.create(amount: 55.44, country: nil, currency: Spree::Config.currency)
+          context "with no price for that country present" do
+            context "and no fallback price for the variant present" do
+              before do
+                variant.prices.delete_all
+              end
+
+              it "returns nil" do
+                expect(subject).to be_nil
+              end
             end
 
-            it "returns the fallback price" do
-              expect(subject.price_for(pricing_options)).to eq(Spree::Money.new(55.44, currency: "USD"))
+            context "and a fallback price for the variant present" do
+              before do
+                variant.prices.create(amount: 55.44, country: nil, currency: Spree::Config.currency)
+              end
+
+              it "returns the fallback price" do
+                expect(subject).to eq(Spree::Money.new(55.44, currency: "USD"))
+              end
             end
           end
         end
       end
-    end
 
-    context "with a different currency" do
-      let(:pricing_options) { described_class.pricing_options_class.new(currency: "EUR") }
+      context "with a different currency" do
+        let(:pricing_options) { described_class.pricing_options_class.new(currency: "EUR") }
 
-      context "when there is a price for that currency" do
-        before do
-          variant.prices.create(amount: 99.00, currency: "EUR")
+        context "when there is a price for that currency" do
+          before do
+            variant.prices.create(amount: 99.00, currency: "EUR")
+          end
+
+          it "returns the price in the correct currency as a Spree::Money object" do
+            expect(subject).to eq(Spree::Money.new(99.00, currency: "EUR"))
+          end
         end
 
-        it "returns the price in the correct currency as a Spree::Money object" do
-          expect(subject.price_for(pricing_options)).to eq(Spree::Money.new(99.00, currency: "EUR"))
+        context "when there is no price for that currency" do
+          it "returns nil" do
+            expect(subject).to be_nil
+          end
         end
       end
+    end
 
-      context "when there is no price for that currency" do
-        it "returns nil" do
-          expect(subject.price_for(pricing_options)).to be_nil
-        end
+    context "when the return_price flag is set to true" do
+      subject { described_class.new(variant).price_for(pricing_options, return_price: true) }
+
+      let(:pricing_options) { described_class.pricing_options_class.new(currency: "USD") }
+
+      it "returns the correct price as a Spree::Price object", :aggregate_failures do
+        expect(subject).to be_a Spree::Price
+        expect(subject.amount).to eq 12.34
+        expect(subject.currency).to eq "USD"
+      end
+    end
+
+    context "when the store is configured globally to return a Spree::Price" do
+      subject { described_class.new(variant).price_for(pricing_options) }
+
+      let(:pricing_options) { described_class.pricing_options_class.new(currency: "USD") }
+
+      before { allow(Spree::Config).to receive(:price_selector_returns_price).and_return(true) }
+
+      it "returns the correct price as a Spree::Price object", :aggregate_failures do
+        expect(subject).to be_a Spree::Price
+        expect(subject.amount).to eq 12.34
+        expect(subject.currency).to eq "USD"
       end
     end
   end