From aad39d49e10ab2b90fd4138da71515be5a404612 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Tue, 2 Jun 2026 14:33:22 +0100 Subject: [PATCH] feat: use helpers where required and update misc tests --- .../bulk_upload_error_row_component.rb | 6 ++-- .../bulk_upload_summary_component.rb | 12 +++---- ...eck_answers_summary_list_card_component.rb | 8 ++--- .../create_log_actions_component.rb | 16 ++++----- ...rotection_confirmation_banner_component.rb | 2 +- .../missing_stock_owners_banner_component.rb | 6 ++-- app/components/search_component.html.erb | 2 +- spec/helpers/application_helper_spec.rb | 34 +++++++++++++++---- spec/requests/users_controller_spec.rb | 23 ++++--------- 9 files changed, 59 insertions(+), 50 deletions(-) diff --git a/app/components/bulk_upload_error_row_component.rb b/app/components/bulk_upload_error_row_component.rb index 036bf785a..569f71b85 100644 --- a/app/components/bulk_upload_error_row_component.rb +++ b/app/components/bulk_upload_error_row_component.rb @@ -17,7 +17,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base def tenant_code_html return if tenant_code.blank? - content_tag :span, class: "govuk-!-margin-left-3" do + helpers.content_tag :span, class: "govuk-!-margin-left-3" do "Tenant code: #{tenant_code}" end end @@ -29,7 +29,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base def purchaser_code_html return if purchaser_code.blank? - content_tag :span, class: "govuk-!-margin-left-3" do + helpers.content_tag :span, class: "govuk-!-margin-left-3" do "Purchaser code: #{purchaser_code}" end end @@ -41,7 +41,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base def property_ref_html return if property_ref.blank? - content_tag :span, class: "govuk-!-margin-left-3" do + helpers.content_tag :span, class: "govuk-!-margin-left-3" do "Property reference: #{property_ref}" end end diff --git a/app/components/bulk_upload_summary_component.rb b/app/components/bulk_upload_summary_component.rb index e59fb7db4..beb850736 100644 --- a/app/components/bulk_upload_summary_component.rb +++ b/app/components/bulk_upload_summary_component.rb @@ -27,8 +27,8 @@ class BulkUploadSummaryComponent < ViewComponent::Base return if count.nil? || count <= 0 text = count > 1 ? (plural_text || singular_text.pluralize(count)) : singular_text - content_tag(:p, class: "govuk-!-font-size-16 govuk-!-margin-bottom-1") do - concat(content_tag(:strong, count)) + helpers.content_tag(:p, class: "govuk-!-font-size-16 govuk-!-margin-bottom-1") do + concat(helpers.content_tag(:strong, count)) concat(" #{text}") end end @@ -44,11 +44,11 @@ class BulkUploadSummaryComponent < ViewComponent::Base end def download_lettings_file_link(bulk_upload) - govuk_link_to "Download file", download_lettings_bulk_upload_path(bulk_upload), class: "govuk-link govuk-!-margin-right-2" + helpers.govuk_link_to "Download file", download_lettings_bulk_upload_path(bulk_upload), class: "govuk-link govuk-!-margin-right-2" end def download_sales_file_link(bulk_upload) - govuk_link_to "Download file", download_sales_bulk_upload_path(bulk_upload), class: "govuk-link govuk-!-margin-right-2" + helpers.govuk_link_to "Download file", download_sales_bulk_upload_path(bulk_upload), class: "govuk-link govuk-!-margin-right-2" end def view_error_report_link(bulk_upload) @@ -61,12 +61,12 @@ class BulkUploadSummaryComponent < ViewComponent::Base "bulk_upload_#{bulk_upload.log_type}_result_path" end - govuk_link_to "View error report", send(path, bulk_upload), class: "govuk-link" + helpers.govuk_link_to "View error report", helpers.send(path, bulk_upload), class: "govuk-link" end def view_logs_link(bulk_upload) return unless bulk_upload.status.to_s == "logs_uploaded_with_errors" - govuk_link_to "View logs with errors", send("#{bulk_upload.log_type}_logs_path", bulk_upload_id: [bulk_upload.id]), class: "govuk-link" + helpers.govuk_link_to "View logs with errors", helpers.send("#{bulk_upload.log_type}_logs_path", bulk_upload_id: [bulk_upload.id]), class: "govuk-link" end end diff --git a/app/components/check_answers_summary_list_card_component.rb b/app/components/check_answers_summary_list_card_component.rb index 0aa3f9769..89345a0eb 100644 --- a/app/components/check_answers_summary_list_card_component.rb +++ b/app/components/check_answers_summary_list_card_component.rb @@ -33,16 +33,16 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base def action_href(question, log) referrer = question.displayed_as_answered?(log) ? "check_answers" : "check_answers_new_answer" - send("#{log.log_type}_#{question.page.id}_path", log, referrer:) + helpers.send("#{log.log_type}_#{question.page.id}_path", log, referrer:) end def correct_validation_action_href(question, log, _related_question_ids, correcting_hard_validation) return action_href(question, log) unless correcting_hard_validation if question.displayed_as_answered?(log) - send("#{log.log_type}_confirm_clear_answer_path", log, question_id: question.id) + helpers.send("#{log.log_type}_confirm_clear_answer_path", log, question_id: question.id) else - send("#{log.log_type}_#{question.page.id}_path", log, referrer: "check_errors", related_question_ids: request.query_parameters["related_question_ids"], original_page_id: request.query_parameters["original_page_id"]) + helpers.send("#{log.log_type}_#{question.page.id}_path", log, referrer: "check_errors", related_question_ids: request.query_parameters["related_question_ids"], original_page_id: request.query_parameters["original_page_id"]) end end @@ -55,7 +55,7 @@ private "govuk-link govuk-link--no-visited-state" end - govuk_link_to question.check_answer_prompt, correct_validation_action_href(question, log, nil, @correcting_hard_validation), class: link_class + helpers.govuk_link_to question.check_answer_prompt, correct_validation_action_href(question, log, nil, @correcting_hard_validation), class: link_class end def number_of_buyers diff --git a/app/components/create_log_actions_component.rb b/app/components/create_log_actions_component.rb index ea3e3f7f2..6883f77be 100644 --- a/app/components/create_log_actions_component.rb +++ b/app/components/create_log_actions_component.rb @@ -23,7 +23,7 @@ class CreateLogActionsComponent < ViewComponent::Base end def create_button_href - send("#{log_type}_logs_path") + helpers.send("#{log_type}_logs_path") end def upload_button_copy @@ -31,23 +31,23 @@ class CreateLogActionsComponent < ViewComponent::Base end def upload_button_href - send("bulk_upload_#{log_type}_log_path", id: "start") + helpers.send("bulk_upload_#{log_type}_log_path", id: "start") end def create_test_log_href - send("create_test_#{log_type}_log_path") + helpers.send("create_test_#{log_type}_log_path") end def create_next_year_test_log_href - send("create_next_year_test_#{log_type}_log_path") + helpers.send("create_next_year_test_#{log_type}_log_path") end def create_setup_test_log_href - send("create_setup_test_#{log_type}_log_path") + helpers.send("create_setup_test_#{log_type}_log_path") end def create_next_year_setup_test_log_href - send("create_next_year_setup_test_#{log_type}_log_path") + helpers.send("create_next_year_setup_test_#{log_type}_log_path") end def current_collection_year_label @@ -59,7 +59,7 @@ class CreateLogActionsComponent < ViewComponent::Base end def create_test_bulk_upload_href(year) - send("create_#{year}_test_#{log_type}_bulk_upload_path") + helpers.send("create_#{year}_test_#{log_type}_bulk_upload_path") end def view_uploads_button_copy @@ -67,6 +67,6 @@ class CreateLogActionsComponent < ViewComponent::Base end def view_uploads_button_href - send("bulk_uploads_#{log_type}_logs_path") + helpers.send("bulk_uploads_#{log_type}_logs_path") end end diff --git a/app/components/data_protection_confirmation_banner_component.rb b/app/components/data_protection_confirmation_banner_component.rb index 56260760b..77cc0193b 100644 --- a/app/components/data_protection_confirmation_banner_component.rb +++ b/app/components/data_protection_confirmation_banner_component.rb @@ -31,7 +31,7 @@ class DataProtectionConfirmationBannerComponent < ViewComponent::Base def banner_text if show_no_dpo_message? || user.is_dpo? || !org_or_user_org.holds_own_stock? - govuk_link_to( + helpers.govuk_link_to( link_text, link_href, class: "govuk-notification-banner__link govuk-!-font-weight-bold", diff --git a/app/components/missing_stock_owners_banner_component.rb b/app/components/missing_stock_owners_banner_component.rb index 1da61d8ae..9a72f42e3 100644 --- a/app/components/missing_stock_owners_banner_component.rb +++ b/app/components/missing_stock_owners_banner_component.rb @@ -35,7 +35,7 @@ class MissingStockOwnersBannerComponent < ViewComponent::Base private def add_stock_owner_link - govuk_link_to( + helpers.govuk_link_to( "add a stock owner", stock_owners_add_organisation_path(id: organisation.id), class: "govuk-notification-banner__link govuk-!-font-weight-bold", @@ -43,7 +43,7 @@ private end def contact_helpdesk_link - govuk_link_to( + helpers.govuk_link_to( "contact the helpdesk", GlobalConstants::HELPDESK_URL, class: "govuk-notification-banner__link govuk-!-font-weight-bold", @@ -51,7 +51,7 @@ private end def users_link - govuk_link_to( + helpers.govuk_link_to( "users page", users_path, class: "govuk-notification-banner__link govuk-!-font-weight-bold", diff --git a/app/components/search_component.html.erb b/app/components/search_component.html.erb index 34fb8f345..373b1545e 100644 --- a/app/components/search_component.html.erb +++ b/app/components/search_component.html.erb @@ -1,4 +1,4 @@ -<%= form_with url: path(current_user), method: "get", local: true do |f| %> +<%= helpers.form_with url: path(current_user), method: "get", local: true do |f| %>
<%= f.govuk_text_field :search, form_group: { diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index e6ae564b9..474685e78 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -8,18 +8,38 @@ RSpec.describe ApplicationHelper do let(:pagy) { nil } let(:current_user) { FactoryBot.create(:user) } - describe "govuk_header_classes" do - context "with external user" do - it "shows the standard app header" do - expect(govuk_header_classes(current_user)).to eq("app-header") + describe "govuk_service_navigation_classes" do + context "with non-support user" do + it "returns empty string for blue border (default)" do + expect(govuk_service_navigation_classes(current_user)).to eq("") + end + + context "when notifications are displayed" do + before do + allow(helper).to receive(:notifications_to_display?).and_return(true) + end + + it "returns no-border class to hide the border (notification banner shows instead)" do + expect(govuk_service_navigation_classes(current_user)).to eq("app-service-navigation--no-border") + end end end - context "with internal support user" do + context "with support user" do let(:current_user) { FactoryBot.create(:user, :support) } - it "shows an orange header" do - expect(govuk_header_classes(current_user)).to eq("app-header app-header--orange") + it "always returns orange class for orange border" do + expect(govuk_service_navigation_classes(current_user)).to eq("app-service-navigation--orange") + end + + context "when notifications are displayed" do + before do + allow(helper).to receive(:notifications_to_display?).and_return(true) + end + + it "still returns orange class (support users always see orange border)" do + expect(govuk_service_navigation_classes(current_user)).to eq("app-service-navigation--orange") + end end end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index d67467998..015cbf008 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -76,13 +76,16 @@ RSpec.describe UsersController, type: :request do end describe "title link" do - it "routes user to the home page" do + it "has GOV.UK header and service navigation both linking to home page" do sign_in user get "/", headers:, params: {} expect(path).to eq("/") expect(page).to have_content("Welcome back") - expected_link = "" - expect(CGI.unescape_html(response.body)).to include(expected_link) + + govuk_header_link = '' + expect(CGI.unescape_html(response.body)).to include(govuk_header_link) + + expect(page).to have_css(".govuk-service-navigation__link[href='/']", text: "Submit social housing lettings and sales data (CORE)") end end @@ -2597,18 +2600,4 @@ RSpec.describe UsersController, type: :request do end end end - - describe "title link" do - before do - sign_in user - end - - it "routes user to the home page" do - get "/", headers:, params: {} - expect(path).to eq("/") - expect(page).to have_content("Welcome back") - expected_link = "" - expect(CGI.unescape_html(response.body)).to include(expected_link) - end - end end