Merge pull request #1811 from alphagov/ldeb-update-existing-error-mes…

…sages-for-branching

Update error messages for routing errors

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

app/components/page_list_component/view.html.erb

@@ -33,6 +33,7 @@
           </div>
 
           <%  page.routing_conditions.each do |condition| %>
+            <% condition_page = self.condition_page(condition) %>
             <% condition_page_position = self.condition_page_position(condition) %>
             <div id="<%= PageListComponent::ErrorSummary::View.error_id(condition.id) %>" class="govuk-summary-list__row app-page-list__row <%= class_names( "govuk-form-group--error": condition.has_routing_errors?) %>">
               <dt class="govuk-summary-list__key govuk-summary-list__key app-page-list__key">
@@ -43,7 +44,7 @@
                 <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) %>
+                      <%= PageListComponent::ErrorSummary::View.generate_error_message(error.name, condition:, page: condition_page) %>
                     </li>
                   <% end %>
                 </ul>

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
 

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}

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

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