Add price_for_options and deprecate price_for

The price_for method does not return a Spree::Price as described.
Instead it does some frontend work and returns a Spree::Money, limiting
the usability of this method, as the price record itself can have useful
information on it.

This deprecates the existing method in favor of one that returns a
Spree::Price.

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

@@ -26,11 +26,24 @@ def initialize(variant)
       # @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)
+        Spree::Deprecation.warn(
+          "price_for is deprecated and will be removed. The price_for method
+          should return a Spree::Price as described. Please use
+          #price_for_options and adjust your frontend code to explicitly call
+          &.money where required"
+        )
+        price_for_options(price_options)&.money
+      end
+
+      # The variant's Spree::Price record, given a set of pricing options
+      # @param [Spree::Variant::PricingOptions] price_options Pricing Options to abide by
+      # @return [Spree::Price, nil] The most specific price for this set of pricing options.
+      def price_for_options(price_options)
         variant.currently_valid_prices.detect do |price|
-          ( price.country_iso == price_options.desired_attributes[:country_iso] ||
-            price.country_iso.nil?
+          (price.country_iso == price_options.desired_attributes[:country_iso] ||
+           price.country_iso.nil?
           ) && price.currency == price_options.desired_attributes[:currency]
-        end.try!(:money)
+        end
       end
     end
   end

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

@@ -9,6 +9,7 @@
 
   it { is_expected.to respond_to(:variant) }
   it { is_expected.to respond_to(:price_for) }
+  it { is_expected.to respond_to(:price_for_options) }
 
   describe ".pricing_options_class" do
     it "returns the standard pricing options class" do
@@ -18,12 +19,32 @@
 
   describe "#price_for(options)" do
     let(:variant) { create(:variant, price: 12.34) }
+    let(:pricing_options) { described_class.pricing_options_class.new(currency: "USD") }
+
+    it "is deprecated" do
+      expect(Spree::Deprecation).to receive(:warn).
+        with(/^price_for is deprecated and will be removed/, any_args)
+      subject.price_for(pricing_options)
+    end
+
+    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
+  end
+
+  describe "#price_for_options(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"))
+      it "returns the price as a Spree::Price object" do
+        expect(subject.price_for_options(pricing_options)).to be_a Spree::Price
+      end
+
+      it "returns the correct (default) price", :aggregate_failures do
+        expect(subject.price_for_options(pricing_options).amount).to eq 12.34
+        expect(subject.price_for_options(pricing_options).currency).to eq "USD"
       end
 
       context "with the another country iso" do
@@ -38,8 +59,9 @@
             variant.prices.create(amount: 44.44, country: country, currency: Spree::Config.currency)
           end
 
-          it "returns the correct price" do
-            expect(subject.price_for(pricing_options)).to eq(Spree::Money.new(44.44, currency: "USD"))
+          it "returns the correct price and currency", :aggregate_failures do
+            expect(subject.price_for_options(pricing_options).amount).to eq 44.44
+            expect(subject.price_for_options(pricing_options).currency).to eq "USD"
           end
         end
 
@@ -50,7 +72,7 @@
             end
 
             it "returns nil" do
-              expect(subject.price_for(pricing_options)).to be_nil
+              expect(subject.price_for_options(pricing_options)).to be_nil
             end
           end
 
@@ -60,7 +82,7 @@
             end
 
             it "returns the fallback price" do
-              expect(subject.price_for(pricing_options)).to eq(Spree::Money.new(55.44, currency: "USD"))
+              expect(subject.price_for_options(pricing_options).amount).to eq 55.44
             end
           end
         end
@@ -75,14 +97,15 @@
           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.price_for(pricing_options)).to eq(Spree::Money.new(99.00, currency: "EUR"))
+        it "returns the price in the correct currency", :aggregate_failures do
+          expect(subject.price_for_options(pricing_options).amount).to eq 99.00
+          expect(subject.price_for_options(pricing_options).currency).to eq "EUR"
         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
+          expect(subject.price_for_options(pricing_options)).to be_nil
         end
       end
     end