Remove action link not just hide

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

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
 

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])

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(

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 %>

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| %>

app/views/schemes/_scheme_summary_list_row.html.erb

@@ -15,7 +15,7 @@
         </dd>
     <% end %>
 
-    <% if can_change_scheme_answer?(attribute[:name], scheme) && attribute[:edit] %>
+    <% if can_change_scheme_answer?(attribute[:name], scheme) && attribute[:edit] && attribute[:value] %>
       <dd class="govuk-summary-list__actions">
         <a class="govuk-link" href="<%= change_link %>"><%= change_link_text(attribute[:id], @scheme) %></a>
       </dd>

app/views/schemes/check_answers.html.erb

@@ -10,7 +10,7 @@
       <dl class="govuk-summary-list">
         <% @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 %>
       </dl>

app/views/schemes/show.html.erb

@@ -38,7 +38,7 @@
             </dd>
           </div>
         <% 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 %>
 

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",