From c53f4a770b5a43e40d0f6ffaa604ef087ec48c47 Mon Sep 17 00:00:00 2001
From: Arthur Campbell <51094020+arfacamble@users.noreply.github.com>
Date: Thu, 15 Jun 2023 15:06:38 +0100
Subject: [PATCH] Remove feature toggle for schemes and locations (#1700)

* remove scheme toggle

* remove location toggle

* minor linting complaint
---
 app/helpers/locations_helper.rb            |   9 +-
 app/helpers/schemes_helper.rb              |   5 +-
 app/services/feature_toggle.rb             |   8 --
 app/views/locations/index.html.erb         | 148 ++++++---------------
 app/views/locations/show.html.erb          |   6 +-
 app/views/schemes/_scheme_list.html.erb    |  22 +--
 app/views/schemes/show.html.erb            |  37 +++---
 spec/features/schemes_spec.rb              |  17 ---
 spec/helpers/schemes_helper_spec.rb        |   8 --
 spec/requests/locations_controller_spec.rb |  26 ----
 10 files changed, 69 insertions(+), 217 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/helpers/schemes_helper.rb b/app/helpers/schemes_helper.rb
index 0503f88b3..50bd4efb2 100644
--- a/app/helpers/schemes_helper.rb
+++ b/app/helpers/schemes_helper.rb
@@ -14,12 +14,9 @@ module SchemesHelper
       { name: "Level of support given", value: scheme.support_type },
       { name: "Intended length of stay", value: scheme.intended_stay },
       { name: "Availability", value: scheme_availability(scheme) },
+      { name: "Status", value: status_tag(scheme.status) },
     ]
 
-    if FeatureToggle.scheme_toggle_enabled?
-      base_attributes.append({ name: "Status", value: status_tag(scheme.status) })
-    end
-
     if user.data_coordinator?
       base_attributes.delete_if { |item| item[:name] == "Housing stock owned by" }
     end
diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb
index 88e1d17ef..2690d1e06 100644
--- a/app/services/feature_toggle.rb
+++ b/app/services/feature_toggle.rb
@@ -12,14 +12,6 @@ class FeatureToggle
     Rails.env.production? || Rails.env.test? || Rails.env.staging? || Rails.env.review?
   end
 
-  def self.scheme_toggle_enabled?
-    true
-  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..e974d7dfb 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? %>
-  <div class="govuk-grid-row">
-    <div class="govuk-grid-column-two-thirds-from-desktop">
-<% end %>
-  <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %>
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %>
 
-  <h2 class="govuk-visually-hidden">Locations</h2>
+    <h2 class="govuk-visually-hidden">Locations</h2>
 
-  <%= 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? %>
-    </div>
+    <%= govuk_section_break(visible: true, size: "m") %>
   </div>
-<% end %>
-
-<% if FeatureToggle.location_toggle_enabled? %>
-  <div class="govuk-grid-row">
-    <div class="govuk-grid-column-two-thirds-from-desktop">
-      <%= 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 %>
+</div>
 
-      <% if LocationPolicy.new(current_user, @scheme.locations.new).create? %>
-        <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %>
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <%= 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 %>
-    </div>
-  </div>
-<% 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 %>
-            <span><%= simple_format(location.type_of_unit) %></span>
-          <% 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 %>
+  </div>
+</div>
 
 <%== 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 @@
   </div>
 </div>
 
-<% 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/_scheme_list.html.erb b/app/views/schemes/_scheme_list.html.erb
index 5fa712d1e..4f3b06454 100644
--- a/app/views/schemes/_scheme_list.html.erb
+++ b/app/views/schemes/_scheme_list.html.erb
@@ -5,20 +5,10 @@
       <% end %>
     <%= table.head do |head| %>
       <%= head.row do |row| %>
-        <% row.cell(header: true, text: "Scheme", html_attributes: {
-          scope: "col",
-        }) %>
-        <% row.cell(header: true, text: "Code", html_attributes: {
-          scope: "col",
-        }) %>
-        <% row.cell(header: true, text: "Locations", html_attributes: {
-          scope: "col",
-        }) %>
-        <% if FeatureToggle.scheme_toggle_enabled? %>
-          <% row.cell(header: true, text: "Status", html_attributes: {
-            scope: "col",
-          }) %>
-        <% end %>
+        <% row.cell(header: true, text: "Scheme", html_attributes: { scope: "col" }) %>
+        <% row.cell(header: true, text: "Code", html_attributes: { scope: "col" }) %>
+        <% row.cell(header: true, text: "Locations", html_attributes: { scope: "col" }) %>
+        <% row.cell(header: true, text: "Status", html_attributes: { scope: "col" }) %>
       <% end %>
     <% end %>
     <% @schemes.each do |scheme| %>
@@ -27,9 +17,7 @@
           <% row.cell(text: simple_format(scheme_cell(scheme), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %>
           <% row.cell(text: scheme.id_to_display) %>
           <% row.cell(text: scheme.locations&.count) %>
-          <% if FeatureToggle.scheme_toggle_enabled? %>
-            <% row.cell(text: status_tag(scheme.status)) %>
-          <% end %>
+          <% row.cell(text: status_tag(scheme.status)) %>
         <% end %>
       <% end %>
     <% end %>
diff --git a/app/views/schemes/show.html.erb b/app/views/schemes/show.html.erb
index 0005582e8..4fe3a65de 100644
--- a/app/views/schemes/show.html.erb
+++ b/app/views/schemes/show.html.erb
@@ -9,33 +9,26 @@
 
 <%= render partial: "organisations/headings", locals: { main: @scheme.service_name, sub: nil } %>
 
-<% if FeatureToggle.location_toggle_enabled? %>
-  <div class="govuk-grid-row">
-    <div class="govuk-grid-column-two-thirds-from-desktop">
-<% end %>
-      <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %>
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds-from-desktop">
+    <%= render SubNavigationComponent.new(items: scheme_items(request.path, @scheme.id, "Locations")) %>
 
-      <h2 class="govuk-visually-hidden">Scheme</h2>
+    <h2 class="govuk-visually-hidden">Scheme</h2>
 
-      <%= 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 %>
+    <%= 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? %>
-    </div>
+    <% end %>
   </div>
-<% end %>
+</div>
 
-<% if FeatureToggle.scheme_toggle_enabled? %>
-  <% if SchemePolicy.new(current_user, @scheme).deactivate? %>
-    <%= toggle_scheme_link(@scheme) %>
-  <% end %>
+<% if SchemePolicy.new(current_user, @scheme).deactivate? %>
+  <%= toggle_scheme_link(@scheme) %>
 <% end %>
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/helpers/schemes_helper_spec.rb b/spec/helpers/schemes_helper_spec.rb
index 09a9c9e2e..905494ac8 100644
--- a/spec/helpers/schemes_helper_spec.rb
+++ b/spec/helpers/schemes_helper_spec.rb
@@ -195,14 +195,6 @@ RSpec.describe SchemesHelper do
         expect(display_scheme_attributes(scheme, coordinator_user)).to eq(attributes)
       end
 
-      context "when the scheme toggle is disabled" do
-        it "doesn't show the scheme status" do
-          allow(FeatureToggle).to receive(:scheme_toggle_enabled?).and_return(false)
-          attributes = display_scheme_attributes(scheme, support_user).find { |x| x[:name] == "Status" }
-          expect(attributes).to be_nil
-        end
-      end
-
       context "when the managing organisation is the owning organisation" do
         it "doesn't show the organisation providing support" do
           attributes = display_scheme_attributes(scheme_where_managing_organisation_is_owning_organisation, support_user).find { |x| x[:name] == "Organisation providing support" }
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