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

Spike out adding custom error messages via locales #8356

Closed
Closed
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
32 changes: 31 additions & 1 deletion app/components/admin/error_summary_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def humanized_class_name
def error_items
errors.map do |error|
error_item = {
text: error.full_message,
text: error_message_text(error),
data_attributes: track_analytics_data("form-error", analytics_action, error.full_message),
}

Expand All @@ -51,4 +51,34 @@ def errors
object.errors
end
end

def error_message_text(error)
if custom_error_present_in_locales?(error)
custom_error_message(error)
Copy link
Contributor Author

@davidgisbey davidgisbey Oct 11, 2023

Choose a reason for hiding this comment

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

i actually think this should just be error.message tbh

I ran into an issue with this being overwritten by this custom error message.

I think the right thing to do though is add custom messages for each affected attribute in the locales files. Having an overwritten error message on the object itself is bad.

else
error.full_message
end
end

def custom_error_present_in_locales?(error)
custom_error_message(error)
rescue RuntimeError
false
end

def custom_error_message(error)
I18n.t("activerecord.errors.models.#{model_for_locale(error)}.attributes.#{attribute_for_locale(error)}.#{error.type}")
end

def model_for_locale(error)
return parent_class unless error.is_a?(ActiveModel::NestedError)

error.attribute.to_s.split(".")[-2]
end

def attribute_for_locale(error)
return error.attribute unless error.is_a?(ActiveModel::NestedError)

error.attribute.to_s.split(".").last
end
end
8 changes: 8 additions & 0 deletions config/locales/en/attachments.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
en:
activerecord:
errors:
models:
attachment:
attributes:
title:
blank: Enter a title
9 changes: 9 additions & 0 deletions config/locales/en/editions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
en:
activerecord:
errors:
models:
edition:
attributes:
first_published_at:
blank: Enter the date when the document was first published
invalid_date: The date is not in the correct format
8 changes: 8 additions & 0 deletions config/locales/en/govspeak_content.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
en:
activerecord:
errors:
models:
govspeak_content:
attributes:
body:
blank: Enter a body
2 changes: 1 addition & 1 deletion features/step_definitions/previously_published_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
end

Then(/^I see a validation error for the missing publication date$/) do
expect(page).to have_content("First published at can't be blank")
expect(page).to have_content("Enter the date when the document was first published")
end

Then(/^I should not see a validation error on the previously published date$/) do
Expand Down
9 changes: 9 additions & 0 deletions test/components/admin/error_summary_component_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ class Admin::ErrorSummaryComponentTest < ViewComponent::TestCase
assert_equal third_link.text, "Date is invalid"
assert_equal third_link[:href], "#error_summary_test_object_date"
end

test "Uses custom error messages when present in the locales" do
attachment = build(:attachment, title: nil)
attachment.validate

render_inline(Admin::ErrorSummaryComponent.new(object: attachment, parent_class: "attachment"))

assert_equal "Enter a title", page.all(".gem-c-error-summary__list-item")[0].find("a").text
end
end

class ErrorSummaryTestObject
Expand Down
2 changes: 1 addition & 1 deletion test/functional/admin/publications_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Admin::PublicationsControllerTest < ActionController::TestCase

test "should validate first_published_at field on create if previously_published is true" do
post :create, params: { edition: controller_attributes_for(:publication).merge(previously_published: "true") }
assert_equal "First published at can't be blank", assigns(:edition).errors.full_messages.last
assert_equal "First published at Enter the date when the document was first published", assigns(:edition).errors.full_messages.last
end

view_test "edit displays publication fields and guidance" do
Expand Down
2 changes: 1 addition & 1 deletion test/unit/app/models/edition_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ class EditionTest < ActiveSupport::TestCase
test "first_published_at required when previously_published" do
edition = build(:edition, previously_published: true)
assert_not edition.valid?
assert_equal "First published at can't be blank", edition.errors.full_messages.first
assert_equal "First published at Enter the date when the document was first published", edition.errors.full_messages.first

edition.first_published_at = Time.zone.now
assert edition.valid?
Expand Down