From cf0e9677375a055340f0282a65b8ed3d482e71db Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Thu, 5 Sep 2024 10:13:03 +0100
Subject: [PATCH] Technical review

---
 app/controllers/schemes_controller.rb         | 14 ++------------
 app/helpers/deactivate_confirm_helper.rb      |  8 ++++++++
 app/models/location.rb                        |  6 ++----
 app/views/schemes/deactivate_confirm.html.erb | 19 +++++++------------
 4 files changed, 19 insertions(+), 28 deletions(-)
 create mode 100644 app/helpers/deactivate_confirm_helper.rb

diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb
index ad79ae4b0..1468bc013 100644
--- a/app/controllers/schemes_controller.rb
+++ b/app/controllers/schemes_controller.rb
@@ -57,20 +57,10 @@ class SchemesController < ApplicationController
 
     scheme_locations = @scheme.locations.confirmed
 
-    locations_active_on_deactivation_date, remaining_locations = scheme_locations.partition do |location|
-      location.status_at(@deactivation_date) == :active
+    @affected_locations = scheme_locations.select do |location|
+      %i[active deactivating_soon reactivating_soon activating_soon].include?(location.status_at(@deactivation_date))
     end
 
-    locations_deactivating_after_deactivation_date, remaining_locations = remaining_locations.partition do |location|
-      location.status_at(@deactivation_date) == :deactivating_soon || location.status_at(@deactivation_date) == :reactivating_soon
-    end
-
-    locations_startdate_after_deactivation_date, = remaining_locations.partition do |location|
-      location.status_at(@deactivation_date) == :activating_soon
-    end
-
-    @affected_locations = locations_active_on_deactivation_date + locations_deactivating_after_deactivation_date + locations_startdate_after_deactivation_date
-
     if @affected_logs.count.zero? && @affected_locations.count.zero?
       deactivate
     end
diff --git a/app/helpers/deactivate_confirm_helper.rb b/app/helpers/deactivate_confirm_helper.rb
new file mode 100644
index 000000000..32c3bc501
--- /dev/null
+++ b/app/helpers/deactivate_confirm_helper.rb
@@ -0,0 +1,8 @@
+module DeactivateConfirmHelper
+  def affected_title(affected_logs, affected_locations)
+    title_parts = []
+    title_parts << pluralize(affected_logs.count, "log") if affected_logs.count > 0
+    title_parts << pluralize(affected_locations.count, "location") if affected_locations.count > 0
+    "This change will affect #{title_parts.join(' and ')}."
+  end
+end
diff --git a/app/models/location.rb b/app/models/location.rb
index 55bf99fe7..19cf5e211 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -113,10 +113,8 @@ class Location < ApplicationRecord
   }
 
   scope :active, lambda { |date = Time.zone.now|
-    where.not(id: joins(scheme: [:scheme_deactivation_periods]).reactivating_soon_by_scheme.pluck(:id))
-         .where.not(id: joins(:location_deactivation_periods).merge(Location.deactivated_directly(date)).pluck(:id))
-         .where.not(id: joins(scheme: [:owning_organisation]).merge(Location.deactivated_by_organisation).pluck(:id))
-         .where.not(id: incomplete.pluck(:id))
+    where.not(id: joins(:location_deactivation_periods).merge(Location.deactivated_directly(date)).pluck(:id))
+          .where.not(id: incomplete.pluck(:id))
          .where.not(id: activating_soon(date).pluck(:id))
          .where(scheme: Scheme.active(date))
   }
diff --git a/app/views/schemes/deactivate_confirm.html.erb b/app/views/schemes/deactivate_confirm.html.erb
index f325c7b98..a73208e75 100644
--- a/app/views/schemes/deactivate_confirm.html.erb
+++ b/app/views/schemes/deactivate_confirm.html.erb
@@ -7,14 +7,7 @@
 
       <h1 class="govuk-heading-l">
         <span class="govuk-caption-l"><%= @scheme.service_name %></span>
-        <% sentence_parts = [] %>
-        <% if @affected_logs.count > 0 %>
-          <% sentence_parts << pluralize(@affected_logs.count, "log") %>
-        <% end %>
-        <% if @affected_locations.count > 0 %>
-          <% sentence_parts << pluralize(@affected_locations.count, "location") %>
-        <% end %>
-        This change will affect <%= sentence_parts.join(" and ") %>.
+        <%= affected_title(@affected_logs, @affected_locations) %>
       </h1>
 
       <% if @affected_logs.count > 0 %>
@@ -24,10 +17,12 @@
         <%= govuk_warning_text text: I18n.t("warnings.scheme.deactivate.review_logs"), html_attributes: { class: "" } %>
     <% end %>
 
-      <p>
-        This scheme has <%= pluralize(@affected_locations.count, "location") %> active on <%= @deactivation_date.to_formatted_s(:govuk_date) %>.  <%= @affected_locations.count == 1 ? "This location" : "These locations" %> will deactivate on that date. If the scheme is ever reactivated, <%= @affected_locations.count == 1 ? "this location" : "these locations" %> will reactivate as well.
-      </p>
-      <br>
+      <% if @affected_locations.count > 0 %>
+        <p>
+          This scheme has <%= pluralize(@affected_locations.count, "location") %> active on <%= @deactivation_date.to_formatted_s(:govuk_date) %>.  <%= @affected_locations.count == 1 ? "This location" : "These locations" %> will deactivate on that date. If the scheme is ever reactivated, <%= @affected_locations.count == 1 ? "this location" : "these locations" %> will reactivate as well.
+        </p>
+        <br>
+      <% end %>
 
       <%= f.hidden_field :confirm, value: true %>
       <%= f.hidden_field :deactivation_date, value: @deactivation_date %>