Improvements

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"

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
 

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"

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

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

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| %>
+              <%= row.with_action(text: "Change", href: location_action_link(attr, @scheme, @location, current_user)) if location_editable_attributes.include?(attr[:attribute]) %>
-                <% if attr[:attribute] == attribute %>
-                  <%= row.with_action(text: "Change", href: location_action_link(attr, @scheme, @location, current_user)) %>
-                <% end %>
-              <% end %>
             <% end %>
           <% end %>
         <% end %>

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

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