Fix money deprecation warning.

CHANGELOG.md

@@ -4,6 +4,9 @@
 
 - Implement stock location sorters [#2783](https://github.com/solidusio/solidus/pull/2783) ([aldesantis](https://github.com/aldesantis))
 
+- Fix multiple Money deprecation warnings. Now using html_wrap option which causes each piece of the price to be wrapped in span tags with specific classes for easier styling, but this may break existing stores custom styles.
+[#2912](https://github.com/solidusio/solidus/pull/2912) ([JDutil](https://github.com/JDutil))
+
 ## Solidus 2.7.0 (2018-09-14)
 
 ### Major Changes

backend/spec/features/admin/users_spec.rb

@@ -16,6 +16,7 @@
     create(:completed_order_with_totals, user: user_a, number: "R456").tap do |o|
       li = o.line_items.last
       li.update_column(:price, li.price + 10)
+      o.recalculate
     end
   end
 
@@ -302,14 +303,24 @@ def always_invalid_email
       end
     end
 
-    [:number, :total].each do |attr|
-      context attr do
-        it_behaves_like "a sortable attribute" do
-          let(:text_match_1) { order.send(attr).to_s }
-          let(:text_match_2) { order_2.send(attr).to_s }
-          let(:table_id) { "listing_orders" }
-          let(:sort_link) { "orders_#{attr}_title" }
-        end
+    context :number do
+      it_behaves_like "a sortable attribute" do
+        let(:text_match_1) { order.number }
+        let(:text_match_2) { order_2.number }
+        let(:table_id) { "listing_orders" }
+        let(:sort_link) { "orders_number_title" }
+      end
+    end
+
+    context :total do
+      it_behaves_like "a sortable attribute" do
+        # Since Spree::Money renders each piece of the total in it's own span,
+        # we are just checking the dollar total matches. Mainly due to how
+        # RSpec matcher appear_before works since it can't index the broken up total.
+        let(:text_match_1) { order.total.to_i.to_s }
+        let(:text_match_2) { order_2.total.to_i.to_s }
+        let(:table_id) { "listing_orders" }
+        let(:sort_link) { "orders_total_title" }
       end
     end
 
@@ -359,9 +370,9 @@ def always_invalid_email
       within_table('listing_items') do
         items.each do |item|
           expect(page).to have_selector(".item-name", text: item.product.name)
-          expect(page).to have_selector(".item-price", text: item.single_money.to_html)
+          expect(page).to have_selector(".item-price", text: item.single_money.to_html(html_wrap: false))
           expect(page).to have_selector(".item-quantity", text: item.quantity)
-          expect(page).to have_selector(".item-total", text: item.money.to_html)
+          expect(page).to have_selector(".item-total", text: item.money.to_html(html_wrap: false))
         end
       end
     end

core/config/initializers/money.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+require 'money'
+
+Money.locale_backend = :i18n

core/lib/spree/money.rb

@@ -1,9 +1,5 @@
 # frozen_string_literal: true
 
-require 'money'
-require 'monetize'
-require 'active_support/core_ext/string/output_safety'
-
 module Spree
   # Spree::Money is a relatively thin wrapper around Monetize which handles
   # formatting via Spree::Config.
@@ -77,16 +73,22 @@ def format(options = {})
       @money.format(@options.merge(options))
     end
 
-    # @note If you pass in options, ensure you pass in the html: true as well.
+    # @note If you pass in options, ensure you pass in the { html_wrap: true } as well.
     # @param options [Hash] additional formatting options
     # @return [String] the value of this money object formatted according to
-    #   its options and any additional options, by default as html.
-    def to_html(options = { html: true })
+    #   its options and any additional options, by default with html_wrap.
+    def to_html(options = { html_wrap: true })
       output = format(options)
-      if options[:html]
-        # 1) prevent blank, breaking spaces
-        # 2) prevent escaping of HTML character entities
-        output = output.sub(" ", " ").html_safe
+      # Maintain compatibility by checking html option renamed to html_wrap.
+      if options[:html] || options[:html] == false
+        Spree::Deprecation.warn <<-WARN.squish, caller
+          Spree::Money#to_html called with Spree::Money#to_html(html: #{options[:html]}),
+          which will not be supported in the future.
+          Instead use :html_wrap e.g. Spree::Money#to_html(html_wrap: #{options[:html]})
+        WARN
+      end
+      if options[:html_wrap] || options[:html]
+        output = output.html_safe
       end
       output
     end

core/spec/helpers/products_helper_spec.rb

@@ -31,15 +31,15 @@ module Spree
         context "when variant is more than master" do
           let(:variant_price) { 15 }
 
-          it { is_expected.to eq("(Add: $5.00)") }
+          it { is_expected.to eq("(Add: <span class=\"money-currency-symbol\">$</span><span class=\"money-whole\">5</span><span class=\"money-decimal-mark\">.</span><span class=\"money-decimal\">00</span>)") }
           # Regression test for https://github.com/spree/spree/issues/2737
           it { is_expected.to be_html_safe }
         end
 
         context "when variant is less than master" do
           let(:product_price) { 15 }
 
-          it { is_expected.to eq("(Subtract: $5.00)") }
+          it { is_expected.to eq("(Subtract: <span class=\"money-currency-symbol\">$</span><span class=\"money-whole\">5</span><span class=\"money-decimal-mark\">.</span><span class=\"money-decimal\">00</span>)") }
         end
       end
 
@@ -56,13 +56,13 @@ module Spree
         context "when variant is more than master" do
           let(:variant_price) { 150 }
 
-          it { is_expected.to eq("(Add: &#x00A5;50)") }
+          it { is_expected.to eq("(Add: <span class=\"money-currency-symbol\">&#x00A5;</span><span class=\"money-whole\">50</span>)") }
         end
 
         context "when variant is less than master" do
           let(:product_price) { 150 }
 
-          it { is_expected.to eq("(Subtract: &#x00A5;50)") }
+          it { is_expected.to eq("(Subtract: <span class=\"money-currency-symbol\">&#x00A5;</span><span class=\"money-whole\">50</span>)") }
         end
       end
     end
@@ -79,8 +79,8 @@ module Spree
 
       context "when currency is default" do
         it "should return the variant price if the price is different than master" do
-          expect(helper.variant_price(variant)).to eq("$15.00")
-          expect(helper.variant_price(variant_2)).to eq("$20.00")
+          expect(helper.variant_price(variant)).to eq("<span class=\"money-currency-symbol\">$</span><span class=\"money-whole\">15</span><span class=\"money-decimal-mark\">.</span><span class=\"money-decimal\">00</span>")
+          expect(helper.variant_price(variant_2)).to eq("<span class=\"money-currency-symbol\">$</span><span class=\"money-whole\">20</span><span class=\"money-decimal-mark\">.</span><span class=\"money-decimal\">00</span>")
         end
       end
 
@@ -95,7 +95,7 @@ module Spree
         end
 
         it "should return the variant price if the price is different than master" do
-          expect(helper.variant_price(variant)).to eq("&#x00A5;150")
+          expect(helper.variant_price(variant)).to eq("<span class=\"money-currency-symbol\">&#x00A5;</span><span class=\"money-whole\">150</span>")
         end
       end
 

core/spec/lib/spree/money_spec.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
-require 'spec_helper'
-require 'spree/money'
+require 'rails_helper'
 
 RSpec.describe Spree::Money do
   before do
@@ -97,7 +96,7 @@
 
   context "with currency" do
     it "passed in option" do
-      money = Spree::Money.new(10, with_currency: true, html: false)
+      money = Spree::Money.new(10, with_currency: true, html_wrap: false)
       expect(money.to_s).to eq("$10.00 USD")
     end
   end
@@ -117,27 +116,27 @@
   context "currency parameter" do
     context "when currency is specified in Canadian Dollars" do
       it "uses the currency param over the global configuration" do
-        money = Spree::Money.new(10, currency: 'CAD', with_currency: true, html: false)
+        money = Spree::Money.new(10, currency: 'CAD', with_currency: true, html_wrap: false)
         expect(money.to_s).to eq("$10.00 CAD")
       end
     end
 
     context "when currency is specified in Japanese Yen" do
       it "uses the currency param over the global configuration" do
-        money = Spree::Money.new(100, currency: 'JPY', html: false)
+        money = Spree::Money.new(100, currency: 'JPY', html_wrap: false)
         expect(money.to_s).to eq("¥100")
       end
     end
   end
 
   context "symbol positioning" do
     it "passed in option" do
-      money = Spree::Money.new(10, symbol_position: :after, html: false)
+      money = Spree::Money.new(10, format: '%n %u', html_wrap: false)
       expect(money.to_s).to eq("10.00 $")
     end
 
     it "config option" do
-      money = Spree::Money.new(10, symbol_position: :after, html: false)
+      money = Spree::Money.new(10, format: '%n %u', html_wrap: false)
       expect(money.to_s).to eq("10.00 $")
     end
   end
@@ -162,7 +161,7 @@
     end
 
     it "formats correctly" do
-      money = Spree::Money.new(1000, html: false)
+      money = Spree::Money.new(1000, html_wrap: false)
       expect(money.to_s).to eq("¥1,000")
     end
   end
@@ -176,20 +175,20 @@
 
     # Regression test for https://github.com/spree/spree/issues/2634
     it "formats as plain by default" do
-      money = Spree::Money.new(10, symbol_position: :after)
+      money = Spree::Money.new(10, format: '%n %u')
       expect(money.to_s).to eq("10.00 €")
     end
 
     it "formats as HTML if asked (nicely) to" do
-      money = Spree::Money.new(10, symbol_position: :after)
+      money = Spree::Money.new(10, format: '%n %u')
       # The HTML'ified version of "10.00 €"
-      expect(money.to_html).to eq("10.00 €")
+      expect(money.to_html).to eq("<span class=\"money-whole\">10</span><span class=\"money-decimal-mark\">.</span><span class=\"money-decimal\">00</span> <span class=\"money-currency-symbol\">&#x20AC;</span>")
     end
 
     it "formats as HTML with currency" do
-      money = Spree::Money.new(10, symbol_position: :after, with_currency: true)
+      money = Spree::Money.new(10, format: '%n %u', with_currency: true)
       # The HTML'ified version of "10.00 €"
-      expect(money.to_html).to eq("10.00&nbsp;&#x20AC; <span class=\"currency\">EUR</span>")
+      expect(money.to_html).to eq("<span class=\"money-whole\">10</span><span class=\"money-decimal-mark\">.</span><span class=\"money-decimal\">00</span> <span class=\"money-currency-symbol\">&#x20AC;</span> <span class=\"money-currency\">EUR</span>")
     end
   end