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

Dynamically render ReportsController translations #2751

Merged
Merged
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
Dynamically render ReportsController translations
This commit modifies how translations for report names/descriptions are
rendered by the `Spree::Admin::ReportsController` class.

The previous implementation generates translations once, at startup,
rendering the translated results statically onto the index page.

This results in confusing behaviour when attempting to add additional
reports to this class via prepended modules or `class_eval`, as the
`i18n` subsystem is not initialized until late in the Rails boot
process, potentially after these modifications have been loaded.

This results in strange and confusing behaviour where the page insists
translations are missing, despite being present.

To remove this source of confusion, we can store the name/description
translation keys in our `@@available_reports` class variable, and
generate the translations from them when the index page is rendered.
stewart committed May 29, 2018
commit a737dd34364c9f9ca3be7a7b3acca41dac5c14ce
6 changes: 5 additions & 1 deletion backend/app/controllers/spree/admin/reports_controller.rb
Original file line number Diff line number Diff line change
@@ -14,7 +14,11 @@ def add_available_report!(report_key, report_description_key = nil)
if report_description_key.nil?
report_description_key = "#{report_key}_description"
end
@@available_reports[report_key] = { name: I18n.t(report_key, scope: 'spree'), description: I18n.t(report_description_key, scope: 'spree') }

@@available_reports[report_key] = {
name: report_key,
description: report_description_key,
}
end
end

4 changes: 2 additions & 2 deletions backend/app/views/spree/admin/reports/index.html.erb
Original file line number Diff line number Diff line change
@@ -11,8 +11,8 @@
<tbody>
<% @reports.each do |key, value| %>
<tr data-hook="reports_row">
<td><%= link_to value[:name], send("#{key}_admin_reports_url".to_sym) %></td>
<td><%= value[:description] %></td>
<td><%= link_to t(value[:name], scope: 'spree'), send("#{key}_admin_reports_url".to_sym) %></td>
<td><%= t(value[:description], scope: 'spree') %></td>
</tr>
<% end %>
</tbody>
12 changes: 2 additions & 10 deletions backend/spec/controllers/spree/admin/reports_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -9,24 +9,16 @@
it 'should contain sales_total' do
expect(Spree::Admin::ReportsController.available_reports.keys.include?(:sales_total)).to be true
end

it 'should have the proper sales total report description' do
expect(Spree::Admin::ReportsController.available_reports[:sales_total][:description]).to eql('Sales Total For All Orders')
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may be worth replacing this test with a view spec, but I'm uncertain how much value testing translations provides in this context.

end

describe 'ReportsController.add_available_report!' do
context 'when adding the report name' do
it 'should contain the report' do
I18n.backend.store_translations(:en, spree: {
some_report: 'Awesome Report',
some_report_description: 'This report is great!'
})
Spree::Admin::ReportsController.add_available_report!(:some_report)
expect(Spree::Admin::ReportsController.available_reports.keys.include?(:some_report)).to be true
expect(Spree::Admin::ReportsController.available_reports[:some_report]).to eq(
name: 'Awesome Report',
description: 'This report is great!'
name: :some_report,
description: 'some_report_description'
)
end
end