From e6f9defe7335b7945d2392d7da82f49758dce7a7 Mon Sep 17 00:00:00 2001 From: Arthur Campbell Date: Thu, 15 Jun 2023 14:04:50 +0100 Subject: [PATCH] remove location toggle --- app/helpers/locations_helper.rb | 9 +- app/services/feature_toggle.rb | 4 - app/views/locations/index.html.erb | 148 ++++++--------------- app/views/locations/show.html.erb | 6 +- app/views/schemes/show.html.erb | 36 +++-- spec/features/schemes_spec.rb | 17 --- spec/requests/locations_controller_spec.rb | 26 ---- 7 files changed, 63 insertions(+), 183 deletions(-) diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb index 7e38ed36c..58fffd2e0 100644 --- a/app/helpers/locations_helper.rb +++ b/app/helpers/locations_helper.rb @@ -24,7 +24,7 @@ module LocationsHelper end def display_location_attributes(location) - base_attributes = [ + [ { name: "Postcode", value: location.postcode, attribute: "postcode" }, { name: "Location name", value: location.name, attribute: "name" }, { name: "Local authority", value: formatted_local_authority_timeline(location, "name"), attribute: "local_authority" }, @@ -33,13 +33,8 @@ module LocationsHelper { name: "Mobility standards", value: location.mobility_type, attribute: "mobility_standards" }, { name: "Location code", value: formatted_local_authority_timeline(location, "code"), attribute: "location_code" }, { name: "Availability", value: location_availability(location), attribute: "availability" }, + { name: "Status", value: location.status, attribute: "status" }, ] - - if FeatureToggle.location_toggle_enabled? - base_attributes.append({ name: "Status", value: location.status, attribute: "status" }) - end - - base_attributes end def display_location_attributes_for_check_answers(location) diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb index ce1e31e01..2690d1e06 100644 --- a/app/services/feature_toggle.rb +++ b/app/services/feature_toggle.rb @@ -12,10 +12,6 @@ class FeatureToggle Rails.env.production? || Rails.env.test? || Rails.env.staging? || Rails.env.review? end - def self.location_toggle_enabled? - true - end - def self.bulk_upload_duplicate_log_check_enabled? !Rails.env.staging? end diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb index 85ae27fed..27bec1303 100644 --- a/app/views/locations/index.html.erb +++ b/app/views/locations/index.html.erb @@ -10,120 +10,60 @@ <%= render partial: "organisations/headings", locals: { main: @scheme.service_name, sub: nil } %> -<% if FeatureToggle.location_toggle_enabled? %> -
-
-<% end %> - <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %> +
+
+ <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %> -

Locations

+

Locations

- <%= render SearchComponent.new(current_user:, search_label: "Search by location name or postcode", value: @searched) %> + <%= render SearchComponent.new(current_user:, search_label: "Search by location name or postcode", value: @searched) %> - <%= govuk_section_break(visible: true, size: "m") %> -<% if FeatureToggle.location_toggle_enabled? %> -
+ <%= govuk_section_break(visible: true, size: "m") %>
-<% end %> - -<% if FeatureToggle.location_toggle_enabled? %> -
-
- <%= govuk_table do |table| %> - <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched: @searched, count: @pagy.count, item_label:, total_count: @total_count, item: "locations", path: request.path)) %> - <% end %> - <%= table.head do |head| %> - <%= head.row do |row| %> - <% row.cell(header: true, text: "Postcode", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Name", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Location code", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Status", html_attributes: { - scope: "col", - }) %> - <% end %> - <% end %> - <% @locations.each do |location| %> - <%= table.body do |body| %> - <%= body.row do |row| %> - <% row.cell(text: simple_format(location_cell_postcode(location, if location.confirmed - scheme_location_path(@scheme, location) - else - location.postcode.present? ? scheme_location_check_answers_path(@scheme, location, route: "locations") : scheme_location_postcode_path(@scheme, location) - end), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> - <% row.cell(text: location.name) %> - <% row.cell(text: location.id) %> - <% row.cell(text: status_tag(location.status)) %> - <% end %> - <% end %> - <% end %> - <% end %> +
- <% if LocationPolicy.new(current_user, @scheme.locations.new).create? %> - <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %> +
+
+ <%= govuk_table do |table| %> + <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> + <%= render(SearchResultCaptionComponent.new(searched: @searched, count: @pagy.count, item_label:, total_count: @total_count, item: "locations", path: request.path)) %> <% end %> -
-
-<% else %> - <%= govuk_table do |table| %> - <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched: @searched, count: @pagy.count, item_label:, total_count: @total_count, item: "locations", path: request.path)) %> - <% end %> - <%= table.head do |head| %> - <%= head.row do |row| %> - <% row.cell(header: true, text: "Code", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Postcode", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Units", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Common unit type", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Mobility type", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Local authority", html_attributes: { - scope: "col", - }) %> - <% row.cell(header: true, text: "Available from", html_attributes: { - scope: "col", - }) %> + <%= table.head do |head| %> + <%= head.row do |row| %> + <% row.cell(header: true, text: "Postcode", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Name", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Location code", html_attributes: { + scope: "col", + }) %> + <% row.cell(header: true, text: "Status", html_attributes: { + scope: "col", + }) %> + <% end %> <% end %> - <% end %> - <% @locations.each do |location| %> - <%= table.body do |body| %> - <%= body.row do |row| %> - <% row.cell(text: location.id) %> - <% row.cell(text: simple_format(location_cell_postcode(location, if location.confirmed - scheme_location_path(@scheme, location) - else - location.postcode.present? ? scheme_location_check_answers_path(@scheme, location, route: "locations") : scheme_location_postcode_path(@scheme, location) - end), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> - <% row.cell(text: location.units) %> - <% row.cell do %> - <%= simple_format(location.type_of_unit) %> - <% end %> - <% row.cell(text: location.mobility_type) %> - <% row.cell(text: location.location_admin_district) %> - <% row.cell(text: location.startdate&.to_formatted_s(:govuk_date)) %> + <% @locations.each do |location| %> + <%= table.body do |body| %> + <%= body.row do |row| %> + <% row.cell(text: simple_format(location_cell_postcode(location, if location.confirmed + scheme_location_path(@scheme, location) + else + location.postcode.present? ? scheme_location_check_answers_path(@scheme, location, route: "locations") : scheme_location_postcode_path(@scheme, location) + end), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> + <% row.cell(text: location.name) %> + <% row.cell(text: location.id) %> + <% row.cell(text: status_tag(location.status)) %> <% end %> + <% end %> <% end %> <% end %> - <% end %> - <% if user_can_edit_scheme?(current_user, @scheme) %> - <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %> - <% end %> -<% end %> + <% if LocationPolicy.new(current_user, @scheme.locations.new).create? %> + <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %> + <% end %> +
+
<%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "locations" } %> diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb index d949a8c63..dd819b2fb 100644 --- a/app/views/locations/show.html.erb +++ b/app/views/locations/show.html.erb @@ -25,8 +25,6 @@
-<% if FeatureToggle.location_toggle_enabled? %> - <% if LocationPolicy.new(current_user, @location).deactivate? %> - <%= toggle_location_link(@location) %> - <% end %> +<% if LocationPolicy.new(current_user, @location).deactivate? %> + <%= toggle_location_link(@location) %> <% end %> diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb index b6022478b..4fe3a65de 100644 --- a/app/views/schemes/show.html.erb +++ b/app/views/schemes/show.html.erb @@ -9,31 +9,25 @@ <%= render partial: "organisations/headings", locals: { main: @scheme.service_name, sub: nil } %> -<% if FeatureToggle.location_toggle_enabled? %> -
-
-<% end %> - - <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %> - -

Scheme

- - <%= govuk_summary_list do |summary_list| %> - <% display_scheme_attributes(@scheme, current_user).each do |attr| %> - <%= summary_list.row do |row| %> - <% row.key { attr[:name] } %> - <% row.value { details_html(attr) } %> - <% if SchemePolicy.new(current_user, @scheme).update? %> - <% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] %> - <% end %> +
+
+ <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %> + +

Scheme

+ + <%= govuk_summary_list do |summary_list| %> + <% display_scheme_attributes(@scheme, current_user).each do |attr| %> + <%= summary_list.row do |row| %> + <% row.key { attr[:name] } %> + <% row.value { details_html(attr) } %> + <% if SchemePolicy.new(current_user, @scheme).update? %> + <% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] %> <% end %> <% end %> <% end %> - -<% if FeatureToggle.location_toggle_enabled? %> -
+ <% end %>
-<% end %> +
<% if SchemePolicy.new(current_user, @scheme).deactivate? %> <%= toggle_scheme_link(@scheme) %> diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb index 1e45634c6..777a6c88d 100644 --- a/spec/features/schemes_spec.rb +++ b/spec/features/schemes_spec.rb @@ -211,23 +211,6 @@ RSpec.describe "Schemes scheme Features" do end end - context "when I click locations link and the new locations layout feature toggle is disabled" do - before do - allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false) - click_link("Locations") - end - - it "shows details of those locations" do - locations.each do |location| - expect(page).to have_content(location.id) - expect(page).to have_content(location.postcode) - expect(page).to have_content(location.units) - expect(page).to have_content(location.type_of_unit) - expect(page).to have_content(location.startdate&.to_formatted_s(:govuk_date)) - end - end - end - context "when I search for a specific location" do before do click_link("Locations") diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb index 04eb71f1a..88d73901c 100644 --- a/spec/requests/locations_controller_spec.rb +++ b/spec/requests/locations_controller_spec.rb @@ -153,19 +153,6 @@ RSpec.describe LocationsController, type: :request do end end - it "shows locations with correct data when the new locations layout feature toggle is disabled" do - allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false) - get "/schemes/#{scheme.id}/locations" - locations.each do |location| - expect(page).to have_content(location.id) - expect(page).to have_content(location.postcode) - expect(page).to have_content(location.type_of_unit) - expect(page).to have_content(location.mobility_type) - expect(page).to have_content(location.location_admin_district) - expect(page).to have_content(location.startdate&.to_formatted_s(:govuk_date)) - end - end - it "has page heading" do expect(page).to have_content(scheme.service_name) end @@ -294,19 +281,6 @@ RSpec.describe LocationsController, type: :request do expect(page).to have_button("Add a location") end - it "shows locations with correct data when the new locations layout feature toggle is disabled" do - allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false) - get "/schemes/#{scheme.id}/locations" - locations.each do |location| - expect(page).to have_content(location.id) - expect(page).to have_content(location.postcode) - expect(page).to have_content(location.type_of_unit) - expect(page).to have_content(location.mobility_type) - expect(page).to have_content(location.location_admin_district) - expect(page).to have_content(location.startdate&.to_formatted_s(:govuk_date)) - end - end - it "has page heading" do expect(page).to have_content(scheme.service_name) end