From a3240e5d161e171565013c246306d1033d8537dc Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Tue, 17 Dec 2024 15:31:33 +0000
Subject: [PATCH] Revert "Remove action link not just hide"
This reverts commit 59ecac5cf3a2c8f1453d7f5ad6c6a1a2ef891578.
---
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, 32 insertions(+), 31 deletions(-)
diff --git a/app/helpers/details_table_helper.rb b/app/helpers/details_table_helper.rb
index 80ae87760..2ba89e2fb 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[:id]), class: "govuk-link govuk-link--no-visited-state")
+ govuk_link_to(scheme_details_link_message(attribute), scheme_edit_path(resource, attribute), 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 212a115bb..6fa50a7f8 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -6,10 +6,6 @@ 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
@@ -24,8 +20,8 @@ module MergeRequestsHelper
messages[page] || "Enter #{lowercase_first_letter(page.humanize)}"
end
- def merge_request_action_text(merge_request, attribute)
- value_exists?(merge_request, attribute) ? "Change" : ""
+ def merge_request_action_text(attribute, merge_request)
+ merge_request.send(attribute).present? || (attribute == "helpdesk_ticket" && merge_request.has_helpdesk_ticket == false) ? "Change" : ""
end
def request_details(merge_request)
@@ -99,10 +95,8 @@ 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(merge_request, attribute), 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/schemes_helper.rb b/app/helpers/schemes_helper.rb
index 6d6c04367..6d638bc61 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 scheme_edit_path(scheme, question_id, user = nil)
+ def change_answer_link(scheme, question_id, user)
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,6 +107,19 @@ 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 4748cdca2..f849847a2 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? && question.answer_label(@log, current_user).present? %>
+ <% if @log.collection_period_open_for_editing? %>
<% 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 497abf63f..089ac37bd 100644
--- a/app/views/locations/check_answers.html.erb
+++ b/app/views/locations/check_answers.html.erb
@@ -29,10 +29,8 @@
<% else %>
<% row.with_value { details_html(attr, @location) if user_can_edit_scheme?(current_user, @scheme) } %>
<% end %>
- <% if LocationPolicy.new(current_user, @location).update? && attr[:value] %>
+ <% if LocationPolicy.new(current_user, @location).update? %>
<% 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 4cdad4a3f..2ff37d308 100644
--- a/app/views/organisations/show.html.erb
+++ b/app/views/organisations/show.html.erb
@@ -23,16 +23,12 @@
<%= summary_list.with_row do |row| %>
<% row.with_key { attr[:name] } %>
<% row.with_value { details_html(attr, @organisation) } %>
- <% 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 %>
+ <% 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}" },
+ ) %>
<% 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 8204d856e..822c70c08 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] && attribute[:value] %>
+ <% if can_change_scheme_answer?(attribute[:name], scheme) && attribute[:edit] %>
<%= 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 2af818727..a98c86a51 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: scheme_edit_path(@scheme, attr[:id], current_user) } %>
+ <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: change_answer_link(@scheme, attr[:id], current_user) } %>
<% end %>
<% end %>
diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb
index 5cf692676..58bcd521e 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: scheme_edit_path(@scheme, attr[:id], current_user) } %>
+ <%= render partial: "scheme_summary_list_row", locals: { scheme: @scheme, attribute: attr, change_link: change_answer_link(@scheme, attr[:id], current_user) } %>
<% end %>
<% end %>
diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb
index 78f2a06d3..a04aa2fa5 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? && @user.phone.present?
+ if UserPolicy.new(current_user, @user).edit_telephone_numbers?
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? && @user.role
+ if UserPolicy.new(current_user, @user).edit_roles?
row.with_action(
text: user_action_text(@user, "role"),
visually_hidden_text: "role",