From b64d73d942427b284ac7f62bc7bbb583ad333065 Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Tue, 17 Dec 2024 11:46:33 +0000 Subject: [PATCH] Improvements --- app/helpers/locations_helper.rb | 6 +++++- app/helpers/merge_requests_helper.rb | 4 ++-- app/helpers/organisations_helper.rb | 2 +- app/helpers/user_helper.rb | 2 +- app/views/locations/check_answers.html.erb | 2 +- app/views/locations/show.html.erb | 6 +----- app/views/organisations/show.html.erb | 2 +- app/views/users/show.html.erb | 4 ++-- 8 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb index 404e7af0f..0500902f0 100644 --- a/app/helpers/locations_helper.rb +++ b/app/helpers/locations_helper.rb @@ -8,6 +8,10 @@ module LocationsHelper selection_options(%w[Yes No]) end + def location_editable_attributes + %w[postcode name units type_of_unit mobility_standards] + end + def type_of_units_selection selection_options(Location.type_of_units) end @@ -64,7 +68,7 @@ module LocationsHelper send("scheme_location_#{attribute}_path", location.scheme, location, referrer: "check_answers", route: params[:route]) end - def location_action_text_helper(attr, location) + def location_action_text(attr, location) return "" if attr[:value].blank? || (attr[:attribute] == "availability" && location.startdate.blank?) "Change" diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 56fa5dbae..6fa50a7f8 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -20,7 +20,7 @@ module MergeRequestsHelper messages[page] || "Enter #{lowercase_first_letter(page.humanize)}" end - def merge_request_action_text_helper(attribute, merge_request) + def merge_request_action_text(attribute, merge_request) merge_request.send(attribute).present? || (attribute == "helpdesk_ticket" && merge_request.has_helpdesk_ticket == false) ? "Change" : "" end @@ -96,7 +96,7 @@ module MergeRequestsHelper def merge_request_action(merge_request, page, attribute = nil) attribute = page if attribute.nil? unless merge_request.status == "request_merged" || merge_request.status == "processing" - { text: merge_request_action_text_helper(attribute, merge_request), href: send("#{page}_merge_request_path", merge_request, referrer: "check_answers"), visually_hidden_text: page.humanize } + { text: merge_request_action_text(attribute, merge_request), href: send("#{page}_merge_request_path", merge_request, referrer: "check_answers"), visually_hidden_text: page.humanize } end end diff --git a/app/helpers/organisations_helper.rb b/app/helpers/organisations_helper.rb index 3c9dabc77..19c77b357 100644 --- a/app/helpers/organisations_helper.rb +++ b/app/helpers/organisations_helper.rb @@ -55,7 +55,7 @@ module OrganisationsHelper govuk_button_link_to "Delete this organisation", delete_confirmation_organisation_path(organisation), warning: true end - def organisation_action_text_helper(attr, organisation) + def organisation_action_text(attr, organisation) return "" if attr[:value].blank? || (attr[:attribute] == "phone" && organisation.phone.blank?) "Change" diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index 8e8e7fce3..13eab0d14 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -90,7 +90,7 @@ module UserHelper end end - def user_action_text_helper(user, attribute) + def user_action_text(user, attribute) return "Change" if %w[role phone].include?(attribute) && user.send(attribute).present? "" diff --git a/app/views/locations/check_answers.html.erb b/app/views/locations/check_answers.html.erb index a69dd0b1b..089ac37bd 100644 --- a/app/views/locations/check_answers.html.erb +++ b/app/views/locations/check_answers.html.erb @@ -30,7 +30,7 @@ <% row.with_value { details_html(attr, @location) if user_can_edit_scheme?(current_user, @scheme) } %> <% end %> <% if LocationPolicy.new(current_user, @location).update? %> - <% row.with_action(text: location_action_text_helper(attr, @location), href: location_edit_path(@location, attr[:attribute])) %> + <% row.with_action(text: location_action_text(attr, @location), href: location_edit_path(@location, attr[:attribute])) %> <% end %> <% end %> <% end %> diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index cc33061ea..16583c6e5 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -34,11 +34,7 @@ <%= row.with_value { details_html(attr, @location) } %> <% end %> <% if LocationPolicy.new(current_user, @location).update? %> - <% %w[postcode name units type_of_unit mobility_standards].each do |attribute| %> - <% if attr[:attribute] == attribute %> - <%= row.with_action(text: "Change", href: location_action_link(attr, @scheme, @location, current_user)) %> - <% end %> - <% end %> + <%= row.with_action(text: "Change", href: location_action_link(attr, @scheme, @location, current_user)) if location_editable_attributes.include?(attr[:attribute]) %> <% end %> <% end %> <% end %> diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index daad69dfd..2ff37d308 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -24,7 +24,7 @@ <% row.with_key { attr[:name] } %> <% row.with_value { details_html(attr, @organisation) } %> <% row.with_action( - text: organisation_action_text_helper(attr, @organisation), + text: organisation_action_text(attr, @organisation), visually_hidden_text: attr[:name].to_s.humanize.downcase, href: edit_organisation_path(@organisation), html_attributes: { "data-qa": "change-#{attr[:name].downcase}" }, diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index a632bf9bc..a04aa2fa5 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -55,7 +55,7 @@ row.with_value { user_details_html(@user, current_user, "phone") } if UserPolicy.new(current_user, @user).edit_telephone_numbers? row.with_action( - text: user_action_text_helper(@user, "phone"), + text: user_action_text(@user, "phone"), visually_hidden_text: "telephone number", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-telephone-number" }, @@ -98,7 +98,7 @@ row.with_value { user_details_html(@user, current_user, "role") } if UserPolicy.new(current_user, @user).edit_roles? row.with_action( - text: user_action_text_helper(@user, "role"), + text: user_action_text(@user, "role"), visually_hidden_text: "role", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-role" },