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