Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Spree.formatMoney using accounting.js #1745

Merged
merged 5 commits into from
Feb 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
* The `AvailabilityValidator` now correctly detects out of stock when there
are multiple shipments from the same stock location.

* The spree/admin/shared/\_translations partial has moved to
spree/admin/shared/\_js\_locale\_data.

* New method Spree.formatMoney(amount, currency) is available to javascript in
the admin.

* Deprecations

* `cache_key_for_taxons` helper has been deprecated in favour of `cache [I18n.locale, @taxons]`
Expand Down
1 change: 1 addition & 0 deletions backend/app/assets/javascripts/spree/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//= require spree/backend/translation
//= require spree/backend/backbone-overrides
//= require spree/backend/spree-select2
//= require spree/backend/format_money
//
//= require spree/backend/templates
//
Expand Down
11 changes: 11 additions & 0 deletions backend/app/assets/javascripts/spree/backend/format_money.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//= require spree/backend/translation
//= require solidus_admin/accounting

Spree.formatMoney = function(amount, currency) {
var currencyInfo = Spree.currencyInfo[currency];

var thousand = Spree.t('currency_delimiter');
var decimal = Spree.t('currency_separator');

return accounting.formatMoney(amount, currencyInfo[0], currencyInfo[1], thousand, decimal, currencyInfo[2]);
}
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/shared/_head.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

<%= javascript_include_tag 'spree/backend/all' %>

<%= render "spree/admin/shared/translations" %>
<%= render "spree/admin/shared/js_locale_data" %>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea 👌🏻 Much more convenient.

This deserves a CHANGELOG entry though, as people may have modified one of these files. Even deface overrides will not work anymore as the file name changed.


<%= javascript_tag do -%>
Spree.api_key = "<%= try_spree_current_user.try(:spree_api_key) %>";
Expand Down
47 changes: 47 additions & 0 deletions backend/app/views/spree/admin/shared/_js_locale_data.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<script>
if (Spree === undefined) {
var Spree = {}
}
Spree.translations = <%==
I18n.t("spree").merge({
date_picker: Spree.t(:js_format,
scope: 'date_picker',
default: 'yy/mm/dd'),
abbr_day_names: I18n.t(:abbr_day_names, scope: :date),
destroy: Spree.t(:destroy, scope: :actions),
edit: Spree.t(:edit, scope: :actions),
save: Spree.t(:save, scope: :actions),
cancel: Spree.t(:cancel, scope: :actions),
first_day: Spree.t(:first_day,
scope: 'date_picker',
default: 0).to_i,
item_stock_placeholder: Spree.t(:choose_location),
month_names: I18n.t(:month_names, scope: :date).reject(&:blank?),
currency_separator: I18n.t('number.currency.format.separator'),
currency_delimiter: I18n.t('number.currency.format.delimiter'),
activerecord: I18n.t('activerecord')
}).to_json
%>;

Spree.currencyInfo = <%==
Money::Currency.all.map { |c|
format =
if c.symbol == "" || c.symbol_first
"%s%v"
else
"%v %s"
end
[c.id.to_s.upcase, [
c.symbol || "¤",
c.exponent,
format
]]
}.to_h.to_json
%>;
</script>
<script data-hook='admin-custom-translations'>
</script>

<% if I18n.locale != :en %>
<%= javascript_include_tag "select2_locale_#{I18n.locale}" %>
<% end %>
30 changes: 0 additions & 30 deletions backend/app/views/spree/admin/shared/_translations.html.erb

This file was deleted.

2 changes: 1 addition & 1 deletion backend/app/views/spree/layouts/admin_style_guide.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<%= stylesheet_link_tag "spree/backend/all" %>
<%= javascript_include_tag "spree/backend/all" %>
<%= render "spree/admin/shared/translations" %>
<%= render "spree/admin/shared/js_locale_data" %>
</head>

<body class="admin style-guide">
Expand Down
20 changes: 20 additions & 0 deletions backend/spec/features/admin/javascript_format_money_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
require 'spec_helper'

RSpec.describe 'JS Spree.formatMoney', js: true do
stub_authorization!

# This is a slightly hacky spec to ensure that our JS will format money in
# the same was as our ruby code.
# This should probably replaced with a pure JS test in the future.
it 'should behave identically to Spree::Money#to_s' do
visit '/admin'

Money::Currency.all.map(&:id).map(&:to_s).map(&:upcase).uniq.each do |currency|
money = Spree::Money.new(1234, currency: currency)

js_result = page.evaluate_script("Spree.formatMoney(#{money.to_d}, '#{currency}')")

expect(js_result).to eq money.to_s
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually find this a very good way to test this. 👍

end
end
4 changes: 4 additions & 0 deletions backend/vendor/assets/javascripts/solidus_admin/accounting.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.