diff --git a/app/components/page_list_component/error_summary/view.rb b/app/components/page_list_component/error_summary/view.rb index 450b6d011..290187f7f 100644 --- a/app/components/page_list_component/error_summary/view.rb +++ b/app/components/page_list_component/error_summary/view.rb @@ -10,8 +10,24 @@ def self.error_id(number) "condition_#{number}" end + def self.generate_error_message(error_name, condition:, page:) + # 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("errors.page_conditions.route_number_for_any_other_answer") : 1 + + interpolation_variables = { question_number: page.position, route_number: } + + scope = "errors.page_conditions" + defaults = [:"#{error_name}"] + defaults.prepend(:"any_other_answer_route.#{error_name}") if condition.secondary_skip? + + I18n.t(defaults.first, default: defaults.drop(1), scope:, **interpolation_variables) + 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)}") + OpenStruct.new( + message: self.class.generate_error_message(error_name, condition:, page:), + link: "##{self.class.error_id(condition.id)}", + ) end def conditions_with_check_pages diff --git a/app/components/page_list_component/view.html.erb b/app/components/page_list_component/view.html.erb index 1e0013c00..9cdccdd8d 100644 --- a/app/components/page_list_component/view.html.erb +++ b/app/components/page_list_component/view.html.erb @@ -33,6 +33,7 @@ <% page.routing_conditions.each do |condition| %> + <% condition_page = self.condition_page(condition) %> <% condition_page_position = self.condition_page_position(condition) %>
">
@@ -43,7 +44,7 @@ diff --git a/app/components/page_list_component/view.rb b/app/components/page_list_component/view.rb index b6f08ad17..8a1dbbca0 100644 --- a/app/components/page_list_component/view.rb +++ b/app/components/page_list_component/view.rb @@ -57,8 +57,12 @@ def page_position(page) page.position end + def condition_page(condition) + condition.attributes["check_page"] ||= @pages.find { |page| page.id == condition.check_page_id } + end + def condition_page_position(condition) - check_page = @pages.find { |page| page.id == condition.check_page_id } + check_page = condition_page(condition) page_position(check_page) end diff --git a/config/locales/en.yml b/config/locales/en.yml index 2e44c0556..3c4daa701 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -292,6 +292,14 @@ en: errors: messages: non_government_email: Enter a government email address + page_conditions: + 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 + any_other_answer_route: + cannot_have_goto_page_before_routing_page: Question %{question_number}’s route for any other answer is going to a question that comes before the question it skips from - 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 prefix: 'Error:' footer: accessibility_statement: Accessibility statement @@ -1022,11 +1030,6 @@ en: condition_goto_page_text: "%{goto_page_question_number}, “%{goto_page_question_text}”" 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. - goto_page_doesnt_exist: The question you’re taking the person to no longer exists - edit question %{question_number}’s route none_of_the_above: None of the above route: Route secondary_skip_description: After %{check_page_question_text} go to %{goto_page_question_text} diff --git a/spec/components/page_list_component/error_summary/view_spec.rb b/spec/components/page_list_component/error_summary/view_spec.rb index 3e7d4f325..0775c9b8c 100644 --- a/spec/components/page_list_component/error_summary/view_spec.rb +++ b/spec/components/page_list_component/error_summary/view_spec.rb @@ -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("errors.page_conditions.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 @@ -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("errors.page_conditions.answer_value_doesnt_exist", question_number: 1, route_number: 1) + condition_goto_page_error = I18n.t("errors.page_conditions.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 @@ -90,7 +90,22 @@ 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("errors.page_conditions.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 + end + + context "and the any other answer route skip to question has been moved to before the skip from question" do + before do + branch_any_other_answer_route.has_routing_errors = true + branch_any_other_answer_route.validation_errors = [OpenStruct.new(name: "cannot_have_goto_page_before_routing_page")] + + render_inline(error_summary_component) + end + + it "renders an error message" do + error_message = I18n.t("errors.page_conditions.any_other_answer_route.cannot_have_goto_page_before_routing_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 @@ -121,7 +136,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("errors.page_conditions.answer_value_doesnt_exist", question_number: 1, route_number: 1), link: "##{described_class.error_id(1)}") end end @@ -149,8 +164,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("errors.page_conditions.answer_value_doesnt_exist", question_number: 1, route_number: 1), link: "##{described_class.error_id(1)}"), + OpenStruct.new(message: I18n.t("errors.page_conditions.goto_page_doesnt_exist", question_number: 2, route_number: 1), link: "##{described_class.error_id(2)}"), ] end end diff --git a/spec/components/page_list_component/view_spec.rb b/spec/components/page_list_component/view_spec.rb index b23f37f7b..b8e35d149 100644 --- a/spec/components/page_list_component/view_spec.rb +++ b/spec/components/page_list_component/view_spec.rb @@ -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("errors.page_conditions.answer_value_doesnt_exist", question_number: 1, route_number: 1) expect(page).to have_css("ul > li", text: condition_answer_value_error) end @@ -125,7 +125,7 @@ let(:routing_conditions) { [(build :condition, :with_goto_page_missing, id: 1, routing_page_id: 1, check_page_id: 1, answer_value: "Wales")] } it "renders the errors in an unordered list" do - condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 1) + condition_goto_page_error = I18n.t("errors.page_conditions.goto_page_doesnt_exist", question_number: 1) expect(page).to have_css("ul > li", text: condition_goto_page_error) end @@ -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("errors.page_conditions.answer_value_doesnt_exist", question_number: 1, route_number: 1) + condition_goto_page_error = I18n.t("errors.page_conditions.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 @@ -176,7 +176,21 @@ 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("errors.page_conditions.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 + + context "and the any other answer route skip to question has been moved to before the skip from question" do + before do + branch_any_other_answer_route.has_routing_errors = true + branch_any_other_answer_route.validation_errors = [OpenStruct.new(name: "cannot_have_goto_page_before_routing_page")] + + render_inline(page_list_component) + end + + it "renders an error message" do + error_message = I18n.t("errors.page_conditions.any_other_answer_route.cannot_have_goto_page_before_routing_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