From 11a9b22620d51be11da7fb8ecb9c53f53e9efa76 Mon Sep 17 00:00:00 2001 From: Chad Pytel Date: Fri, 20 Jul 2018 14:35:20 +0100 Subject: [PATCH] Fix i18n inconsistencies in forms for associations Fixes #1105. As described in #1105, there are some inconsistencies in the way Administrate handles labels for `HasMany`/`BelongsTo`/`HasOne` associations. * `HasMany` (eg: `Order#line_items`) - Gives "line_items" as the label - No i18n. * `BelongsTo` (eg: `Customer#country`) - Defaults to "Country code" (similar to "Country id" in this case). - Allows i18n as `helpers.label.customer.country_code`. * `HasOne` (eg: `Product#product_meta_tag`) - Gives "Product Meta Tag". - No i18n. The behavior of the `BelongsTo` field is most correct, and is preserved in this commit. The behavior of `HasMany` and `HasOne` has been improved as follows: * `HasMany` (eg: `Order#line_items`) now no longer overrides the text given to the `label` helper, and passes the attribute name. This results in a default label of "Line items" and one that can be overridden with i18n with a key at the standard location (`helpers.label.order.line_items`) * `HasOne` (eg: `Product#product_meta_tag`) now can be overriden with the key `helpers.label.order.line_items`. The `HasOne` is rendered into a nested form, inside of a fieldset and with a legend. The text of the "label" in this nested form is actually in a legend tag. I considered the fact that this was not using the `label` helper, whether it should be switch to a label from a legend, or whether we should have a non-standard i18n key. In the end, I decided that the least surprising behavior would be to customize the "label" for that attribute at the expected label's i18n key. --- app/views/fields/has_many/_form.html.erb | 2 +- app/views/fields/has_one/_form.html.erb | 2 +- .../views/fields/has_many/_form_spec.rb | 4 +-- .../views/fields/has_one/_form_spec.rb | 1 + spec/features/orders_form_spec.rb | 31 ++++++++++++++++--- spec/features/products_form_spec.rb | 25 +++++++++++++++ 6 files changed, 56 insertions(+), 9 deletions(-) diff --git a/app/views/fields/has_many/_form.html.erb b/app/views/fields/has_many/_form.html.erb index ccabd09c9f..da36d904db 100644 --- a/app/views/fields/has_many/_form.html.erb +++ b/app/views/fields/has_many/_form.html.erb @@ -20,7 +20,7 @@ and is augmented with [Selectize]. %>
- <%= f.label field.attribute_key, field.attribute %> + <%= f.label field.attribute, for: "#{f.object_name}_#{field.attribute_key}" %>
<%= f.select(field.attribute_key, nil, {}, multiple: true) do %> diff --git a/app/views/fields/has_one/_form.html.erb b/app/views/fields/has_one/_form.html.erb index 701f9e7ce0..8bdfa53eef 100644 --- a/app/views/fields/has_one/_form.html.erb +++ b/app/views/fields/has_one/_form.html.erb @@ -18,7 +18,7 @@ The form will be rendered as nested_from to parent relationship. <%= f.fields_for field.attribute, field.data || field.nested_form.resource.class.new do |has_one_f| %>
- <%= field.nested_form.resource_name.titleize %> + <%= t "helpers.label.#{f.object_name}.#{field.nested_form.resource_name}", default: field.nested_form.resource_name.titleize %> <% field.nested_form.attributes.each do |attribute| -%>
<%= render_field attribute, f: has_one_f %> diff --git a/spec/administrate/views/fields/has_many/_form_spec.rb b/spec/administrate/views/fields/has_many/_form_spec.rb index c52d90a3f3..83de3a1494 100644 --- a/spec/administrate/views/fields/has_many/_form_spec.rb +++ b/spec/administrate/views/fields/has_many/_form_spec.rb @@ -13,14 +13,14 @@ locals: { f: fake_form_builder, field: has_many }, ) - expect(rendered).to include("Associated Objects") + expect(rendered).to include("Associated objects") end end def fake_form_builder double("Form Builder").as_null_object.tap do |form_builder| allow(form_builder).to receive(:label) do |*args| - args.second.to_s.titleize + args.first.to_s.humanize end end end diff --git a/spec/administrate/views/fields/has_one/_form_spec.rb b/spec/administrate/views/fields/has_one/_form_spec.rb index c7e9fc0851..713700eaca 100644 --- a/spec/administrate/views/fields/has_one/_form_spec.rb +++ b/spec/administrate/views/fields/has_one/_form_spec.rb @@ -23,6 +23,7 @@ def form_builder allow(builder).to receive(:fields_for) do |&block| block.call(double("Fields For Form Builder")) end + allow(builder).to receive(:object_name).and_return(:product) builder end diff --git a/spec/features/orders_form_spec.rb b/spec/features/orders_form_spec.rb index 6738e0e18d..0d6725cf1d 100644 --- a/spec/features/orders_form_spec.rb +++ b/spec/features/orders_form_spec.rb @@ -36,8 +36,8 @@ line_items = create_list(:line_item, 3) visit edit_admin_order_path(order) - find_option(line_items.first, "line_item_ids").select_option - find_option(line_items.last, "line_item_ids").select_option + find_option(line_items.first, "Line items").select_option + find_option(line_items.last, "Line items").select_option click_on "Update Order" order.reload @@ -51,7 +51,7 @@ line_item = create(:line_item, order: order) visit edit_admin_order_path(order) - find_option(line_item, "line_item_ids").unselect_option + find_option(line_item, "Line items").unselect_option click_on "Update Order" order.reload @@ -71,8 +71,29 @@ expect(find("#order_line_item_ids").value).to match_array(expected) end - def find_option(associated_model, field_id) - field = find("#order_" + field_id) + it "displays translated labels" do + custom_label = "Lines" + order = create(:order) + + translations = { + helpers: { + label: { + order: { + line_items: custom_label, + }, + }, + }, + } + + with_translations(:en, translations) do + visit edit_admin_order_path(order) + + expect(page).to have_css("label", text: custom_label) + end + end + + def find_option(associated_model, field_locator) + field = find_field(field_locator) field.find("option", text: displayed(associated_model)) end end diff --git a/spec/features/products_form_spec.rb b/spec/features/products_form_spec.rb index c3f84ab8bb..383f98a151 100644 --- a/spec/features/products_form_spec.rb +++ b/spec/features/products_form_spec.rb @@ -11,6 +11,8 @@ fill_in "Meta title", with: "Example meta title" fill_in "Meta description", with: "Example meta description" + expect(page).to have_css("legend", text: "Product Meta Tag") + click_on "Create Product" expect(page).to have_link(ProductMetaTag.last.id) @@ -31,4 +33,27 @@ t("administrate.controller.update.success", resource: "Product") ) end + + describe "has_one relationships" do + it "displays translated labels" do + custom_label = "Meta Tags" + product = create(:product) + + translations = { + helpers: { + label: { + product: { + product_meta_tag: custom_label, + }, + }, + }, + } + + with_translations(:en, translations) do + visit edit_admin_product_path(product) + + expect(page).to have_css("legend", text: custom_label) + end + end + end end