From 54dc04b2301bcea98d9f9058b445088ad77acd6f Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 18 Feb 2025 15:17:21 +0200 Subject: [PATCH 1/6] Remove unneeded route_index from translations --- app/components/page_list_component/view.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/page_list_component/view.html.erb b/app/components/page_list_component/view.html.erb index 3bff51665..668f4f131 100644 --- a/app/components/page_list_component/view.html.erb +++ b/app/components/page_list_component/view.html.erb @@ -32,10 +32,10 @@ - <% conditions_for_page_with_index(page.id).each do |condition, route_index| %> + <% conditions_for_page_with_index(page.id).each do |condition, _route_index| %>
">
- <%= t("page_conditions.condition_name", page_index: condition_page_position(condition), route_index: route_index) %> + <%= t("page_conditions.condition_name", page_index: condition_page_position(condition)) %>
@@ -53,7 +53,7 @@
<%= govuk_link_to show_routes_path(form_id: @form_id, page_id: condition.check_page_id) do %> - <%= t("forms.form_overview.edit_with_visually_hidden_text_html", visually_hidden_text: t("page_conditions.condition_name", page_index: page.position, route_index: route_index)) %> + <%= t("forms.form_overview.edit_with_visually_hidden_text_html", visually_hidden_text: t("page_conditions.condition_name", page_index: page.position)) %> <% end %>
From de9eea1f378f59580d02300ade310dedf74d0c79 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Tue, 18 Feb 2025 09:13:00 +0200 Subject: [PATCH 2/6] Refactor translations to reduce duplication for page list component --- .../page_list_component/error_summary/view.rb | 2 +- app/components/page_list_component/view.html.erb | 2 +- config/locales/en.yml | 14 ++++---------- .../page_list_component/error_summary/view_spec.rb | 12 ++++++------ spec/components/page_list_component/view_spec.rb | 8 ++++---- 5 files changed, 16 insertions(+), 22 deletions(-) diff --git a/app/components/page_list_component/error_summary/view.rb b/app/components/page_list_component/error_summary/view.rb index 72224466f..14e2422b6 100644 --- a/app/components/page_list_component/error_summary/view.rb +++ b/app/components/page_list_component/error_summary/view.rb @@ -11,7 +11,7 @@ def self.error_id(number) end def error_object(error_name:, condition_id:, page_index:) - OpenStruct.new(message: I18n.t("page_conditions.errors.error_summary.#{error_name}", page_index:), link: "##{self.class.error_id(condition_id)}") + OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", page_index:), link: "##{self.class.error_id(condition_id)}") end def conditions_with_page_indexes diff --git a/app/components/page_list_component/view.html.erb b/app/components/page_list_component/view.html.erb index 668f4f131..5da7762a8 100644 --- a/app/components/page_list_component/view.html.erb +++ b/app/components/page_list_component/view.html.erb @@ -42,7 +42,7 @@ diff --git a/config/locales/en.yml b/config/locales/en.yml index 5435e8edf..269e426d0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1023,16 +1023,10 @@ en: condition_goto_page_text_with_errors: "[Question not selected]" condition_name: Question %{page_index}’s routes errors: - error_summary: - answer_value_doesnt_exist: The answer that question %{page_index}’s route is based on no longer exists - edit question %{page_index}’s route - cannot_have_goto_page_before_routing_page: The question you’re taking the person to is now above the route - edit question %{page_index}’s route - cannot_route_to_next_page: Question %{page_index}’s route is now next to the route’s end question - this makes the route unnecessary. Edit question %{page_index}’s route. - goto_page_doesnt_exist: The question you’re taking the person to no longer exists - edit question %{page_index}’s route - page_list: - answer_value_doesnt_exist: The answer that question %{page_index}’s route is based on no longer exists - edit question %{page_index}’s route - cannot_have_goto_page_before_routing_page: The question you’re taking the person to is now above the route - edit question %{page_index}’s route - cannot_route_to_next_page: Question %{page_index}’s route is now next to the route’s end question - this makes the route unnecessary. Edit question %{page_index}’s route. - goto_page_doesnt_exist: The question you’re taking the person to no longer exists - edit question %{page_index}’s route + answer_value_doesnt_exist: The answer that question %{page_index}’s route is based on no longer exists - edit question %{page_index}’s route + cannot_have_goto_page_before_routing_page: The question you’re taking the person to is now above the route - edit question %{page_index}’s route + cannot_route_to_next_page: Question %{page_index}’s route is now next to the route’s end question - this makes the route unnecessary. Edit question %{page_index}’s route. + goto_page_doesnt_exist: The question you’re taking the person to no longer exists - edit question %{page_index}’s route none_of_the_above: None of the above route: Route secondary_skip_description: After %{check_page_text} go to %{goto_page_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 4090b1ef2..08f5ed29c 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.error_summary.answer_value_doesnt_exist", page_index: 1) + condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 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.error_summary.answer_value_doesnt_exist", page_index: 1) - condition_goto_page_error = I18n.t("page_conditions.errors.error_summary.goto_page_doesnt_exist", page_index: 2) + condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 1) + condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 2) 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 @@ -100,7 +100,7 @@ describe "#error_object" do it "returns an error object in the correct format" do - expect(error_summary_component.error_object(error_name: "answer_value_doesnt_exist", condition_id: 1, page_index: 1)).to eq OpenStruct.new(message: I18n.t("page_conditions.errors.error_summary.answer_value_doesnt_exist", page_index: 1), link: "##{described_class.error_id(1)}") + expect(error_summary_component.error_object(error_name: "answer_value_doesnt_exist", condition_id: 1, page_index: 1)).to eq OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 1), link: "##{described_class.error_id(1)}") end end @@ -113,8 +113,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.error_summary.answer_value_doesnt_exist", page_index: 1), link: "##{described_class.error_id(1)}"), - OpenStruct.new(message: I18n.t("page_conditions.errors.error_summary.goto_page_doesnt_exist", page_index: 2), link: "##{described_class.error_id(2)}"), + OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 1), link: "##{described_class.error_id(1)}"), + OpenStruct.new(message: I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 2), 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 fc96d9804..974667248 100644 --- a/spec/components/page_list_component/view_spec.rb +++ b/spec/components/page_list_component/view_spec.rb @@ -105,7 +105,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.page_list.answer_value_doesnt_exist", page_index: 1) + condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 1) expect(page).to have_css("ul > li", text: condition_answer_value_error) end @@ -124,7 +124,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.page_list.goto_page_doesnt_exist", page_index: 1) + condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 1) expect(page).to have_css("ul > li", text: condition_goto_page_error) end @@ -143,8 +143,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.page_list.answer_value_doesnt_exist", page_index: 1) - condition_goto_page_error = I18n.t("page_conditions.errors.page_list.goto_page_doesnt_exist", page_index: 1) + condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 1) + condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 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 From 66c1531432feb0629ef1835b6aefb9daaf16ef12 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Thu, 20 Feb 2025 10:30:47 +0200 Subject: [PATCH 3/6] Refactor helpers for page list error summary component Pass the full objects through rather than unpacking early. This makes it easier to change names of variables later. --- .../page_list_component/error_summary/view.rb | 18 +++++++++--------- .../error_summary/view_spec.rb | 10 ++++++---- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/app/components/page_list_component/error_summary/view.rb b/app/components/page_list_component/error_summary/view.rb index 14e2422b6..1a91ac7aa 100644 --- a/app/components/page_list_component/error_summary/view.rb +++ b/app/components/page_list_component/error_summary/view.rb @@ -10,23 +10,23 @@ def self.error_id(number) "condition_#{number}" end - def error_object(error_name:, condition_id:, page_index:) - OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", page_index:), link: "##{self.class.error_id(condition_id)}") + def error_object(error_name:, condition:, page:) + OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", page_index: page.position), link: "##{self.class.error_id(condition.id)}") end - def conditions_with_page_indexes - @pages.map { |page| page.routing_conditions.map { |condition| OpenStruct.new(condition:, page_index: page.position) } } + def conditions_with_routing_pages + @pages.map { |page| page.routing_conditions.map { |condition| OpenStruct.new(condition:, routing_page: page) } } .flatten end def errors_for_summary - conditions_with_page_indexes - .map { |condition_with_page_index| - condition_with_page_index.condition.validation_errors.map do |error| + conditions_with_routing_pages + .map { |condition_with_routing_page| + condition_with_routing_page.condition.validation_errors.map do |error| error_object( error_name: error.name, - page_index: condition_with_page_index.page_index, - condition_id: condition_with_page_index.condition.id, + page: condition_with_routing_page.routing_page, + condition: condition_with_routing_page.condition, ) end } 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 08f5ed29c..bc074dd64 100644 --- a/spec/components/page_list_component/error_summary/view_spec.rb +++ b/spec/components/page_list_component/error_summary/view_spec.rb @@ -100,13 +100,15 @@ describe "#error_object" do it "returns an error object in the correct format" do - expect(error_summary_component.error_object(error_name: "answer_value_doesnt_exist", condition_id: 1, page_index: 1)).to eq OpenStruct.new(message: I18n.t("page_conditions.errors.answer_value_doesnt_exist", page_index: 1), link: "##{described_class.error_id(1)}") + 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", page_index: 1), link: "##{described_class.error_id(1)}") end end - describe "#conditions_with_page_indexes" do - it "returns all of the conditions for a form with their respective conditions and page positions" do - expect(error_summary_component.conditions_with_page_indexes).to eq [OpenStruct.new(condition: routing_conditions_page_with_answer_value_missing[0], page_index: 1), OpenStruct.new(condition: routing_conditions_page_with_goto_page_missing[0], page_index: 2)] + describe "#conditions_with_routing_pages" do + it "returns all of the conditions for a form with their respective conditions and routing pages" do + expect(error_summary_component.conditions_with_routing_pages).to eq [OpenStruct.new(condition: routing_conditions_page_with_answer_value_missing[0], routing_page: pages.first), OpenStruct.new(condition: routing_conditions_page_with_goto_page_missing[0], routing_page: pages.second)] end end From 6ef2775251f2005f1262e72b21d191e5096746c4 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Thu, 20 Feb 2025 11:34:51 +0200 Subject: [PATCH 4/6] Change name of translation keys for page_route_card Use question instead of page where appropriate --- .../pages/secondary_skip_input.rb | 8 +++--- .../route_summary_card_data_presenter.rb | 26 ++++++++++--------- app/views/pages/routes/show.html.erb | 2 +- config/locales/en.yml | 6 ++--- .../route_summary_card_data_presenter_spec.rb | 2 +- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/app/input_objects/pages/secondary_skip_input.rb b/app/input_objects/pages/secondary_skip_input.rb index 3c7e3a647..3db0ce50a 100644 --- a/app/input_objects/pages/secondary_skip_input.rb +++ b/app/input_objects/pages/secondary_skip_input.rb @@ -55,13 +55,13 @@ def routing_page_options pages_after_current_page(FormRepository.pages(form), page).map { |p| OpenStruct.new(id: p.id, question_text: p.question_with_number) } end - def page_name(page_id) + def question_name(page_id) target_page = FormRepository.pages(form).find { |page| page.id == page_id } - page_name = target_page.question_text + question_text = target_page.question_text page_position = target_page.position - I18n.t("page_route_card.page_name", page_position:, page_name:) + I18n.t("page_route_card.question_name_long", page_position:, question_text:) end def end_page_name @@ -73,7 +73,7 @@ def answer_value end def continue_to - page.has_next_page? ? page_name(page.next_page) : end_page_name + page.has_next_page? ? question_name(page.next_page) : end_page_name end def assign_values diff --git a/app/presenters/route_summary_card_data_presenter.rb b/app/presenters/route_summary_card_data_presenter.rb index 3f77d98ab..8c8d7ef6a 100644 --- a/app/presenters/route_summary_card_data_presenter.rb +++ b/app/presenters/route_summary_card_data_presenter.rb @@ -33,7 +33,7 @@ def conditional_route_cards end def conditional_route_card(routing_condition, index) - goto_page_name = routing_condition.skip_to_end ? end_page_name : page_name(routing_condition.goto_page_id) + goto_page_name = routing_condition.skip_to_end ? end_page_name : goto_question_name(routing_condition.goto_page_id) { card: { @@ -57,7 +57,7 @@ def conditional_route_card(routing_condition, index) end def secondary_skip_card - continue_to_name = page.has_next_page? ? page_name(page.next_page) : end_page_name + continue_to_name = page.has_next_page? ? question_name(page.next_page) : end_page_name actions = if FeatureService.new(group: form.group).enabled?(:branch_routing) && secondary_skip [ @@ -106,8 +106,8 @@ def secondary_skip_rows end end - goto_page_name = secondary_skip.skip_to_end ? end_page_name : page_name(secondary_skip.goto_page_id) - routing_page_name = page_name(secondary_skip.routing_page_id) + goto_page_name = secondary_skip.skip_to_end ? end_page_name : goto_question_name(secondary_skip.goto_page_id) + routing_page_name = question_name(secondary_skip.routing_page_id) [ { @@ -121,17 +121,19 @@ def secondary_skip_rows ] end - def page_name(page_id) + def question_name(page_id) target_page = pages.find { |page| page.id == page_id } - if target_page.present? - page_name = target_page.question_text - page_position = target_page.position + return if target_page.blank? - I18n.t("page_route_card.page_name", page_position:, page_name:) - else - I18n.t("page_route_card.page_name_not_exist") - end + question_text = target_page.question_text + page_position = target_page.position + + I18n.t("page_route_card.question_name_long", page_position:, question_text:) + end + + def goto_question_name(page_id) + question_name(page_id) || I18n.t("page_route_card.goto_page_invalid") end def end_page_name diff --git a/app/views/pages/routes/show.html.erb b/app/views/pages/routes/show.html.erb index cc05a9fac..8513dcde0 100644 --- a/app/views/pages/routes/show.html.erb +++ b/app/views/pages/routes/show.html.erb @@ -11,7 +11,7 @@ <%= govuk_summary_list(actions: false) do |summary_list| summary_list.with_row do |row| - row.with_key { t("page_route_card.question_title", position: page.position) } + row.with_key { t("page_route_card.question_name_short", position: page.position) } row.with_value { "#{page.question_text}" } end; end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 269e426d0..3fe9be784 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1053,10 +1053,10 @@ en: delete: Delete delete_route: Delete routes edit: Edit + goto_page_invalid: Edit this route to select the question or page you want to take the person to, or delete the route if_answer_is: If the answer is - page_name: "%{page_position}. %{page_name}" - page_name_not_exist: Edit this route to select the question or page you want to take the person to, or delete the route - question_title: Question %{position} + question_name_long: "%{page_position}. %{question_text}" + question_name_short: Question %{position} route_title: Route %{index} secondary_skip_after: Then after the person answers secondary_skip_then: take them to diff --git a/spec/presenters/route_summary_card_data_presenter_spec.rb b/spec/presenters/route_summary_card_data_presenter_spec.rb index 1c6c4018b..2f736fabd 100644 --- a/spec/presenters/route_summary_card_data_presenter_spec.rb +++ b/spec/presenters/route_summary_card_data_presenter_spec.rb @@ -40,7 +40,7 @@ it "uses placeholder text instead of question text" do result = service.summary_card_data - expect(result[0][:rows][1][:value][:text]).to eq(I18n.t("page_route_card.page_name_not_exist")) + expect(result[0][:rows][1][:value][:text]).to eq(I18n.t("page_route_card.goto_page_invalid")) end end end From bf564723eacf4fc4a9c201cf5daa770e5906bb8a Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Mon, 24 Feb 2025 10:52:25 +0200 Subject: [PATCH 5/6] Rename route_owner to route_page --- app/controllers/pages_controller.rb | 16 ++++++++-------- app/views/pages/delete.html.erb | 4 ++-- config/locales/en.yml | 10 +++++----- spec/views/pages/delete.html.erb_spec.rb | 8 ++++---- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 184af2365..c400d5e38 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -22,23 +22,23 @@ def delete if @page.routing_conditions.any? && @page.routing_conditions.first.secondary_skip? @routing = :start_of_secondary_skip_route - # route owner is condition check page - @route_owner = PageRepository.find(page_id: @page.routing_conditions.first.check_page_id, form_id: current_form.id) + # route page is condition check page + @route_page = PageRepository.find(page_id: @page.routing_conditions.first.check_page_id, form_id: current_form.id) elsif @page.routing_conditions.any? @routing = :start_of_route - # route owner is us - @route_owner = @page + # route page is us + @route_page = @page elsif @page_goto_conditions.any? && @page_goto_conditions.first.secondary_skip? @routing = :end_of_secondary_skip_route - # route owner is condition check page - @route_owner = PageRepository.find(page_id: @page_goto_conditions.first.check_page_id, form_id: current_form.id) + # route page is condition check page + @route_page = PageRepository.find(page_id: @page_goto_conditions.first.check_page_id, form_id: current_form.id) elsif @page_goto_conditions.any? @routing = :end_of_route - # route owner is condition routing page - @route_owner = PageRepository.find(page_id: @page_goto_conditions.first.routing_page_id, form_id: current_form.id) + # route page is condition routing page + @route_page = PageRepository.find(page_id: @page_goto_conditions.first.routing_page_id, form_id: current_form.id) end @delete_confirmation_input = Forms::DeleteConfirmationInput.new diff --git a/app/views/pages/delete.html.erb b/app/views/pages/delete.html.erb index 8de33d382..8a20a59ff 100644 --- a/app/views/pages/delete.html.erb +++ b/app/views/pages/delete.html.erb @@ -11,8 +11,8 @@ t( ".notification_banner.#{@routing}.html", - route_owner_position: @route_owner.position, - show_routes_href: show_routes_path(current_form.id, @route_owner.id), + route_page_position: @route_page.position, + show_routes_href: show_routes_path(current_form.id, @route_page.id), ) end %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 3fe9be784..a6f8f72ba 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1155,14 +1155,14 @@ en: heading_text: Question %{page_position} is at the end of a route html: |

- Question %{route_owner_position}’s route - goes to this question. If you delete this question, question %{route_owner_position}’s routes will also be deleted. + Question %{route_page_position}’s route + goes to this question. If you delete this question, question %{route_page_position}’s routes will also be deleted.

end_of_secondary_skip_route: heading_text: Question %{page_position} is at the end of a route html: |

- Question %{route_owner_position}’s route + Question %{route_page_position}’s route goes to this question. If you delete this question, the route to it will also be deleted.

start_of_route: @@ -1170,13 +1170,13 @@ en: html: |

If you delete this question, its routes will also be deleted. - View question %{route_owner_position}’s routes. + View question %{route_page_position}’s routes.

start_of_secondary_skip_route: heading_text: Question %{page_position} is the start of a route html: |

- Question %{route_owner_position}’s route + Question %{route_page_position}’s route starts at this question. If you delete this question, the route from it will also be deleted.

title: Are you sure you want to delete this question? diff --git a/spec/views/pages/delete.html.erb_spec.rb b/spec/views/pages/delete.html.erb_spec.rb index 30bbc9eff..0fc6b52b0 100644 --- a/spec/views/pages/delete.html.erb_spec.rb +++ b/spec/views/pages/delete.html.erb_spec.rb @@ -67,7 +67,7 @@ before do assign(:routing, :start_of_route) - assign(:route_owner, page) + assign(:route_page, page) render locals: { current_form: } end @@ -122,7 +122,7 @@ before do assign(:routing, :end_of_route) - assign(:route_owner, routing_page) + assign(:route_page, routing_page) assign(:page_goto_conditions, routing_page.routing_conditions) @@ -182,7 +182,7 @@ before do assign(:routing, :start_of_secondary_skip_route) - assign(:route_owner, check_page) + assign(:route_page, check_page) render locals: { current_form: } end @@ -250,7 +250,7 @@ before do assign(:page_goto_conditions, routing_page.routing_conditions) assign(:routing, :end_of_secondary_skip_route) - assign(:route_owner, check_page) + assign(:route_page, check_page) render locals: { current_form: } end From 95c1e9b3e4989e0ca968c851421445e2df07bea8 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Fri, 21 Feb 2025 13:16:49 +0200 Subject: [PATCH 6/6] Use question_number, question_text, and route_number consistently --- .../page_list_component/error_summary/view.rb | 2 +- .../page_list_component/view.html.erb | 6 +- app/components/page_list_component/view.rb | 10 +-- .../pages/conditions_controller.rb | 6 +- app/controllers/pages/routes_controller.rb | 2 +- app/controllers/pages_controller.rb | 2 +- .../pages/secondary_skip_input.rb | 4 +- .../route_summary_card_data_presenter.rb | 8 +- app/services/page_options_service.rb | 6 +- app/views/pages/conditions/delete.html.erb | 4 +- app/views/pages/conditions/edit.html.erb | 8 +- app/views/pages/conditions/new.html.erb | 8 +- app/views/pages/delete.html.erb | 4 +- app/views/pages/routes/delete.html.erb | 10 +-- app/views/pages/routes/show.html.erb | 8 +- .../pages/secondary_skip/delete.html.erb | 6 +- app/views/pages/secondary_skip/edit.html.erb | 4 +- app/views/pages/secondary_skip/new.html.erb | 6 +- app/views/pages/selection/options.html.erb | 2 +- config/locales/en.yml | 80 +++++++++---------- .../error_summary/view_spec.rb | 12 +-- .../page_list_component/view_spec.rb | 10 +-- .../pages/conditions_controller_spec.rb | 6 +- spec/services/page_options_service_spec.rb | 4 +- .../pages/conditions/delete.html.erb_spec.rb | 4 +- .../pages/routes/delete.html.erb_spec.rb | 8 +- spec/views/pages/routes/show.html.erb_spec.rb | 2 +- .../pages/secondary_skip/new.html.erb_spec.rb | 2 +- 28 files changed, 117 insertions(+), 117 deletions(-) diff --git a/app/components/page_list_component/error_summary/view.rb b/app/components/page_list_component/error_summary/view.rb index 1a91ac7aa..d8b82b4ec 100644 --- a/app/components/page_list_component/error_summary/view.rb +++ b/app/components/page_list_component/error_summary/view.rb @@ -11,7 +11,7 @@ def self.error_id(number) end def error_object(error_name:, condition:, page:) - OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", page_index: page.position), link: "##{self.class.error_id(condition.id)}") + OpenStruct.new(message: I18n.t("page_conditions.errors.#{error_name}", question_number: page.position), link: "##{self.class.error_id(condition.id)}") end def conditions_with_routing_pages diff --git a/app/components/page_list_component/view.html.erb b/app/components/page_list_component/view.html.erb index 5da7762a8..a6c8d2e01 100644 --- a/app/components/page_list_component/view.html.erb +++ b/app/components/page_list_component/view.html.erb @@ -35,14 +35,14 @@ <% conditions_for_page_with_index(page.id).each do |condition, _route_index| %>
">
- <%= t("page_conditions.condition_name", page_index: condition_page_position(condition)) %> + <%= t("page_conditions.condition_name", question_number: condition_page_position(condition)) %>
    <% condition.validation_errors.each do |error| %>
  • - <%= t("page_conditions.errors.#{error.name}", page_index: page.position) %> + <%= t("page_conditions.errors.#{error.name}", question_number: page.position) %>
    • <% end %>
@@ -53,7 +53,7 @@
<%= govuk_link_to show_routes_path(form_id: @form_id, page_id: condition.check_page_id) do %> - <%= t("forms.form_overview.edit_with_visually_hidden_text_html", visually_hidden_text: t("page_conditions.condition_name", page_index: page.position)) %> + <%= t("forms.form_overview.edit_with_visually_hidden_text_html", visually_hidden_text: t("page_conditions.condition_name", question_number: page.position)) %> <% end %>
diff --git a/app/components/page_list_component/view.rb b/app/components/page_list_component/view.rb index a1d62447d..2c05f4667 100644 --- a/app/components/page_list_component/view.rb +++ b/app/components/page_list_component/view.rb @@ -22,15 +22,15 @@ def page_row_id(record) def condition_description(condition) if condition.secondary_skip? - I18n.t("page_conditions.secondary_skip_description", check_page_text: skip_condition_route_page_text(condition), goto_page_text: goto_page_text_for_condition(condition)) + I18n.t("page_conditions.secondary_skip_description", check_page_question_text: skip_condition_route_page_text(condition), goto_page_question_text: goto_page_text_for_condition(condition)) else - I18n.t("page_conditions.condition_description", check_page_text: condition_check_page_text(condition), goto_page_text: goto_page_text_for_condition(condition), answer_value: answer_value_text_for_condition(condition)) + I18n.t("page_conditions.condition_description", check_page_question_text: condition_check_page_text(condition), goto_page_question_text: goto_page_text_for_condition(condition), answer_value: answer_value_text_for_condition(condition)) end end def condition_check_page_text(condition) check_page = @pages.find { |page| page.id == condition.check_page_id } - I18n.t("page_conditions.condition_check_page_text", check_page_text: check_page.question_text) + I18n.t("page_conditions.condition_check_page_text", check_page_question_text: check_page.question_text) end def answer_value_text_for_condition(condition) @@ -45,7 +45,7 @@ def answer_value_text_for_condition(condition) def goto_page_text_for_condition(condition) if condition.goto_page_id.present? goto_page = @pages.find { |page| page.id == condition.goto_page_id } - I18n.t("page_conditions.condition_goto_page_text", goto_page_position: goto_page.position, goto_page_text: goto_page.question_text) + I18n.t("page_conditions.condition_goto_page_text", goto_page_question_number: goto_page.position, goto_page_question_text: goto_page.question_text) elsif condition.skip_to_end I18n.t("page_conditions.condition_goto_page_check_your_answers") else @@ -72,7 +72,7 @@ def routing_conditions_with_index def skip_condition_route_page_text(condition) routing_page = @pages.find { |page| page.id == condition.routing_page_id } - I18n.t("page_conditions.skip_condition_route_page_text", route_page_text: routing_page.question_text, route_page_position: routing_page.position) + I18n.t("page_conditions.skip_condition_route_page_text", route_page_question_text: routing_page.question_text, route_page_question_number: routing_page.position) end # Create hash of page_id => [condition, index] diff --git a/app/controllers/pages/conditions_controller.rb b/app/controllers/pages/conditions_controller.rb index de84a5e88..4909e5b60 100644 --- a/app/controllers/pages/conditions_controller.rb +++ b/app/controllers/pages/conditions_controller.rb @@ -27,7 +27,7 @@ def create condition_input = Pages::ConditionsInput.new(condition_input_params) if condition_input.submit - redirect_to show_routes_path(form_id: current_form.id, page_id: page.id), success: t("banner.success.route_created", question_position: condition_input.page.position) + redirect_to show_routes_path(form_id: current_form.id, page_id: page.id), success: t("banner.success.route_created", question_number: condition_input.page.position) else render template: "pages/conditions/new", locals: { condition_input: }, status: :unprocessable_entity end @@ -51,7 +51,7 @@ def update condition_input = Pages::ConditionsInput.new(form_params) if condition_input.update_condition - redirect_to show_routes_path(form_id: current_form.id, page_id: page.id), success: t("banner.success.route_updated", question_position: condition_input.page.position) + redirect_to show_routes_path(form_id: current_form.id, page_id: page.id), success: t("banner.success.route_updated", question_number: condition_input.page.position) else render template: "pages/conditions/edit", locals: { condition_input: }, status: :unprocessable_entity end @@ -74,7 +74,7 @@ def destroy if delete_condition_input.submit if delete_condition_input.confirmed? - redirect_to form_pages_path(current_form.id, page.id), success: t("banner.success.route_deleted", question_position: delete_condition_input.page.position) + redirect_to form_pages_path(current_form.id, page.id), success: t("banner.success.route_deleted", question_number: delete_condition_input.page.position) else redirect_to edit_condition_path(current_form.id, page.id, condition.id) end diff --git a/app/controllers/pages/routes_controller.rb b/app/controllers/pages/routes_controller.rb index 07229ba21..10c0d4174 100644 --- a/app/controllers/pages/routes_controller.rb +++ b/app/controllers/pages/routes_controller.rb @@ -20,7 +20,7 @@ def destroy if @delete_confirmation_input.submit if @delete_confirmation_input.confirmed? - return redirect_to form_pages_path, success: t("banner.success.page_routes_deleted", question_position: current_form.page_number(page)) + return redirect_to form_pages_path, success: t("banner.success.page_routes_deleted", question_number: current_form.page_number(page)) end else return render :delete, locals: { current_form:, page:, delete_confirmation_input: @delete_confirmation_input }, status: :unprocessable_entity diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index c400d5e38..0bf6bb4f3 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -85,7 +85,7 @@ def move_page page_to_move.position + 1 end - redirect_to form_pages_path, success: t("banner.success.form.page_moved", question_text: page_to_move.question_text, direction: move_params[:direction], position:) + redirect_to form_pages_path, success: t("banner.success.form.page_moved", question_text: page_to_move.question_text, direction: move_params[:direction], question_number: position) end private diff --git a/app/input_objects/pages/secondary_skip_input.rb b/app/input_objects/pages/secondary_skip_input.rb index 3db0ce50a..ad1207bb1 100644 --- a/app/input_objects/pages/secondary_skip_input.rb +++ b/app/input_objects/pages/secondary_skip_input.rb @@ -59,9 +59,9 @@ def question_name(page_id) target_page = FormRepository.pages(form).find { |page| page.id == page_id } question_text = target_page.question_text - page_position = target_page.position + question_number = target_page.position - I18n.t("page_route_card.question_name_long", page_position:, question_text:) + I18n.t("page_route_card.question_name_long", question_number:, question_text:) end def end_page_name diff --git a/app/presenters/route_summary_card_data_presenter.rb b/app/presenters/route_summary_card_data_presenter.rb index 8c8d7ef6a..bbab9ce2f 100644 --- a/app/presenters/route_summary_card_data_presenter.rb +++ b/app/presenters/route_summary_card_data_presenter.rb @@ -32,12 +32,12 @@ def conditional_route_cards conditional_routes.map.with_index(1) { |routing_condition, index| conditional_route_card(routing_condition, index) } end - def conditional_route_card(routing_condition, index) + def conditional_route_card(routing_condition, route_number) goto_page_name = routing_condition.skip_to_end ? end_page_name : goto_question_name(routing_condition.goto_page_id) { card: { - title: I18n.t("page_route_card.route_title", index:), + title: I18n.t("page_route_card.route_title", route_number:), classes: "app-summary-card", actions: [ govuk_link_to(I18n.t("page_route_card.edit"), edit_condition_path(form_id: form.id, page_id: page.id, condition_id: routing_condition.id)), @@ -127,9 +127,9 @@ def question_name(page_id) return if target_page.blank? question_text = target_page.question_text - page_position = target_page.position + question_number = target_page.position - I18n.t("page_route_card.question_name_long", page_position:, question_text:) + I18n.t("page_route_card.question_name_long", question_number:, question_text:) end def goto_question_name(page_id) diff --git a/app/services/page_options_service.rb b/app/services/page_options_service.rb index eac521128..49f1151ca 100644 --- a/app/services/page_options_service.rb +++ b/app/services/page_options_service.rb @@ -158,10 +158,10 @@ def print_route(condition) I18n.t("page_conditions.condition_compact_html_end_of_form", answer_value:).html_safe else goto_question = @pages.find { |page| page.id == condition.goto_page_id } - goto_page_text = ActionController::Base.helpers.sanitize(goto_question.question_text) - goto_page_number = @pages.find_index(goto_question) + 1 + goto_page_question_text = ActionController::Base.helpers.sanitize(goto_question.question_text) + goto_page_question_number = @pages.find_index(goto_question) + 1 - I18n.t("page_conditions.condition_compact_html", answer_value:, goto_page_number:, goto_page_text:).html_safe + I18n.t("page_conditions.condition_compact_html", answer_value:, goto_page_question_number:, goto_page_question_text:).html_safe end end diff --git a/app/views/pages/conditions/delete.html.erb b/app/views/pages/conditions/delete.html.erb index 1a6f4f6d6..e1e2ab771 100644 --- a/app/views/pages/conditions/delete.html.erb +++ b/app/views/pages/conditions/delete.html.erb @@ -1,4 +1,4 @@ -<% set_page_title(title_with_error_prefix(t("page_titles.routing_page_delete", question_position: delete_condition_input.page.position), delete_condition_input.errors&.any?)) %> +<% set_page_title(title_with_error_prefix(t("page_titles.routing_page_delete", question_number: delete_condition_input.page.position), delete_condition_input.errors&.any?)) %> <% content_for :back_link, govuk_back_link_to(edit_condition_path(delete_condition_input.form.id, delete_condition_input.page.id, delete_condition_input.record.id)) %>
@@ -14,7 +14,7 @@

<%= delete_condition_input.form.name %> - <%= t("page_titles.routing_page_delete", question_position: delete_condition_input.page.position) %> + <%= t("page_titles.routing_page_delete", question_number: delete_condition_input.page.position) %>

<%= govuk_summary_list(actions: false) do |summary_list| diff --git a/app/views/pages/conditions/edit.html.erb b/app/views/pages/conditions/edit.html.erb index b8ea2a94b..83a1331de 100644 --- a/app/views/pages/conditions/edit.html.erb +++ b/app/views/pages/conditions/edit.html.erb @@ -1,5 +1,5 @@ -<%# TODO: route_position is hardcoded as 1 here because we know there are only two conditions. It will need to change in the future %> -<% set_page_title(title_with_error_prefix(t('page_titles.routing_page_edit', question_position: condition_input.page.position, route_position: 1), condition_input.errors&.any?)) %> +<%# TODO: route_number is hardcoded as 1 here because we know there can be only two conditions. It will need to change in the future %> +<% set_page_title(title_with_error_prefix(t('page_titles.routing_page_edit', question_number: condition_input.page.position, route_number: 1), condition_input.errors&.any?)) %> <% content_for :back_link, govuk_back_link_to(show_routes_path(condition_input.form.id, page_id: condition_input.page.id), t(".back_link", question_number: condition_input.page.position)) %>
@@ -10,8 +10,8 @@ <% end %>

- <%= t("page_titles.routing_page_caption", question_position: condition_input.page.position) %> - <%= t("page_titles.routing_page_edit", question_position: condition_input.page.position, route_position: 1) %> + <%= t("page_titles.routing_page_caption", question_number: condition_input.page.position) %> + <%= t("page_titles.routing_page_edit", question_number: condition_input.page.position, route_number: 1) %>

<%= govuk_summary_list(actions: false) do |summary_list| diff --git a/app/views/pages/conditions/new.html.erb b/app/views/pages/conditions/new.html.erb index 0339431a5..83083492e 100644 --- a/app/views/pages/conditions/new.html.erb +++ b/app/views/pages/conditions/new.html.erb @@ -1,5 +1,5 @@ -<%# TODO: route_position is hardcoded as 1 here because we know there are only two conditions. It will need to change in the future %> -<% set_page_title(title_with_error_prefix(t('page_titles.routing_page_new', route_position: 1), condition_input.errors&.any?)) %> +<%# TODO: route_number is hardcoded as 1 here because we know there can be only two conditions. It will need to change in the future %> +<% set_page_title(title_with_error_prefix(t('page_titles.routing_page_new', route_number: 1), condition_input.errors&.any?)) %> <% content_for :back_link, govuk_back_link_to(routing_page_path(condition_input.form.id, routing_page_id: condition_input.page.id)) %>
@@ -10,8 +10,8 @@ <% end %>

- <%= t("page_titles.routing_page_caption", question_position: condition_input.page.position) %> - <%= t("page_titles.routing_page_new", route_position: 1) %> + <%= t("page_titles.routing_page_caption", question_number: condition_input.page.position) %> + <%= t("page_titles.routing_page_new", route_number: 1) %>

<%= govuk_summary_list do |summary_list| diff --git a/app/views/pages/delete.html.erb b/app/views/pages/delete.html.erb index 8a20a59ff..69f33a751 100644 --- a/app/views/pages/delete.html.erb +++ b/app/views/pages/delete.html.erb @@ -7,11 +7,11 @@ <% unless @delete_confirmation_input.errors.any? %> <% if @routing.present? %> <%= govuk_notification_banner(title_text: "Important") do |banner| - banner.with_heading(text: t(".notification_banner.#{@routing}.heading_text", page_position: @page.position), tag: :h3) + banner.with_heading(text: t(".notification_banner.#{@routing}.heading_text", question_number: @page.position), tag: :h3) t( ".notification_banner.#{@routing}.html", - route_page_position: @route_page.position, + route_page_question_number: @route_page.position, show_routes_href: show_routes_path(current_form.id, @route_page.id), ) end %> diff --git a/app/views/pages/routes/delete.html.erb b/app/views/pages/routes/delete.html.erb index b0779ea36..f720f3f9f 100644 --- a/app/views/pages/routes/delete.html.erb +++ b/app/views/pages/routes/delete.html.erb @@ -1,14 +1,14 @@ -<% page_number = current_form.page_number(page) %> -<% set_page_title(title_with_error_prefix(t(".title", page_number: page_number), delete_confirmation_input.errors.any?)) %> -<% content_for :back_link, govuk_back_link_to(show_routes_path(form_id: current_form.id, page_id: page.id), t("pages.routes.delete.back", page_number: page_number)) %> +<% question_number = current_form.page_number(page) %> +<% set_page_title(title_with_error_prefix(t(".title", question_number:), delete_confirmation_input.errors.any?)) %> +<% content_for :back_link, govuk_back_link_to(show_routes_path(form_id: current_form.id, page_id: page.id), t("pages.routes.delete.back", question_number:)) %>
<%= render( delete_confirmation_input, url: destroy_routes_path(current_form.id, page.id), - caption_text: t("pages.routes.delete.caption", page_number: page_number), - legend_text: t("pages.routes.delete.title", page_number: page_number), + caption_text: t("pages.routes.delete.caption", question_number:), + legend_text: t("pages.routes.delete.title", question_number:), hint_text: nil, ) %>
diff --git a/app/views/pages/routes/show.html.erb b/app/views/pages/routes/show.html.erb index 8513dcde0..863566146 100644 --- a/app/views/pages/routes/show.html.erb +++ b/app/views/pages/routes/show.html.erb @@ -1,4 +1,4 @@ -<% set_page_title(title_with_error_prefix(t('page_titles.routes_show', position: page.position), false)) %> +<% set_page_title(title_with_error_prefix(t('page_titles.routes_show', question_number: page.position), false)) %> <% content_for :back_link, govuk_back_link_to(back_link_url, t('pages.go_to_your_questions')) %>
@@ -6,12 +6,12 @@

<%= current_form.name %> - <%= t("page_titles.routes_show", position: page.position) %> + <%= t("page_titles.routes_show", question_number: page.position) %>

<%= govuk_summary_list(actions: false) do |summary_list| summary_list.with_row do |row| - row.with_key { t("page_route_card.question_name_short", position: page.position) } + row.with_key { t("page_route_card.question_name_short", question_number: page.position) } row.with_value { "#{page.question_text}" } end; end %> @@ -23,7 +23,7 @@ <% if routes.map(&:secondary_skip?).none? %>

<%= t(".any_other_answer.heading") %>

-

<%= t(".any_other_answer.will_continue_to", next_page_position: next_page.position) %>

+

<%= t(".any_other_answer.will_continue_to", next_question_number: next_page.position) %>

<% if FeatureService.new(group: current_form.group).enabled? :branch_routing %>

<%= t(".any_other_answer.skip_later") %>

diff --git a/app/views/pages/secondary_skip/delete.html.erb b/app/views/pages/secondary_skip/delete.html.erb index 34e771423..2d38ea58a 100644 --- a/app/views/pages/secondary_skip/delete.html.erb +++ b/app/views/pages/secondary_skip/delete.html.erb @@ -1,12 +1,12 @@ <% set_page_title(title_with_error_prefix(t('page_titles.delete_secondary_skip'), false)) %> -<% content_for :back_link, govuk_back_link_to(back_link_url, t('secondary_skip.new.back', page_position: delete_secondary_skip_input.page.position)) %> +<% content_for :back_link, govuk_back_link_to(back_link_url, t('secondary_skip.new.back', question_number: delete_secondary_skip_input.page.position)) %> <%= form_with(model: delete_secondary_skip_input, url: destroy_secondary_skip_path(form_id: delete_secondary_skip_input.form.id, page_id: delete_secondary_skip_input.page.id), method: :delete) do |f| %> <% if delete_secondary_skip_input.errors.any? %> <%= f.govuk_error_summary %> <% end %> - <%= t("secondary_skip.new.caption", page_position: delete_secondary_skip_input.page.position) %> + <%= t("secondary_skip.new.caption", question_number: delete_secondary_skip_input.page.position) %> <%= f.govuk_collection_radio_buttons :confirm, delete_secondary_skip_input.values, ->(option) { option }, @@ -15,4 +15,4 @@ %> <%= f.govuk_submit t("save_and_continue") %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/pages/secondary_skip/edit.html.erb b/app/views/pages/secondary_skip/edit.html.erb index b0cf85cd8..f8ee6a198 100644 --- a/app/views/pages/secondary_skip/edit.html.erb +++ b/app/views/pages/secondary_skip/edit.html.erb @@ -1,9 +1,9 @@ <% set_page_title(title_with_error_prefix(t('page_titles.new_secondary_skip', route_index: 2), false)) %> -<% content_for :back_link, govuk_back_link_to(back_link_url, t('secondary_skip.new.back', page_position: secondary_skip_input.page.position)) %> +<% content_for :back_link, govuk_back_link_to(back_link_url, t('secondary_skip.new.back', question_number: secondary_skip_input.page.position)) %> <% content_for :heading do %>

- <%= t("secondary_skip.new.caption", page_position: secondary_skip_input.page.position) %> + <%= t("secondary_skip.new.caption", question_number: secondary_skip_input.page.position) %> <%= t("page_titles.new_secondary_skip") %>

<% end %> diff --git a/app/views/pages/secondary_skip/new.html.erb b/app/views/pages/secondary_skip/new.html.erb index 0754422a0..9e2692233 100644 --- a/app/views/pages/secondary_skip/new.html.erb +++ b/app/views/pages/secondary_skip/new.html.erb @@ -1,11 +1,11 @@ <% set_page_title(title_with_error_prefix(t('page_titles.new_secondary_skip', route_index: 2), false)) %> -<% content_for :back_link, govuk_back_link_to(back_link_url, t('secondary_skip.new.back', page_position: secondary_skip_input.page.position)) %> +<% content_for :back_link, govuk_back_link_to(back_link_url, t('secondary_skip.new.back', question_number: secondary_skip_input.page.position)) %> <% content_for :heading do %>

- <%= t("secondary_skip.new.caption", page_position: secondary_skip_input.page.position) %> + <%= t("secondary_skip.new.caption", question_number: secondary_skip_input.page.position) %> <%= t("page_titles.new_secondary_skip", route_index: 2) %> - <%# TODO: route_index is hardcoded as 2 here because we know there are only two conditions. It will need to change in the future %> + <%# TODO: route_number is hardcoded as 2 here because we know there can be only two conditions. It will need to change in the future %>

<% end %> diff --git a/app/views/pages/selection/options.html.erb b/app/views/pages/selection/options.html.erb index 584b5ff31..2b702e650 100644 --- a/app/views/pages/selection/options.html.erb +++ b/app/views/pages/selection/options.html.erb @@ -32,7 +32,7 @@ <% @selection_options_input.selection_options_form_objects.each_with_index do |selection_option, index| %> <%= f.fields_for :selection_options, selection_option, index: index do |selection_options_form| %>
  • - <%= selection_options_form.govuk_text_field :name, id: "forms-selections-settings-form-selection-options-name-field-#{index}", label: { text: t("selection_options.option", number: index + 1), for: "forms-selections-settings-form-selection-options-name-field-#{index}" }, class: "govuk-input--width-20", form_group: { classes: "app-select-options__form-group" } %> + <%= selection_options_form.govuk_text_field :name, id: "forms-selections-settings-form-selection-options-name-field-#{index}", label: { text: t("selection_options.option", option_number: index + 1), for: "forms-selections-settings-form-selection-options-name-field-#{index}" }, class: "govuk-input--width-20", form_group: { classes: "app-select-options__form-group" } %> <%= f.govuk_submit t("selection_options.remove_html", option_number: index + 1), class: "app-select-options__button", name: :remove, value: index, secondary: true %>
    • <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index a6f8f72ba..2e44c0556 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -186,7 +186,7 @@ en: change_name: Your form name has been saved declaration_saved: Your declaration has been saved declaration_saved_and_completed: Your declaration has been saved and marked as complete - page_moved: "‘%{question_text}’ has moved %{direction} to number %{position}" + page_moved: "‘%{question_text}’ has moved %{direction} to number %{question_number}" pages_saved: Your questions have been saved pages_saved_and_section_completed: Your questions have been saved and marked as complete payment_link_saved: Your payment link has been saved @@ -196,10 +196,10 @@ en: share_preview_completed: The preview task has been completed support_details_saved: Your contact details for support have been saved what_happens_next_saved: Your information about what happens next has been saved - page_routes_deleted: Question %{question_position}’s routes have been deleted - route_created: Question %{question_position}’s route has been created - route_deleted: Question %{question_position}’s route has been deleted - route_updated: Question %{question_position}’s route has been updated + page_routes_deleted: Question %{question_number}’s routes have been deleted + route_created: Question %{question_number}’s route has been created + route_deleted: Question %{question_number}’s route has been deleted + route_updated: Question %{question_number}’s route has been updated title: Success bulk_options: hint: Enter each option on a new line @@ -1014,23 +1014,23 @@ en: check_your_answers: Check your answers before submitting condition_answer_value_text: "“%{answer_value}”" condition_answer_value_text_with_errors: "[Answer not selected]" - condition_check_page_text: "“%{check_page_text}”" - condition_compact_html: 'If the answer is “%{answer_value}”
    then skip to %{goto_page_number}: “%{goto_page_text}”' + condition_check_page_text: "“%{check_page_question_text}”" + condition_compact_html: 'If the answer is “%{answer_value}”
    then skip to %{goto_page_question_number}: “%{goto_page_question_text}”' condition_compact_html_end_of_form: If the answer is “%{answer_value}”
    then skip to “Check your answers before submitting” - condition_description: If %{check_page_text} is answered as %{answer_value} go to %{goto_page_text} + condition_description: If %{check_page_question_text} is answered as %{answer_value} go to %{goto_page_question_text} condition_goto_page_check_your_answers: "“Check your answers before submitting”." - condition_goto_page_text: "%{goto_page_position}, “%{goto_page_text}”" + condition_goto_page_text: "%{goto_page_question_number}, “%{goto_page_question_text}”" condition_goto_page_text_with_errors: "[Question not selected]" - condition_name: Question %{page_index}’s routes + condition_name: Question %{question_number}’s routes errors: - answer_value_doesnt_exist: The answer that question %{page_index}’s route is based on no longer exists - edit question %{page_index}’s route - cannot_have_goto_page_before_routing_page: The question you’re taking the person to is now above the route - edit question %{page_index}’s route - cannot_route_to_next_page: Question %{page_index}’s route is now next to the route’s end question - this makes the route unnecessary. Edit question %{page_index}’s route. - goto_page_doesnt_exist: The question you’re taking the person to no longer exists - edit question %{page_index}’s route + 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_text} go to %{goto_page_text} - skip_condition_route_page_text: "%{route_page_position}, “%{route_page_text}”" + secondary_skip_description: After %{check_page_question_text} go to %{goto_page_question_text} + skip_condition_route_page_text: "%{route_page_question_number}, “%{route_page_question_text}”" page_options_service: answer_type: Answer type date: Date @@ -1055,9 +1055,9 @@ en: edit: Edit goto_page_invalid: Edit this route to select the question or page you want to take the person to, or delete the route if_answer_is: If the answer is - question_name_long: "%{page_position}. %{question_text}" - question_name_short: Question %{position} - route_title: Route %{index} + question_name_long: "%{question_number}. %{question_text}" + question_name_short: Question %{question_number} + route_title: Route %{route_number} secondary_skip_after: Then after the person answers secondary_skip_then: take them to set_secondary_skip: Set one or more questions to skip later in the form (optional) @@ -1123,12 +1123,12 @@ en: payment_link: Add a link to a payment page on GOV.UK Pay privacy_policy: Provide a link to privacy information for this form question_text: What’s your question? - routes_show: Question %{position}’s routes + routes_show: Question %{question_number}’s routes routing_page: Add a route from a question - routing_page_caption: Question %{question_position}’s routes - routing_page_delete: Delete question %{question_position}’s route - routing_page_edit: Edit route %{route_position} - routing_page_new: Add route %{route_position} + routing_page_caption: Question %{question_number}’s routes + routing_page_delete: Delete question %{question_number}’s route + routing_page_edit: Edit route %{route_number} + routing_page_new: Add route %{route_number} selection_options: Create a list of options selection_type: How many options should people be able to select? separator: " – " @@ -1152,31 +1152,31 @@ en: delete: notification_banner: end_of_route: - heading_text: Question %{page_position} is at the end of a route + heading_text: Question %{question_number} is at the end of a route html: |

    - Question %{route_page_position}’s route - goes to this question. If you delete this question, question %{route_page_position}’s routes will also be deleted. + Question %{route_page_question_number}’s route + goes to this question. If you delete this question, question %{route_page_question_number}’s routes will also be deleted.

    end_of_secondary_skip_route: - heading_text: Question %{page_position} is at the end of a route + heading_text: Question %{question_number} is at the end of a route html: |

    - Question %{route_page_position}’s route + Question %{route_page_question_number}’s route goes to this question. If you delete this question, the route to it will also be deleted.

    start_of_route: - heading_text: Question %{page_position} is the start of a route + heading_text: Question %{question_number} is the start of a route html: |

    If you delete this question, its routes will also be deleted. - View question %{route_page_position}’s routes. + View question %{route_page_question_number}’s routes.

    start_of_secondary_skip_route: - heading_text: Question %{page_position} is the start of a route + heading_text: Question %{question_number} is the start of a route html: |

    - Question %{route_page_position}’s route + Question %{route_page_question_number}’s route starts at this question. If you delete this question, the route from it will also be deleted.

    title: Are you sure you want to delete this question? @@ -1193,15 +1193,15 @@ en: question: Question routes: delete: - back: Back to question %{page_number}’s routes - caption: Question %{page_number}’s routes - title: Are you sure you want to delete all question %{page_number}’s routes? + back: Back to question %{question_number}’s routes + caption: Question %{question_number}’s routes + title: Are you sure you want to delete all question %{question_number}’s routes? show: any_other_answer: heading: Any other answer set_questions_to_skip: Set questions to skip skip_later: If you need to, you can make these people skip one or more questions later in the form. For example, you might want them to skip route 1’s questions to create 2 different ‘branches’ of questions. - will_continue_to: People who select any other answer will continue to question %{next_page_position}. + will_continue_to: People who select any other answer will continue to question %{next_question_number}. submit_save: Save question payment_link_input: body_html: | @@ -1426,8 +1426,8 @@ en: save_and_continue: Save and continue secondary_skip: new: - back: Back to question %{page_position}’s routes - caption: Question %{page_position}’s routes + back: Back to question %{question_number}’s routes + caption: Question %{question_number}’s routes details_html: |

    People who select an answer that does not have a route assigned to it will continue to the next question.

    If you need to, you can make these people skip one or more questions later in the form. For example, you might want them to skip route 1’s questions. This allows you to create 2 different ‘branches’ of questions.

    @@ -1441,7 +1441,7 @@ en: add_options_below_or_in_text_box_html: You can add your options individually below, or you can enter all the options into one text box. cannot_add_more_options: You cannot add any more options as you have reached the maximum of %{maximum} options. enter_all_options_into_textbox: Enter all the options into one text box - option: Option %{number} + option: Option %{option_number} remove_html: Remove option %{option_number} select_more_than_one_option: People will be able to select one or more options from your list. select_one_option: People will be able to select one option from your list. 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 bc074dd64..c2d94d27c 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", page_index: 1) + condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_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", page_index: 1) - condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 2) + 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) 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 @@ -102,7 +102,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", page_index: 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), link: "##{described_class.error_id(1)}") end end @@ -115,8 +115,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", page_index: 1), link: "##{described_class.error_id(1)}"), - OpenStruct.new(message: I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 2), link: "##{described_class.error_id(2)}"), + 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)}"), ] end end diff --git a/spec/components/page_list_component/view_spec.rb b/spec/components/page_list_component/view_spec.rb index 974667248..8347c3d0c 100644 --- a/spec/components/page_list_component/view_spec.rb +++ b/spec/components/page_list_component/view_spec.rb @@ -91,7 +91,7 @@ it "renders the routing details" do condition_answer_value_text = I18n.t("page_conditions.condition_answer_value_text", answer_value: "Wales") - condition_goto_page_text = I18n.t("page_conditions.condition_goto_page_text", goto_page_position: 3, goto_page_text: "What is your pet's name?") + condition_goto_page_text = I18n.t("page_conditions.condition_goto_page_text", goto_page_question_number: 3, goto_page_question_text: "What is your pet's name?") expect(page).to have_css("dd.govuk-summary-list__value", text: condition_answer_value_text) expect(page).to have_css("dd.govuk-summary-list__value", text: condition_goto_page_text) end @@ -105,7 +105,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", page_index: 1) + condition_answer_value_error = I18n.t("page_conditions.errors.answer_value_doesnt_exist", question_number: 1) expect(page).to have_css("ul > li", text: condition_answer_value_error) end @@ -124,7 +124,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", page_index: 1) + condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", question_number: 1) expect(page).to have_css("ul > li", text: condition_goto_page_error) end @@ -143,8 +143,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", page_index: 1) - condition_goto_page_error = I18n.t("page_conditions.errors.goto_page_doesnt_exist", page_index: 1) + 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) 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 diff --git a/spec/requests/pages/conditions_controller_spec.rb b/spec/requests/pages/conditions_controller_spec.rb index 071d7b26b..175920ce7 100644 --- a/spec/requests/pages/conditions_controller_spec.rb +++ b/spec/requests/pages/conditions_controller_spec.rb @@ -155,7 +155,7 @@ it "displays success message" do follow_redirect! - expect(response.body).to include(I18n.t("banner.success.route_created", question_position: selected_page.position)) + expect(response.body).to include(I18n.t("banner.success.route_created", question_number: selected_page.position)) end context "when form submit fails" do @@ -264,7 +264,7 @@ it "displays success message" do follow_redirect! - expect(response.body).to include(I18n.t("banner.success.route_updated", question_position: 1)) + expect(response.body).to include(I18n.t("banner.success.route_updated", question_number: 1)) end context "when form submit fails" do @@ -355,7 +355,7 @@ it "displays success message" do follow_redirect! - expect(response.body).to include(I18n.t("banner.success.route_deleted", question_position: 1)) + expect(response.body).to include(I18n.t("banner.success.route_deleted", question_number: 1)) end context "when confirm deletion is false" do diff --git a/spec/services/page_options_service_spec.rb b/spec/services/page_options_service_spec.rb index 41851bde0..5ec59aac3 100644 --- a/spec/services/page_options_service_spec.rb +++ b/spec/services/page_options_service_spec.rb @@ -215,7 +215,7 @@ expect(page_options_service.all_options_for_answer_type).to include( { key: { text: I18n.t("page_conditions.route") }, - value: { text: I18n.t("page_conditions.condition_compact_html", answer_value: condition_pointing_to_page_3.answer_value, goto_page_number: 3, goto_page_text: pages[2].question_text) }, + value: { text: I18n.t("page_conditions.condition_compact_html", answer_value: condition_pointing_to_page_3.answer_value, goto_page_question_number: 3, goto_page_question_text: pages[2].question_text) }, }, ) end @@ -228,7 +228,7 @@ expect(page_options_service.all_options_for_answer_type).to include( { key: { text: I18n.t("page_conditions.route") }, - value: { text: "
    1. #{I18n.t('page_conditions.condition_compact_html', answer_value: condition_pointing_to_page_3.answer_value, goto_page_number: 3, goto_page_text: pages[2].question_text)}
    2. #{I18n.t('page_conditions.condition_compact_html', answer_value: condition_pointing_to_page_4.answer_value, goto_page_number: 4, goto_page_text: pages[3].question_text)}
    " }, + value: { text: "
    1. #{I18n.t('page_conditions.condition_compact_html', answer_value: condition_pointing_to_page_3.answer_value, goto_page_question_number: 3, goto_page_question_text: pages[2].question_text)}
    2. #{I18n.t('page_conditions.condition_compact_html', answer_value: condition_pointing_to_page_4.answer_value, goto_page_question_number: 4, goto_page_question_text: pages[3].question_text)}
    " }, }, ) end diff --git a/spec/views/pages/conditions/delete.html.erb_spec.rb b/spec/views/pages/conditions/delete.html.erb_spec.rb index 22f9f6525..015fba738 100644 --- a/spec/views/pages/conditions/delete.html.erb_spec.rb +++ b/spec/views/pages/conditions/delete.html.erb_spec.rb @@ -15,12 +15,12 @@ end it "sets the correct title" do - expect(view.content_for(:title)).to eq(t("page_titles.routing_page_delete", question_position: page.position)) + expect(view.content_for(:title)).to eq(t("page_titles.routing_page_delete", question_number: page.position)) end it "contains page heading and sub-heading" do expect(rendered).to have_css("h1 .govuk-caption-l", text: form.name) - expect(rendered).to have_css("h1.govuk-heading-l", text: t("page_titles.routing_page_delete", question_position: page.position)) + expect(rendered).to have_css("h1.govuk-heading-l", text: t("page_titles.routing_page_delete", question_number: page.position)) end it "contains the condition details" do diff --git a/spec/views/pages/routes/delete.html.erb_spec.rb b/spec/views/pages/routes/delete.html.erb_spec.rb index aeb1a0193..698e50173 100644 --- a/spec/views/pages/routes/delete.html.erb_spec.rb +++ b/spec/views/pages/routes/delete.html.erb_spec.rb @@ -9,15 +9,15 @@ end it "has the correct title" do - expect(view.content_for(:title)).to have_content(I18n.t("pages.routes.delete.title", page_number: 1)) + expect(view.content_for(:title)).to have_content(I18n.t("pages.routes.delete.title", question_number: 1)) end it "has the correct back link" do - expect(view.content_for(:back_link)).to have_link(I18n.t("pages.routes.delete.back", page_number: form.page_number(page)), href: show_routes_path(form_id: form.id, page_id: page.id)) + expect(view.content_for(:back_link)).to have_link(I18n.t("pages.routes.delete.back", question_number: form.page_number(page)), href: show_routes_path(form_id: form.id, page_id: page.id)) end it "has the correct heading" do - expect(rendered).to have_selector("h1", text: I18n.t("pages.routes.delete.title", page_number: 1)) + expect(rendered).to have_selector("h1", text: I18n.t("pages.routes.delete.title", question_number: 1)) end it "posts the confirm value to the destroy action" do @@ -30,7 +30,7 @@ end it "has a legend for the radio buttons" do - expect(rendered).to have_css "fieldset legend:has(~ .govuk-radios)", text: I18n.t("pages.routes.delete.title", page_number: 1) + expect(rendered).to have_css "fieldset legend:has(~ .govuk-radios)", text: I18n.t("pages.routes.delete.title", question_number: 1) end it "has a submit button" do diff --git a/spec/views/pages/routes/show.html.erb_spec.rb b/spec/views/pages/routes/show.html.erb_spec.rb index 4eff0c6dd..8dba58300 100644 --- a/spec/views/pages/routes/show.html.erb_spec.rb +++ b/spec/views/pages/routes/show.html.erb_spec.rb @@ -36,7 +36,7 @@ it "has the correct heading and caption" do expect(rendered).to have_selector("h1", text: form.name) - expect(rendered).to have_selector("h1", text: I18n.t("page_titles.routes_show", position: page.position)) + expect(rendered).to have_selector("h1", text: I18n.t("page_titles.routes_show", question_number: page.position)) end it "shows the page title as a summary list" do diff --git a/spec/views/pages/secondary_skip/new.html.erb_spec.rb b/spec/views/pages/secondary_skip/new.html.erb_spec.rb index 5fab99ca1..f3392ec38 100644 --- a/spec/views/pages/secondary_skip/new.html.erb_spec.rb +++ b/spec/views/pages/secondary_skip/new.html.erb_spec.rb @@ -30,7 +30,7 @@ end it "has the correct back link" do - expect(view.content_for(:back_link)).to have_link(I18n.t("secondary_skip.new.back", page_position: 1), href: "/back") + expect(view.content_for(:back_link)).to have_link(I18n.t("secondary_skip.new.back", question_number: 1), href: "/back") end it "has the correct heading and caption" do