Skip to content

Commit

Permalink
Update error messages for routing errors
Browse files Browse the repository at this point in the history
- Add route_number to available interpolations for conditions error messages translations
- Update error message for route when end question is before the start question
- Update error message for route answer value has been deleted or changed
- Update error message for route where start and end are consecutive
  • Loading branch information
lfdebrux committed Feb 27, 2025
1 parent 53c1175 commit 6354749
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 16 deletions.
4 changes: 3 additions & 1 deletion app/components/page_list_component/error_summary/view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ def self.error_id(number)
end

def error_object(error_name:, condition:, page:)
OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", question_number: page.position), link: "##{self.class.error_id(condition.id)}")
# TODO: route_number is hardcoded as 1 here because we know there can be only two conditions. It will need to change in future
route_number = condition.secondary_skip? ? I18n.t("page_conditions.errors.route_number_for_any_other_answer") : 1
OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", question_number: page.position, route_number:), link: "##{self.class.error_id(condition.id)}")
end

def conditions_with_check_pages
Expand Down
4 changes: 3 additions & 1 deletion app/components/page_list_component/view.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
<ul class="govuk-list govuk-!-margin-0">
<% condition.validation_errors.each do |error| %>
<li class="app-page_list__route-text--error">
<%= t("page_conditions.errors.#{error.name}", question_number: condition_page_position) %>
<%# TODO: route_number is hardcoded as 1 here because we know there can be only two conditions. It will need to change in future %>
<% route_number = condition.secondary_skip? ? t("page_conditions.errors.route_number_for_any_other_answer") : 1 %>
<%= t("page_conditions.errors.#{error.name}", question_number: condition_page_position, route_number:) %>
</li>
<% end %>
</ul>
Expand Down
7 changes: 4 additions & 3 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1023,10 +1023,11 @@ en:
condition_goto_page_text_with_errors: "[Question not selected]"
condition_name: Question %{question_number}’s routes
errors:
answer_value_doesnt_exist: The answer that question %{question_number}’s route is based on no longer exists - edit question %{question_number}’s route
cannot_have_goto_page_before_routing_page: The question you’re taking the person to is now above the route - edit question %{question_number}’s route
cannot_route_to_next_page: Question %{question_number}’s route is now next to the route’s end question - this makes the route unnecessary. Edit question %{question_number}’s route.
answer_value_doesnt_exist: The answer that question %{question_number}’s route %{route_number} is based on no longer exists - edit or delete this route
cannot_have_goto_page_before_routing_page: Question %{question_number}’s route %{route_number} is going to a question that comes before the start of the route - edit or delete this route
cannot_route_to_next_page: Question %{question_number}’s route %{route_number} is not skipping any questions - edit or delete this route
goto_page_doesnt_exist: The question you’re taking the person to no longer exists - edit question %{question_number}’s route
route_number_for_any_other_answer: for any other answer
none_of_the_above: None of the above
route: Route
secondary_skip_description: After %{check_page_question_text} go to %{goto_page_question_text}
Expand Down
14 changes: 7 additions & 7 deletions spec/components/page_list_component/error_summary/view_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
end

it "renders the error link" do
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1)
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1, route_number: 1)
expect(page).to have_link(condition_answer_value_error, href: "##{described_class.error_id(routing_conditions[0].id)}")
end
end
Expand All @@ -71,8 +71,8 @@
end

it "renders both error links" do
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1)
condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 2)
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1, route_number: 1)
condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 2, route_number: 1)
expect(page).to have_link(condition_answer_value_error, href: "##{described_class.error_id(routing_conditions_page_with_answer_value_missing[0].id)}")
expect(page).to have_link(condition_goto_page_error, href: "##{described_class.error_id(routing_conditions_page_with_goto_page_missing[0].id)}")
end
Expand All @@ -90,7 +90,7 @@
end

it "renders the error summary" do
error_message = I18n.t("page_conditions.errors.cannot_route_to_next_page", question_number: 2)
error_message = I18n.t("page_conditions.errors.cannot_route_to_next_page", question_number: 2, route_number: "for any other answer")
expect(page).to have_css ".govuk-error-summary", text: error_message
expect(page).to have_link error_message, href: "#condition_#{branch_any_other_answer_route.id}"
end
Expand Down Expand Up @@ -121,7 +121,7 @@
it "returns an error object in the correct format" do
condition = build :condition, id: 1
page = build :page, position: 1
expect(error_summary_component.error_object(error_name: "answer_value_doesnt_exist", condition:, page:)).to eq OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1), link: "##{described_class.error_id(1)}")
expect(error_summary_component.error_object(error_name: "answer_value_doesnt_exist", condition:, page:)).to eq OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1, route_number: 1), link: "##{described_class.error_id(1)}")
end
end

Expand Down Expand Up @@ -149,8 +149,8 @@
describe "#errors_for_summary" do
it "returns all of the routing errors for a form with their respective positions and links" do
expect(error_summary_component.errors_for_summary).to eq [
OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1), link: "##{described_class.error_id(1)}"),
OpenStruct.new(message: I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 2), link: "##{described_class.error_id(2)}"),
OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1, route_number: 1), link: "##{described_class.error_id(1)}"),
OpenStruct.new(message: I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 2, route_number: 1), link: "##{described_class.error_id(2)}"),
]
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/components/page_list_component/view_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@
let(:routing_conditions) { [(build :condition, :with_answer_value_missing, id: 1, routing_page_id: 1, check_page_id: 1, goto_page_id: 3)] }

it "renders the errors in an unordered list" do
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1)
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1, route_number: 1)
expect(page).to have_css("ul > li", text: condition_answer_value_error)
end

Expand Down Expand Up @@ -144,8 +144,8 @@
let(:routing_conditions) { [(build :condition, :with_answer_value_and_goto_page_missing, id: 1, routing_page_id: 1, check_page_id: 1)] }

it "renders the errors in an unordered list" do
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1)
condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 1)
condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1, route_number: 1)
condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 1, route_number: 1)
expect(page).to have_css("ul > li", text: condition_answer_value_error)
expect(page).to have_css("ul > li", text: condition_goto_page_error)
end
Expand Down Expand Up @@ -176,7 +176,7 @@
end

it "renders an error message" do
error_message = I18n.t("page_conditions.errors.cannot_route_to_next_page", question_number: 2)
error_message = I18n.t("page_conditions.errors.cannot_route_to_next_page", question_number: 2, route_number: "for any other answer")
expect(page).to have_css ".app-page_list__route-text--error", text: error_message
end
end
Expand Down

0 comments on commit 6354749

Please sign in to comment.