From 59ecac5cf3a2c8f1453d7f5ad6c6a1a2ef891578 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 17 Dec 2024 13:17:12 +0000
Subject: [PATCH] Remove action link not just hide
---
app/helpers/details_table_helper.rb | 2 +-
app/helpers/merge_requests_helper.rb | 12 +++++++++---
app/helpers/schemes_helper.rb | 17 ++---------------
.../form/_check_answers_summary_list.html.erb | 2 +-
app/views/locations/check_answers.html.erb | 4 +++-
app/views/organisations/show.html.erb | 16 ++++++++++------
.../schemes/_scheme_summary_list_row.html.erb | 2 +-
app/views/schemes/check_answers.html.erb | 2 +-
app/views/schemes/show.html.erb | 2 +-
app/views/users/show.html.erb | 4 ++--
10 files changed, 31 insertions(+), 32 deletions(-)
diff --git a/app/helpers/details_table_helper.rb b/app/helpers/details_table_helper.rb
index 2ba89e2fb..80ae87760 100644
--- a/app/helpers/details_table_helper.rb
+++ b/app/helpers/details_table_helper.rb
@@ -32,6 +32,6 @@ private
def scheme_value(attribute, resource)
return nil unless can_change_scheme_answer?(attribute[:name], resource)
- govuk_link_to(scheme_details_link_message(attribute), scheme_edit_path(resource, attribute), class: "govuk-link govuk-link--no-visited-state")
+ govuk_link_to(scheme_details_link_message(attribute), scheme_edit_path(resource, attribute[:id]), class: "govuk-link govuk-link--no-visited-state")
end
end
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index 6fa50a7f8..212a115bb 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -6,6 +6,10 @@ module MergeRequestsHelper
value.presence || content_tag(:span, placeholder, class: "app-!-colour-muted")
end
+ def value_exists?(merge_request, attribute)
+ merge_request.send(attribute).present? || (attribute == "helpdesk_ticket" && merge_request.has_helpdesk_ticket == false)
+ end
+
def details_prompt_link(page, merge_request)
govuk_link_to(merge_request_details_prompt(page), send("#{page}_merge_request_path", merge_request, referrer: "check_answers"), class: "govuk-link govuk-link--no-visited-state")
end
@@ -20,8 +24,8 @@ module MergeRequestsHelper
messages[page] || "Enter #{lowercase_first_letter(page.humanize)}"
end
- def merge_request_action_text(attribute, merge_request)
- merge_request.send(attribute).present? || (attribute == "helpdesk_ticket" && merge_request.has_helpdesk_ticket == false) ? "Change" : ""
+ def merge_request_action_text(merge_request, attribute)
+ value_exists?(merge_request, attribute) ? "Change" : ""
end
def request_details(merge_request)
@@ -95,8 +99,10 @@ module MergeRequestsHelper
def merge_request_action(merge_request, page, attribute = nil)
attribute = page if attribute.nil?
+ return nil unless value_exists?(merge_request, attribute)
+
unless merge_request.status == "request_merged" || merge_request.status == "processing"
- { text: merge_request_action_text(attribute, merge_request), href: send("#{page}_merge_request_path", merge_request, referrer: "check_answers"), visually_hidden_text: page.humanize }
+ { text: merge_request_action_text(merge_request, attribute), href: send("#{page}_merge_request_path", merge_request, referrer: "check_answers"), visually_hidden_text: page.humanize }
end
end
diff --git a/app/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb
index 6d638bc61..6d6c04367 100644
--- a/app/helpers/schemes_helper.rb
+++ b/app/helpers/schemes_helper.rb
@@ -64,10 +64,10 @@ module SchemesHelper
schemes_csv_download_organisation_path(organisation, search:, download_type:)
end
- def change_answer_link(scheme, question_id, user)
+ def scheme_edit_path(scheme, question_id, user = nil)
case question_id
when "service_name", "sensitive", "scheme_type", "registered_under_care_act", "owning_organisation_id", "arrangement_type"
- user.support? || !scheme.confirmed? ? scheme_details_path(scheme, referrer: "check-answers") : scheme_edit_name_path(scheme)
+ user&.support? || !scheme.confirmed? ? scheme_details_path(scheme, referrer: "check-answers") : scheme_edit_name_path(scheme)
when "primary_client_group"
scheme_primary_client_group_path(scheme, referrer: "check-answers")
when "has_other_client_group"
@@ -107,19 +107,6 @@ module SchemesHelper
organisation.owned_schemes.duplicate_sets.any? || organisation.owned_schemes.any? { |scheme| scheme.locations.duplicate_sets.any? }
end
- def scheme_edit_path(scheme, attribute)
- case attribute[:id]
- when "primary_client_group"
- scheme_primary_client_group_path(scheme, referrer: "check-answers")
- when "has_other_client_group"
- scheme_confirm_secondary_client_group_path(scheme, referrer: "check-answers")
- when "secondary_client_group"
- scheme_secondary_client_group_path(scheme, referrer: "check-answers")
- when "support_type", "intended_stay"
- scheme_support_path(scheme, referrer: "check-answers")
- end
- end
-
def scheme_details_link_message(attribute)
text = lowercase_first_letter(attribute[:name])
return "Select #{text}" if %w[primary_client_group secondary_client_group support_type intended_stay].include?(attribute[:id])
diff --git a/app/views/form/_check_answers_summary_list.html.erb b/app/views/form/_check_answers_summary_list.html.erb
index f849847a2..4748cdca2 100644
--- a/app/views/form/_check_answers_summary_list.html.erb
+++ b/app/views/form/_check_answers_summary_list.html.erb
@@ -25,7 +25,7 @@
<% end %>
<% end %>
- <% if @log.collection_period_open_for_editing? %>
+ <% if @log.collection_period_open_for_editing? && question.answer_label(@log, current_user).present? %>
<% row.with_action(
text: question.action_text(@log),
href: action_href(
diff --git a/app/views/locations/check_answers.html.erb b/app/views/locations/check_answers.html.erb
index 089ac37bd..497abf63f 100644
--- a/app/views/locations/check_answers.html.erb
+++ b/app/views/locations/check_answers.html.erb
@@ -29,8 +29,10 @@
<% else %>
<% row.with_value { details_html(attr, @location) if user_can_edit_scheme?(current_user, @scheme) } %>
<% end %>
- <% if LocationPolicy.new(current_user, @location).update? %>
+ <% if LocationPolicy.new(current_user, @location).update? && attr[:value] %>
<% row.with_action(text: location_action_text(attr, @location), href: location_edit_path(@location, attr[:attribute])) %>
+ <% else %>
+ <% row.with_action %>
<% end %>
<% end %>
<% end %>
diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb
index 2ff37d308..4cdad4a3f 100644
--- a/app/views/organisations/show.html.erb
+++ b/app/views/organisations/show.html.erb
@@ -23,12 +23,16 @@
<%= summary_list.with_row do |row| %>
<% row.with_key { attr[:name] } %>
<% row.with_value { details_html(attr, @organisation) } %>
- <% row.with_action(
- 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}" },
- ) %>
+ <% if attr[:value] %>
+ <% row.with_action(
+ 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}" },
+ ) %>
+ <% else %>
+ <% row.with_action %>
+ <% end %>
<% end %>
<% else %>
<%= summary_list.with_row do |row| %>
diff --git a/app/views/schemes/_scheme_summary_list_row.html.erb b/app/views/schemes/_scheme_summary_list_row.html.erb
index 822c70c08..8204d856e 100644
--- a/app/views/schemes/_scheme_summary_list_row.html.erb
+++ b/app/views/schemes/_scheme_summary_list_row.html.erb
@@ -15,7 +15,7 @@
<% end %>
- <% if can_change_scheme_answer?(attribute[:name], scheme) && attribute[:edit] %>
+ <% if can_change_scheme_answer?(attribute[:name], scheme) && attribute[:edit] && attribute[:value] %>
<%= change_link_text(attribute[:id], @scheme) %>
diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb
index a98c86a51..2af818727 100644
--- a/app/views/schemes/check_answers.html.erb
+++ b/app/views/schemes/check_answers.html.erb
@@ -10,7 +10,7 @@
<% @scheme.check_details_attributes.each do |attr| %>
<% if attr[:name] != "Status" && (attr[:id] != "secondary_client_group" || @scheme.has_other_client_group == "Yes") %>
- <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: change_answer_link(@scheme, attr[:id], current_user) } %>
+ <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_edit_path(@scheme, attr[:id], current_user) } %>
<% end %>
<% end %>
diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb
index 58bcd521e..5cf692676 100644
--- a/app/views/schemes/show.html.erb
+++ b/app/views/schemes/show.html.erb
@@ -38,7 +38,7 @@
<% elsif attr[:id] != "secondary_client_group" || @scheme.has_other_client_group == "Yes" %>
- <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: change_answer_link(@scheme, attr[:id], current_user) } %>
+ <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: scheme_edit_path(@scheme, attr[:id], current_user) } %>
<% end %>
<% end %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index a04aa2fa5..78f2a06d3 100644
--- a/app/views/users/show.html.erb
+++ b/app/views/users/show.html.erb
@@ -53,7 +53,7 @@
<%= summary_list.with_row do |row|
row.with_key { "Telephone number" }
row.with_value { user_details_html(@user, current_user, "phone") }
- if UserPolicy.new(current_user, @user).edit_telephone_numbers?
+ if UserPolicy.new(current_user, @user).edit_telephone_numbers? && @user.phone.present?
row.with_action(
text: user_action_text(@user, "phone"),
visually_hidden_text: "telephone number",
@@ -96,7 +96,7 @@
<%= summary_list.with_row do |row|
row.with_key { "Role" }
row.with_value { user_details_html(@user, current_user, "role") }
- if UserPolicy.new(current_user, @user).edit_roles?
+ if UserPolicy.new(current_user, @user).edit_roles? && @user.role
row.with_action(
text: user_action_text(@user, "role"),
visually_hidden_text: "role",