From 74bfd49352345b7abd3793bca898409a41b9e7b2 Mon Sep 17 00:00:00 2001
From: Kat <katrina@kosiak.co.uk>
Date: Tue, 18 Jul 2023 16:02:48 +0100
Subject: [PATCH] Update incomplete, make reactivated scope work in conjunction
 with other scopes. Lint

---
 app/models/location.rb       | 66 +++++++++++++++++++-----------------
 spec/models/location_spec.rb | 30 ++++++++--------
 2 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/app/models/location.rb b/app/models/location.rb
index 3c816dcd5..f25fd7d5d 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -27,51 +27,55 @@ class Location < ApplicationRecord
   scope :active_in_2_weeks, -> { where(confirmed: true).and(started_in_2_weeks) }
   scope :confirmed, -> { where(confirmed: true) }
   scope :unconfirmed, -> { where.not(confirmed: true) }
-  scope :filter_by_status, ->(statuses, _user = nil) {
+  scope :filter_by_status, lambda { |statuses, _user = nil|
     filtered_records = all
     scopes = []
-  
+
     statuses.each do |status|
       if respond_to?(status, true)
-        status == "active" ? scopes << send("active_status") : scopes << send(status)
+        scopes << (status == "active" ? send("active_status") : send(status))
       end
     end
-  
-    filtered_records = filtered_records.left_outer_joins(:location_deactivation_periods).merge(scopes.reduce(&:or)) if scopes.any?
-  
+
+    if scopes.any?
+      filtered_records = filtered_records
+      .left_outer_joins(:location_deactivation_periods)
+      .order("location_deactivation_periods.created_at DESC")
+      .merge(scopes.reduce(&:or))
+    end
+
     filtered_records
   }
-  
-  scope :incomplete, -> {
-      where(confirmed: false)
+
+  scope :incomplete, lambda {
+    where.not(confirmed: true)
   }
-  
-  scope :deactivated, -> {
-      merge(LocationDeactivationPeriod.deactivations_without_reactivation)
-      .where("location_deactivation_periods.deactivation_date <= ?", Time.zone.now)
+
+  scope :deactivated, lambda {
+    merge(LocationDeactivationPeriod.deactivations_without_reactivation)
+    .where("location_deactivation_periods.deactivation_date <= ?", Time.zone.now)
   }
-  
-  scope :deactivating_soon, -> {
-      merge(LocationDeactivationPeriod.deactivations_without_reactivation)
-      .where("location_deactivation_periods.deactivation_date > ?", Time.zone.now)
+
+  scope :deactivating_soon, lambda {
+    merge(LocationDeactivationPeriod.deactivations_without_reactivation)
+    .where("location_deactivation_periods.deactivation_date > ?", Time.zone.now)
   }
-  
-  scope :reactivating_soon, -> {
-      where.not("location_deactivation_periods.reactivation_date IS NULL")
-      .order("location_deactivation_periods.created_at DESC")
-      .where("location_deactivation_periods.reactivation_date > ?", Time.zone.now)
+
+  scope :reactivating_soon, lambda {
+    where.not("location_deactivation_periods.reactivation_date IS NULL")
+    .where("location_deactivation_periods.reactivation_date > ?", Time.zone.now)
   }
-  
-  scope :activating_soon, -> {
+
+  scope :activating_soon, lambda {
     where("startdate > ?", Time.zone.now)
   }
-  
-  scope :active_status, -> {
-      where.not(id: joins(:location_deactivation_periods).reactivating_soon.pluck(:id))
-      .where.not(id: joins(:location_deactivation_periods).deactivated.pluck(:id))
-      .where.not(id: incomplete.pluck(:id))
-      .where.not(id: joins(:location_deactivation_periods).deactivating_soon.pluck(:id))
-      .where.not(id: activating_soon.pluck(:id))
+
+  scope :active_status, lambda {
+    where.not(id: joins(:location_deactivation_periods).reactivating_soon.pluck(:id))
+    .where.not(id: joins(:location_deactivation_periods).deactivated.pluck(:id))
+    .where.not(id: incomplete.pluck(:id))
+    .where.not(id: joins(:location_deactivation_periods).deactivating_soon.pluck(:id))
+    .where.not(id: activating_soon.pluck(:id))
   }
 
   LOCAL_AUTHORITIES = LocalAuthority.all.map { |la| [la.name, la.code] }.to_h
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index 6636e30a8..bbcb90e54 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -954,51 +954,51 @@ RSpec.describe Location, type: :model do
 
     context "when filtering by incomplete status" do
       it "returns only incomplete locations" do
-        expect(described_class.filter_by_status(["incomplete"]).count).to eq(1)
-        expect(described_class.filter_by_status(["incomplete"]).first).to eq(incomplete_location)
+        expect(described_class.filter_by_status(%w[incomplete]).count).to eq(1)
+        expect(described_class.filter_by_status(%w[incomplete]).first).to eq(incomplete_location)
       end
     end
 
     context "when filtering by active status" do
       it "returns only active locations" do
-        expect(described_class.filter_by_status(["active"]).count).to eq(1)
-        expect(described_class.filter_by_status(["active"]).first).to eq(active_location)
+        expect(described_class.filter_by_status(%w[active]).count).to eq(1)
+        expect(described_class.filter_by_status(%w[active]).first).to eq(active_location)
       end
     end
 
     context "when filtering by deactivating_soon status" do
       it "returns only deactivating_soon locations" do
-        expect(described_class.filter_by_status(["deactivating_soon"]).count).to eq(1)
-        expect(described_class.filter_by_status(["deactivating_soon"]).first).to eq(deactivating_soon_location)
+        expect(described_class.filter_by_status(%w[deactivating_soon]).count).to eq(1)
+        expect(described_class.filter_by_status(%w[deactivating_soon]).first).to eq(deactivating_soon_location)
       end
     end
 
     context "when filtering by deactivated status" do
       it "returns only deactivated locations" do
-        expect(described_class.filter_by_status(["deactivated"]).count).to eq(1)
-        expect(described_class.filter_by_status(["deactivated"]).first).to eq(deactivated_location)
+        expect(described_class.filter_by_status(%w[deactivated]).count).to eq(1)
+        expect(described_class.filter_by_status(%w[deactivated]).first).to eq(deactivated_location)
       end
     end
 
     context "when filtering by reactivating_soon status" do
       it "returns only reactivating_soon locations" do
-        expect(described_class.filter_by_status(["reactivating_soon"]).count).to eq(1)
-        expect(described_class.filter_by_status(["reactivating_soon"]).first).to eq(reactivating_soon_location)
+        expect(described_class.filter_by_status(%w[reactivating_soon]).count).to eq(1)
+        expect(described_class.filter_by_status(%w[reactivating_soon]).first).to eq(reactivating_soon_location)
       end
     end
 
     context "when filtering by activating_soon status" do
       it "returns only activating_soon locations" do
-        expect(described_class.filter_by_status(["activating_soon"]).count).to eq(1)
-        expect(described_class.filter_by_status(["activating_soon"]).first).to eq(activating_soon_location)
+        expect(described_class.filter_by_status(%w[activating_soon]).count).to eq(1)
+        expect(described_class.filter_by_status(%w[activating_soon]).first).to eq(activating_soon_location)
       end
     end
 
     context "when filtering by multiple statuses" do
       it "returns only activating_soon locations" do
-        expect(described_class.filter_by_status(["deactivating_soon", "activating_soon"]).count).to eq(2)
-        expect(described_class.filter_by_status(["deactivating_soon", "activating_soon"])).to include(activating_soon_location)
-        expect(described_class.filter_by_status(["deactivating_soon", "activating_soon"])).to include(deactivating_soon_location)
+        expect(described_class.filter_by_status(%w[deactivating_soon activating_soon]).count).to eq(2)
+        expect(described_class.filter_by_status(%w[deactivating_soon activating_soon])).to include(activating_soon_location)
+        expect(described_class.filter_by_status(%w[deactivating_soon activating_soon])).to include(deactivating_soon_location)
       end
     end
   end