From e529d89310a8dd6a6674b312122815c357270744 Mon Sep 17 00:00:00 2001 From: JDutil Date: Thu, 11 Oct 2018 22:29:56 -0600 Subject: [PATCH] Fix multiple Money gem deprecations. * [DEPRECATION] `use_i18n` is deprecated - use `Money.locale_backend = :i18n` instead * [DEPRECATION] `html` is deprecated - use `html_wrap` instead. Please note that `html_wrap` will wrap all parts of currency and if you use `with_currency` option, currency element class changes from `currency` to `money-currency`. * [DEPRECATION] `symbol_position:` option is deprecated - use `format` to specify the formatting template. * Initialize Money in initializer for main app to override. --- CHANGELOG.md | 3 +++ backend/spec/features/admin/users_spec.rb | 31 +++++++++++++++-------- core/config/initializers/money.rb | 5 ++++ core/lib/spree/money.rb | 24 ++++++++++-------- core/spec/helpers/products_helper_spec.rb | 14 +++++----- core/spec/lib/spree/money_spec.rb | 25 +++++++++--------- 6 files changed, 61 insertions(+), 41 deletions(-) create mode 100644 core/config/initializers/money.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 7448c27bf10..74943847e41 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/backend/spec/features/admin/users_spec.rb b/backend/spec/features/admin/users_spec.rb index c4c51e5b2be..8f972f6b68c 100644 --- a/backend/spec/features/admin/users_spec.rb +++ b/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 diff --git a/core/config/initializers/money.rb b/core/config/initializers/money.rb new file mode 100644 index 00000000000..0d216f7896b --- /dev/null +++ b/core/config/initializers/money.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +require 'money' + +Money.locale_backend = :i18n diff --git a/core/lib/spree/money.rb b/core/lib/spree/money.rb index 5c3d1d8b8ce..96e54200ad7 100644 --- a/core/lib/spree/money.rb +++ b/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 diff --git a/core/spec/helpers/products_helper_spec.rb b/core/spec/helpers/products_helper_spec.rb index 6799b6b1a76..da38bf267d5 100644 --- a/core/spec/helpers/products_helper_spec.rb +++ b/core/spec/helpers/products_helper_spec.rb @@ -31,7 +31,7 @@ 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: $5.00)") } # Regression test for https://github.com/spree/spree/issues/2737 it { is_expected.to be_html_safe } end @@ -39,7 +39,7 @@ module Spree 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: $5.00)") } 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: ¥50)") } + it { is_expected.to eq("(Add: ¥50)") } end context "when variant is less than master" do let(:product_price) { 150 } - it { is_expected.to eq("(Subtract: ¥50)") } + it { is_expected.to eq("(Subtract: ¥50)") } 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("$15.00") + expect(helper.variant_price(variant_2)).to eq("$20.00") 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("¥150") + expect(helper.variant_price(variant)).to eq("¥150") end end diff --git a/core/spec/lib/spree/money_spec.rb b/core/spec/lib/spree/money_spec.rb index f2461074cfb..e2d5619518c 100644 --- a/core/spec/lib/spree/money_spec.rb +++ b/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,14 +116,14 @@ 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 @@ -132,12 +131,12 @@ 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("10.00 €") 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 € EUR") + expect(money.to_html).to eq("10.00 € EUR") end end