From 96c792b472827bec373760a2352e422c09f198da Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 17 Nov 2022 09:00:13 +0000
Subject: [PATCH] Cldc 1668 multiple location deactivations (#996)

* Add location_deactivations table and remove deactivation_date from locations

* Add location location_deactivations relationship

* Update logs status determination for location

* Update affected logs with deactivated location.

* lint

* Update scheme

* Extract location_deactivation factory

* Update location availability

* lint

* Rename deactivations table

* fix schema

* rebase fixes

* fixes

* refactor
---
 app/controllers/locations_controller.rb       |  8 ++-
 app/helpers/locations_helper.rb               | 11 ++-
 .../form/lettings/questions/location_id.rb    |  2 +-
 app/models/lettings_log.rb                    |  1 +
 app/models/location.rb                        | 10 +--
 app/models/location_deactivation_period.rb    |  3 +
 app/views/schemes/check_answers.html.erb      |  2 +-
 .../20221115101732_add_deactivations_table.rb | 10 +++
 .../20221115113437_remove_deactivatio_date.rb | 13 ++++
 db/schema.rb                                  | 12 +++-
 .../factories/location_deactivation_period.rb |  5 ++
 spec/features/schemes_spec.rb                 |  2 +-
 spec/helpers/locations_helper_spec.rb         | 27 ++++++--
 spec/models/location_spec.rb                  | 68 +++++++++++++------
 spec/requests/locations_controller_spec.rb    | 41 ++++++++---
 15 files changed, 166 insertions(+), 49 deletions(-)
 create mode 100644 app/models/location_deactivation_period.rb
 create mode 100644 db/migrate/20221115101732_add_deactivations_table.rb
 create mode 100644 db/migrate/20221115113437_remove_deactivatio_date.rb
 create mode 100644 spec/factories/location_deactivation_period.rb

diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb
index b25f951ea..b9132305f 100644
--- a/app/controllers/locations_controller.rb
+++ b/app/controllers/locations_controller.rb
@@ -43,7 +43,7 @@ class LocationsController < ApplicationController
   def deactivate
     @location.run_deactivation_validations!
 
-    if @location.update!(deactivation_date:)
+    if @location.location_deactivation_periods.create!(deactivation_date:) && update_affected_logs
       flash[:notice] = deactivate_success_notice
     end
     redirect_to scheme_location_path(@scheme, @location)
@@ -185,10 +185,14 @@ private
     when :deactivated
       "#{@location.name} has been deactivated"
     when :deactivating_soon
-      "#{@location.name} will deactivate on #{@location.deactivation_date.to_formatted_s(:govuk_date)}"
+      "#{@location.name} will deactivate on #{deactivation_date.to_time.to_formatted_s(:govuk_date)}"
     end
   end
 
+  def update_affected_logs
+    @location.lettings_logs.filter_by_before_startdate(deactivation_date.to_time).update!(location: nil, scheme: nil)
+  end
+
   def deactivation_date
     if params[:location].blank?
       return
diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb
index 2a02ebc94..eaf1e07af 100644
--- a/app/helpers/locations_helper.rb
+++ b/app/helpers/locations_helper.rb
@@ -32,8 +32,17 @@ module LocationsHelper
       { name: "Common type of unit", value: location.type_of_unit },
       { name: "Mobility type", value: location.mobility_type },
       { name: "Code", value: location.location_code },
-      { name: "Availability", value: "Available from #{location.available_from.to_formatted_s(:govuk_date)}" },
+      { name: "Availability", value: location_availability(location) },
       { name: "Status", value: location.status },
     ]
   end
+
+  def location_availability(location)
+    availability = "Active from #{location.available_from.to_formatted_s(:govuk_date)}"
+    location.location_deactivation_periods.each do |deactivation|
+      availability << " to #{(deactivation.deactivation_date - 1.day).to_formatted_s(:govuk_date)}\nDeactivated on #{deactivation.deactivation_date.to_formatted_s(:govuk_date)}"
+      availability << "\nActive from #{deactivation.reactivation_date.to_formatted_s(:govuk_date)}" if deactivation.reactivation_date.present?
+    end
+    availability
+  end
 end
diff --git a/app/models/form/lettings/questions/location_id.rb b/app/models/form/lettings/questions/location_id.rb
index cae6463a9..84b37ec50 100644
--- a/app/models/form/lettings/questions/location_id.rb
+++ b/app/models/form/lettings/questions/location_id.rb
@@ -23,7 +23,7 @@ class Form::Lettings::Questions::LocationId < ::Form::Question
     end
   end
 
-  def displayed_answer_options(lettings_log)
+  def displayed_answer_options(lettings_log, _user = nil)
     return {} unless lettings_log.scheme
 
     scheme_location_ids = lettings_log.scheme.locations.pluck(:id)
diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb
index bc011fdfd..574694f51 100644
--- a/app/models/lettings_log.rb
+++ b/app/models/lettings_log.rb
@@ -45,6 +45,7 @@ class LettingsLog < Log
                           .or(filter_by_postcode(param))
                           .or(filter_by_id(param))
                     }
+  scope :filter_by_before_startdate, ->(date) { left_joins(:location).where("lettings_logs.startdate >= ?", date) }
 
   AUTOGENERATED_FIELDS = %w[id status created_at updated_at discarded_at].freeze
   OPTIONAL_FIELDS = %w[first_time_property_let_as_social_housing tenancycode propcode].freeze
diff --git a/app/models/location.rb b/app/models/location.rb
index 2df620292..25fc05fd9 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -4,6 +4,7 @@ class Location < ApplicationRecord
   validates :units, :type_of_unit, :mobility_type, presence: true
   belongs_to :scheme
   has_many :lettings_logs, class_name: "LettingsLog"
+  has_many :location_deactivation_periods, class_name: "LocationDeactivationPeriod"
 
   has_paper_trail
 
@@ -11,7 +12,7 @@ class Location < ApplicationRecord
 
   auto_strip_attributes :name
 
-  attr_accessor :add_another_location, :deactivation_date_type, :run_deactivation_validations
+  attr_accessor :add_another_location, :deactivation_date_type, :deactivation_date, :run_deactivation_validations
 
   scope :search_by_postcode, ->(postcode) { where("REPLACE(postcode, ' ', '') ILIKE ?", "%#{postcode.delete(' ')}%") }
   scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") }
@@ -374,8 +375,9 @@ class Location < ApplicationRecord
   end
 
   def status
-    return :active if deactivation_date.blank?
-    return :deactivating_soon if Time.zone.now < deactivation_date
+    recent_deactivation = location_deactivation_periods.deactivations_without_reactivation.first
+    return :active if recent_deactivation.blank?
+    return :deactivating_soon if Time.zone.now < recent_deactivation.deactivation_date
 
     :deactivated
   end
@@ -403,7 +405,7 @@ class Location < ApplicationRecord
       end
     else
       collection_start_date = FormHandler.instance.current_collection_start_date
-      unless deactivation_date.between?(collection_start_date, Date.new(2200, 1, 1))
+      unless deactivation_date.between?(collection_start_date, Time.zone.local(2200, 1, 1))
         errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.out_of_range", date: collection_start_date.to_formatted_s(:govuk_date)))
       end
     end
diff --git a/app/models/location_deactivation_period.rb b/app/models/location_deactivation_period.rb
new file mode 100644
index 000000000..b8579f0bf
--- /dev/null
+++ b/app/models/location_deactivation_period.rb
@@ -0,0 +1,3 @@
+class LocationDeactivationPeriod < ApplicationRecord
+  scope :deactivations_without_reactivation, -> { where(reactivation_date: nil) }
+end
diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb
index a41b202ad..9f87ec566 100644
--- a/app/views/schemes/check_answers.html.erb
+++ b/app/views/schemes/check_answers.html.erb
@@ -73,7 +73,7 @@
               <% row.cell(text: simple_format("<span>#{location.type_of_unit}</span>")) %>
               <% row.cell(text: location.mobility_type) %>
               <% row.cell(text: simple_format(location_cell_location_admin_district(location, get_location_change_link_href_location_admin_district(@scheme, location)), wrapper_tag: "div")) %>
-              <% row.cell(text: location.startdate&.to_formatted_s(:govuk_date)) %>
+              <% row.cell(text: location_availability(location)) %>
               <% end %>
           <% end %>
         <% end %>
diff --git a/db/migrate/20221115101732_add_deactivations_table.rb b/db/migrate/20221115101732_add_deactivations_table.rb
new file mode 100644
index 000000000..b8d055a0d
--- /dev/null
+++ b/db/migrate/20221115101732_add_deactivations_table.rb
@@ -0,0 +1,10 @@
+class AddDeactivationsTable < ActiveRecord::Migration[7.0]
+  def change
+    create_table :location_deactivation_periods do |t|
+      t.datetime :deactivation_date
+      t.datetime :reactivation_date
+      t.belongs_to :location
+      t.timestamps
+    end
+  end
+end
diff --git a/db/migrate/20221115113437_remove_deactivatio_date.rb b/db/migrate/20221115113437_remove_deactivatio_date.rb
new file mode 100644
index 000000000..c21c671bb
--- /dev/null
+++ b/db/migrate/20221115113437_remove_deactivatio_date.rb
@@ -0,0 +1,13 @@
+class RemoveDeactivatioDate < ActiveRecord::Migration[7.0]
+  def up
+    change_table :locations, bulk: true do |t|
+      t.remove :deactivation_date
+    end
+  end
+
+  def down
+    change_table :locations, bulk: true do |t|
+      t.column :deactivation_date, :datetime
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 1ff8f157c..2644c8b0b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do
+ActiveRecord::Schema[7.0].define(version: 2022_11_15_113437) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -245,6 +245,15 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do
     t.index ["scheme_id"], name: "index_lettings_logs_on_scheme_id"
   end
 
+  create_table "location_deactivation_periods", force: :cascade do |t|
+    t.datetime "deactivation_date"
+    t.datetime "reactivation_date"
+    t.bigint "location_id"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["location_id"], name: "index_location_deactivation_periods_on_location_id"
+  end
+
   create_table "locations", force: :cascade do |t|
     t.string "location_code"
     t.string "postcode"
@@ -260,7 +269,6 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do
     t.datetime "startdate", precision: nil
     t.string "location_admin_district"
     t.boolean "confirmed"
-    t.datetime "deactivation_date"
     t.index ["old_id"], name: "index_locations_on_old_id", unique: true
     t.index ["scheme_id"], name: "index_locations_on_scheme_id"
   end
diff --git a/spec/factories/location_deactivation_period.rb b/spec/factories/location_deactivation_period.rb
new file mode 100644
index 000000000..6a3ab70c3
--- /dev/null
+++ b/spec/factories/location_deactivation_period.rb
@@ -0,0 +1,5 @@
+FactoryBot.define do
+  factory :location_deactivation_period do
+    reactivation_date { nil }
+  end
+end
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index 1500a728c..67660b8ad 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -766,7 +766,7 @@ RSpec.describe "Schemes scheme Features" do
                 expect(page).to have_content(location.type_of_unit)
                 expect(page).to have_content(location.mobility_type)
                 expect(page).to have_content(location.location_code)
-                expect(page).to have_content("Available from 4 April 2022")
+                expect(page).to have_content("Active from 4 April 2022")
                 expect(page).to have_content("Active")
               end
 
diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb
index 2adfab35f..e0d2abfea 100644
--- a/spec/helpers/locations_helper_spec.rb
+++ b/spec/helpers/locations_helper_spec.rb
@@ -59,18 +59,35 @@ RSpec.describe LocationsHelper do
         { name: "Common type of unit", value: location.type_of_unit },
         { name: "Mobility type", value: location.mobility_type },
         { name: "Code", value: location.location_code },
-        { name: "Availability", value: "Available from 8 August 2022" },
+        { name: "Availability", value: "Active from 8 August 2022" },
         { name: "Status", value: :active },
       ]
 
       expect(display_location_attributes(location)).to eq(attributes)
     end
 
-    it "displays created_at as availability date if startdate is not present" do
-      location.update!(startdate: nil)
-      availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value]
+    context "when viewing availability" do
+      context "with are no deactivations" do
+        it "displays created_at as availability date if startdate is not present" do
+          location.update!(startdate: nil)
+          availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value]
 
-      expect(availability_attribute).to eq("Available from #{location.created_at.to_formatted_s(:govuk_date)}")
+          expect(availability_attribute).to eq("Active from #{location.created_at.to_formatted_s(:govuk_date)}")
+        end
+      end
+
+      context "with previous deactivations" do
+        before do
+          location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 8, 10), reactivation_date: Time.zone.local(2022, 9, 1))
+          location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 9, 15), reactivation_date: nil)
+        end
+
+        it "displays the timeline of availability" do
+          availability_attribute = display_location_attributes(location).find { |x| x[:name] == "Availability" }[:value]
+
+          expect(availability_attribute).to eq("Active from 8 August 2022 to 9 August 2022\nDeactivated on 10 August 2022\nActive from 1 September 2022 to 14 September 2022\nDeactivated on 15 September 2022")
+        end
+      end
     end
   end
 end
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index 069bb21d7..2a23f0299 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -119,32 +119,56 @@ RSpec.describe Location, type: :model do
       Timecop.freeze(2022, 6, 7)
     end
 
-    it "returns active if the location is not deactivated" do
-      location.deactivation_date = nil
-      location.deactivation_date_type = nil
-      location.save!
-      expect(location.status).to eq(:active)
-    end
+    context "when there have not been any previous deactivations" do
+      it "returns active if the location has no deactivation records" do
+        expect(location.status).to eq(:active)
+      end
 
-    it "returns deactivating soon if deactivation_date is in the future" do
-      location.deactivation_date = Time.zone.local(2022, 8, 8)
-      location.deactivation_date_type = "other"
-      location.save!
-      expect(location.status).to eq(:deactivating_soon)
-    end
+      it "returns deactivating soon if deactivation_date is in the future" do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 8, 8))
+        location.save!
+        expect(location.status).to eq(:deactivating_soon)
+      end
 
-    it "returns deactivated if deactivation_date is in the past" do
-      location.deactivation_date = Time.zone.local(2022, 4, 8)
-      location.deactivation_date_type = "other"
-      location.save!
-      expect(location.status).to eq(:deactivated)
+      it "returns deactivated if deactivation_date is in the past" do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 6))
+        location.save!
+        expect(location.status).to eq(:deactivated)
+      end
+
+      it "returns deactivated if deactivation_date is today" do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 7))
+        location.save!
+        expect(location.status).to eq(:deactivated)
+      end
     end
 
-    it "returns deactivated if deactivation_date is today" do
-      location.deactivation_date = Time.zone.local(2022, 6, 7)
-      location.deactivation_date_type = "other"
-      location.save!
-      expect(location.status).to eq(:deactivated)
+    context "when there have been previous deactivations" do
+      before do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 4), reactivation_date: Time.zone.local(2022, 6, 5))
+      end
+
+      it "returns active if the location has no relevant deactivation records" do
+        expect(location.status).to eq(:active)
+      end
+
+      it "returns deactivating soon if deactivation_date is in the future" do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 8, 8))
+        location.save!
+        expect(location.status).to eq(:deactivating_soon)
+      end
+
+      it "returns deactivated if deactivation_date is in the past" do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 6))
+        location.save!
+        expect(location.status).to eq(:deactivated)
+      end
+
+      it "returns deactivated if deactivation_date is today" do
+        location.location_deactivation_periods << FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 6, 7))
+        location.save!
+        expect(location.status).to eq(:deactivated)
+      end
     end
   end
 
diff --git a/spec/requests/locations_controller_spec.rb b/spec/requests/locations_controller_spec.rb
index 2aca9bd93..ff3d77ab0 100644
--- a/spec/requests/locations_controller_spec.rb
+++ b/spec/requests/locations_controller_spec.rb
@@ -1239,8 +1239,9 @@ RSpec.describe LocationsController, type: :request do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
       let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
       let!(:location) { FactoryBot.create(:location, scheme:) }
-      let(:startdate) { Time.utc(2021, 1, 2) }
       let(:deactivation_date) { Time.utc(2022, 10, 10) }
+      let!(:lettings_log) { FactoryBot.create(:lettings_log, :sh, location:, scheme:, startdate:, owning_organisation: user.organisation) }
+      let(:startdate) { Time.utc(2022, 10, 11) }
 
       before do
         Timecop.freeze(Time.utc(2022, 10, 10))
@@ -1282,7 +1283,30 @@ RSpec.describe LocationsController, type: :request do
           expect(response).to have_http_status(:ok)
           expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success")
           location.reload
-          expect(location.deactivation_date).to eq(deactivation_date)
+          expect(location.location_deactivation_periods.count).to eq(1)
+          expect(location.location_deactivation_periods.first.deactivation_date).to eq(deactivation_date)
+        end
+
+        context "and a log startdate is after location deactivation date" do
+          it "clears the location and scheme answers" do
+            expect(lettings_log.location).to eq(location)
+            expect(lettings_log.scheme).to eq(scheme)
+            lettings_log.reload
+            expect(lettings_log.location).to eq(nil)
+            expect(lettings_log.scheme).to eq(nil)
+          end
+        end
+
+        context "and a log startdate is before location deactivation date" do
+          let(:startdate) { Time.utc(2022, 10, 9) }
+
+          it "does not update the log" do
+            expect(lettings_log.location).to eq(location)
+            expect(lettings_log.scheme).to eq(scheme)
+            lettings_log.reload
+            expect(lettings_log.location).to eq(location)
+            expect(lettings_log.scheme).to eq(scheme)
+          end
         end
       end
 
@@ -1368,19 +1392,18 @@ RSpec.describe LocationsController, type: :request do
       let(:user) { FactoryBot.create(:user, :data_coordinator) }
       let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
       let!(:location) { FactoryBot.create(:location, scheme:) }
+      let(:add_deactivations) { location.location_deactivation_periods << location_deactivation_period }
 
       before do
         Timecop.freeze(Time.utc(2022, 10, 10))
         sign_in user
-        location.deactivation_date = deactivation_date
-        location.deactivation_date_type = deactivation_date_type
+        add_deactivations
         location.save!
         get "/schemes/#{scheme.id}/locations/#{location.id}"
       end
 
       context "with active location" do
-        let(:deactivation_date) { nil }
-        let(:deactivation_date_type) { nil }
+        let(:add_deactivations) {}
 
         it "renders deactivate this location" do
           expect(response).to have_http_status(:ok)
@@ -1389,8 +1412,7 @@ RSpec.describe LocationsController, type: :request do
       end
 
       context "with deactivated location" do
-        let(:deactivation_date) { Time.utc(2022, 10, 9) }
-        let(:deactivation_date_type) { "other" }
+        let(:location_deactivation_period) { FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 10, 9)) }
 
         it "renders reactivate this location" do
           expect(response).to have_http_status(:ok)
@@ -1399,8 +1421,7 @@ RSpec.describe LocationsController, type: :request do
       end
 
       context "with location that's deactivating soon" do
-        let(:deactivation_date) { Time.utc(2022, 10, 12) }
-        let(:deactivation_date_type) { "other" }
+        let(:location_deactivation_period) { FactoryBot.create(:location_deactivation_period, deactivation_date: Time.zone.local(2022, 10, 12)) }
 
         it "renders reactivate this location" do
           expect(response).to have_http_status(:ok)