From ef354fada695d7f7ceaca928b07f7d5344760005 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Wed, 9 Nov 2022 15:42:52 +0000
Subject: [PATCH 1/9] Update routes (#978)

* Update routes

* Remove member from routes
---
 app/controllers/locations_controller.rb           | 12 ++++++------
 app/controllers/schemes_controller.rb             |  2 +-
 app/helpers/check_answers_helper.rb               |  6 +++---
 app/views/locations/edit.html.erb                 |  2 +-
 app/views/locations/edit_local_authority.html.erb |  2 +-
 app/views/locations/edit_name.html.erb            |  2 +-
 app/views/locations/index.html.erb                |  2 +-
 app/views/locations/new.html.erb                  |  2 +-
 app/views/schemes/check_answers.html.erb          |  2 +-
 config/routes.rb                                  |  9 +++------
 10 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb
index 532ab915a..0a857bb13 100644
--- a/app/controllers/locations_controller.rb
+++ b/app/controllers/locations_controller.rb
@@ -23,9 +23,9 @@ class LocationsController < ApplicationController
       @location = Location.new(location_params)
       if @location.save
         if @location.location_admin_district.nil?
-          redirect_to(location_edit_local_authority_path(id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location]))
+          redirect_to(scheme_location_edit_local_authority_path(scheme_id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location]))
         elsif location_params[:add_another_location] == "Yes"
-          redirect_to new_location_path(@scheme)
+          redirect_to new_scheme_location_path(@scheme)
         else
           check_answers_path = @scheme.confirmed? ? scheme_check_answers_path(@scheme, anchor: "locations") : scheme_check_answers_path(@scheme)
           redirect_to check_answers_path
@@ -69,9 +69,9 @@ class LocationsController < ApplicationController
         case page
         when "edit"
           if @location.location_admin_district.nil?
-            redirect_to(location_edit_local_authority_path(id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location]))
+            redirect_to(scheme_location_edit_local_authority_path(scheme_id: @scheme.id, location_id: @location.id, add_another_location: location_params[:add_another_location]))
           elsif location_params[:add_another_location] == "Yes"
-            redirect_to(new_location_path(@location.scheme))
+            redirect_to(new_scheme_location_path(@location.scheme))
           else
             redirect_to(scheme_check_answers_path(@scheme, anchor: "locations"))
           end
@@ -79,7 +79,7 @@ class LocationsController < ApplicationController
           redirect_to(scheme_check_answers_path(@scheme, anchor: "locations"))
         when "edit-local-authority"
           if params[:add_another_location] == "Yes"
-            redirect_to(new_location_path(@location.scheme))
+            redirect_to(new_scheme_location_path(@location.scheme))
           else
             redirect_to(scheme_check_answers_path(@scheme, anchor: "locations"))
           end
@@ -107,7 +107,7 @@ private
 
   def find_scheme
     @scheme = if %w[new create index edit_name].include?(action_name)
-                Scheme.find(params[:id])
+                Scheme.find(params[:scheme_id])
               else
                 @location&.scheme
               end
diff --git a/app/controllers/schemes_controller.rb b/app/controllers/schemes_controller.rb
index d4168e355..9a259e3a1 100644
--- a/app/controllers/schemes_controller.rb
+++ b/app/controllers/schemes_controller.rb
@@ -176,7 +176,7 @@ private
     when "secondary-client-group"
       scheme_support_path(@scheme)
     when "support"
-      new_location_path
+      new_scheme_location_path(@scheme)
     when "details"
       if @scheme.arrangement_type_before_type_cast == "D"
         scheme_primary_client_group_path(@scheme)
diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb
index b334ab073..1583b3791 100644
--- a/app/helpers/check_answers_helper.rb
+++ b/app/helpers/check_answers_helper.rb
@@ -18,14 +18,14 @@ module CheckAnswersHelper
 
   def get_location_change_link_href_postcode(scheme, location)
     if location.confirmed?
-      location_edit_name_path(id: scheme.id, location_id: location.id)
+      scheme_location_edit_name_path(scheme_id: scheme.id, location_id: location.id)
     else
-      location_edit_path(id: scheme.id, location_id: location.id)
+      edit_scheme_location_path(scheme_id: scheme.id, id: location.id)
     end
   end
 
   def get_location_change_link_href_location_admin_district(scheme, location)
-    location_edit_local_authority_path(id: scheme.id, location_id: location.id)
+    scheme_location_edit_local_authority_path(scheme_id: scheme.id, location_id: location.id)
   end
 
   def any_questions_have_summary_card_number?(subsection, lettings_log)
diff --git a/app/views/locations/edit.html.erb b/app/views/locations/edit.html.erb
index 6e14503ed..dca0d04fb 100644
--- a/app/views/locations/edit.html.erb
+++ b/app/views/locations/edit.html.erb
@@ -7,7 +7,7 @@
   ) %>
 <% end %>
 
-<%= form_for(@location, method: :patch, url: location_path(@location)) do |f| %>
+<%= form_for(@location, method: :patch, url: scheme_location_path(scheme_id: @scheme.id, id: @location.id)) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>
diff --git a/app/views/locations/edit_local_authority.html.erb b/app/views/locations/edit_local_authority.html.erb
index 43c351d28..1e5ab9fbb 100644
--- a/app/views/locations/edit_local_authority.html.erb
+++ b/app/views/locations/edit_local_authority.html.erb
@@ -5,7 +5,7 @@
   ) %>
 <% end %>
 
-<%= form_for(@location, method: :patch, url: location_path(location_id: @location.id, add_another_location: params[:add_another_location])) do |f| %>
+<%= form_for(@location, method: :patch, url: scheme_location_path(scheme_id: @scheme.id, id: @location.id, add_another_location: params[:add_another_location])) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>
diff --git a/app/views/locations/edit_name.html.erb b/app/views/locations/edit_name.html.erb
index 087ca0812..ece4df866 100644
--- a/app/views/locations/edit_name.html.erb
+++ b/app/views/locations/edit_name.html.erb
@@ -7,7 +7,7 @@
   ) %>
 <% end %>
 
-<%= form_for(@location, method: :patch, url: location_path(location_id: @location.id)) do |f| %>
+<%= form_for(@location, method: :patch, url: scheme_location_path(scheme_id: @scheme.id, id: @location.id)) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>
diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb
index 6672fc661..703fd9fa5 100644
--- a/app/views/locations/index.html.erb
+++ b/app/views/locations/index.html.erb
@@ -62,6 +62,6 @@
     <% end %>
   <% end %>
 <% end %>
-<%= govuk_button_link_to "Add a location", new_location_path(id: @scheme.id), secondary: true %>
+<%= govuk_button_link_to "Add a location", new_scheme_location_path(scheme_id: @scheme.id), secondary: true %>
 
 <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "locations" } %>
diff --git a/app/views/locations/new.html.erb b/app/views/locations/new.html.erb
index 71690f8e6..69f56744b 100644
--- a/app/views/locations/new.html.erb
+++ b/app/views/locations/new.html.erb
@@ -7,7 +7,7 @@
   ) %>
 <% end %>
 
-<%= form_for(@location, method: :post, url: locations_path) do |f| %>
+<%= form_for(@location, method: :post, url: scheme_locations_path) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>
diff --git a/app/views/schemes/check_answers.html.erb b/app/views/schemes/check_answers.html.erb
index 2cb154e1f..a41b202ad 100644
--- a/app/views/schemes/check_answers.html.erb
+++ b/app/views/schemes/check_answers.html.erb
@@ -78,7 +78,7 @@
           <% end %>
         <% end %>
       <% end %>
-      <%= govuk_button_link_to "Add a location", new_location_path(id: @scheme.id), secondary: true %>
+      <%= govuk_button_link_to "Add a location", new_scheme_location_path(scheme_id: @scheme.id), secondary: true %>
     <% end %>
   <% end %>
   <%= f.hidden_field :page, value: "check-answers" %>
diff --git a/config/routes.rb b/config/routes.rb
index 9d5083e1f..eab3d3222 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -50,12 +50,9 @@ Rails.application.routes.draw do
     get "edit-name", to: "schemes#edit_name"
     get "support-services-provider", to: "schemes#support_services_provider"
 
-    member do
-      resources :locations do
-        get "edit-name", to: "locations#edit_name"
-        get "edit", to: "locations#edit"
-        get "edit-local-authority", to: "locations#edit_local_authority"
-      end
+    resources :locations do
+      get "edit-name", to: "locations#edit_name"
+      get "edit-local-authority", to: "locations#edit_local_authority"
     end
   end
 

From a4fc795e31fba1914eab6eba68ded9131d73f1e7 Mon Sep 17 00:00:00 2001
From: James Rose <james@jbpr.net>
Date: Thu, 10 Nov 2022 09:50:44 +0000
Subject: [PATCH 2/9] Parse meta fields where namespace declarations aren't
 included (#983)

We import XML files that define logs from the previous CORE service. For an unknown reason, some of these files don't include the required namespace declarations to parse `meta` values. Instead of changing the source, this change introduces a new method that forces the namespace declaration for meta fields.
---
 app/services/imports/import_service.rb               |  8 ++++++--
 .../imports/lettings_logs_field_import_service.rb    | 10 +++++-----
 app/services/imports/lettings_logs_import_service.rb | 12 ++++++------
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/app/services/imports/import_service.rb b/app/services/imports/import_service.rb
index e45498004..1db1f4d6a 100644
--- a/app/services/imports/import_service.rb
+++ b/app/services/imports/import_service.rb
@@ -19,8 +19,12 @@ module Imports
       end
     end
 
-    def field_value(xml_document, namespace, field)
-      xml_document.at_xpath("//#{namespace}:#{field}")&.text
+    def field_value(xml_document, namespace, field, *args)
+      xml_document.at_xpath("//#{namespace}:#{field}", *args)&.text
+    end
+
+    def meta_field_value(xml_document, field)
+      field_value(xml_document, "meta", field, { "meta" => "http://data.gov.uk/core/metadata" })
     end
 
     def overridden?(xml_document, namespace, field)
diff --git a/app/services/imports/lettings_logs_field_import_service.rb b/app/services/imports/lettings_logs_field_import_service.rb
index 218decb7a..e59e178f2 100644
--- a/app/services/imports/lettings_logs_field_import_service.rb
+++ b/app/services/imports/lettings_logs_field_import_service.rb
@@ -16,8 +16,8 @@ module Imports
   private
 
     def update_lettings_allocation(xml_doc)
-      old_id = field_value(xml_doc, "meta", "document-id")
-      previous_status = field_value(xml_doc, "meta", "status")
+      old_id = meta_field_value(xml_doc, "document-id")
+      previous_status = meta_field_value(xml_doc, "status")
       record = LettingsLog.find_by(old_id:)
 
       if record.present? && previous_status.include?("submitted")
@@ -46,11 +46,11 @@ module Imports
     end
 
     def update_major_repairs(xml_doc)
-      old_id = field_value(xml_doc, "meta", "document-id")
+      old_id = meta_field_value(xml_doc, "document-id")
       record = LettingsLog.find_by(old_id:)
 
       if record.present?
-        previous_status = field_value(xml_doc, "meta", "status")
+        previous_status = meta_field_value(xml_doc, "status")
         major_repairs_date = compose_date(xml_doc, "MRCDAY", "MRCMONTH", "MRCYEAR")
         major_repairs = if major_repairs_date.present? && previous_status.include?("submitted")
                           1
@@ -69,7 +69,7 @@ module Imports
     end
 
     def update_tenant_code(xml_doc)
-      old_id = field_value(xml_doc, "meta", "document-id")
+      old_id = meta_field_value(xml_doc, "document-id")
       record = LettingsLog.find_by(old_id:)
 
       if record.present?
diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index c7721b666..1aecc4f81 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -57,7 +57,7 @@ module Imports
     def create_log(xml_doc)
       attributes = {}
 
-      previous_status = field_value(xml_doc, "meta", "status")
+      previous_status = meta_field_value(xml_doc, "status")
 
       # Required fields for status complete or logic to work
       # Note: order matters when we derive from previous values (attributes parameter)
@@ -191,9 +191,9 @@ module Imports
 
       # Not specific to our form but required for consistency (present in import)
       attributes["old_form_id"] = safe_string_as_integer(xml_doc, "FORM")
-      attributes["created_at"] = Time.zone.parse(field_value(xml_doc, "meta", "created-date"))
-      attributes["updated_at"] = Time.zone.parse(field_value(xml_doc, "meta", "modified-date"))
-      attributes["old_id"] = field_value(xml_doc, "meta", "document-id")
+      attributes["created_at"] = Time.zone.parse(meta_field_value(xml_doc, "created-date"))
+      attributes["updated_at"] = Time.zone.parse(meta_field_value(xml_doc, "modified-date"))
+      attributes["old_id"] = meta_field_value(xml_doc, "document-id")
 
       # Required for our form invalidated questions (not present in import)
       attributes["previous_la_known"] = attributes["prevloc"].nil? ? 0 : 1
@@ -236,7 +236,7 @@ module Imports
       attributes["net_income_value_check"] = 0
 
       # Sets the log creator
-      owner_id = field_value(xml_doc, "meta", "owner-user-id").strip
+      owner_id = meta_field_value(xml_doc, "owner-user-id").strip
       if owner_id.present?
         user = LegacyUser.find_by(old_user_id: owner_id)&.user
         @logger.warn "Missing user! We expected to find a legacy user with old_user_id #{owner_id}" unless user
@@ -355,7 +355,7 @@ module Imports
     end
 
     def get_form_name_component(xml_doc, index)
-      form_name = field_value(xml_doc, "meta", "form-name")
+      form_name = meta_field_value(xml_doc, "form-name")
       form_type_components = form_name.split("-")
       form_type_components[index]
     end

From ed956b23d32f5ac94a41bee7e3622e3b16897d94 Mon Sep 17 00:00:00 2001
From: James Rose <james@jbpr.net>
Date: Thu, 10 Nov 2022 14:41:17 +0000
Subject: [PATCH 3/9] Add validation to dependent ccharge field (#979)

Although both fields are on the same page, we need to add a validation here so that we can clear both values on import.
---
 app/models/validations/financial_validations.rb       | 1 +
 spec/models/validations/financial_validations_spec.rb | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb
index 4dec7bb78..b1bddd9cb 100644
--- a/app/models/validations/financial_validations.rb
+++ b/app/models/validations/financial_validations.rb
@@ -110,6 +110,7 @@ module Validations::FinancialValidations
     if record.is_carehome?
       period = record.form.get_question("period", record).label_from_value(record.period).downcase
       if record.chcharge.blank?
+        record.errors.add :is_carehome, I18n.t("validations.financial.carehome.not_provided", period:)
         record.errors.add :chcharge, I18n.t("validations.financial.carehome.not_provided", period:)
       elsif !weekly_value_in_range(record, "chcharge", 10, 1000)
         max_chcharge = record.weekly_to_value_per_period(1000)
diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb
index c4c4958db..0676e900b 100644
--- a/spec/models/validations/financial_validations_spec.rb
+++ b/spec/models/validations/financial_validations_spec.rb
@@ -1010,6 +1010,8 @@ RSpec.describe Validations::FinancialValidations do
           financial_validator.validate_care_home_charges(record)
           expect(record.errors["chcharge"])
             .to include(match I18n.t("validations.financial.carehome.not_provided", period: "every 4 weeks"))
+          expect(record.errors["is_carehome"])
+            .to include(match I18n.t("validations.financial.carehome.not_provided", period: "every 4 weeks"))
         end
       end
 

From 219bf937ccdbb39d28c3b66b262c0bfa6492ef9a Mon Sep 17 00:00:00 2001
From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com>
Date: Thu, 10 Nov 2022 14:44:29 +0000
Subject: [PATCH 4/9] CLDC-1694 Fix issue with non-renewal lettings allowing
 'renewal' for vacancy reason (#982)

* feat: add renewal validation on rsnvac

* test: check sensible error seen when rsnvac is renewal and but letting is not a renewal

* test: change renewal validation check to fetch error message dynamically

* feat: lint code

* feat: write record.renewal.present?, not record.renewal

Co-authored-by: James Rose <james@jbpr.net>

Co-authored-by: James Rose <james@jbpr.net>
---
 app/models/validations/property_validations.rb       |  4 ++++
 config/locales/en.yml                                |  1 +
 spec/models/validations/property_validations_spec.rb | 11 +++++++++++
 3 files changed, 16 insertions(+)

diff --git a/app/models/validations/property_validations.rb b/app/models/validations/property_validations.rb
index d5ab8e9e0..c655d8c85 100644
--- a/app/models/validations/property_validations.rb
+++ b/app/models/validations/property_validations.rb
@@ -38,6 +38,10 @@ module Validations::PropertyValidations
       record.errors.add :rsnvac, I18n.t("validations.property.rsnvac.referral_invalid")
       record.errors.add :referral, I18n.t("validations.household.referral.rsnvac_non_temp")
     end
+
+    if record.renewal.present? && record.renewal.zero? && record.rsnvac == 14
+      record.errors.add :rsnvac, I18n.t("validations.property.rsnvac.not_a_renewal")
+    end
   end
 
   def validate_unitletas(record)
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 55ea3a0f7..9a45d96dd 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -141,6 +141,7 @@ en:
         previous_let_social: "Property cannot have a previous let type if being let as social housing for the first time"
         non_temp_accommodation: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as this accommodation is not temporary"
         referral_invalid: "Answer cannot be re-let to tenant who occupied the same property as temporary accommodation as a different source of referral for this letting"
+        not_a_renewal: "Reason for vacancy cannot be 'Renewal of fixed-term tenancy' if letting is not a renewal"
       unittype_gn:
         one_bedroom_bedsit: "A bedsit can only have one bedroom"
         one_seven_bedroom_shared: "A shared house must have 1 to 7 bedrooms"
diff --git a/spec/models/validations/property_validations_spec.rb b/spec/models/validations/property_validations_spec.rb
index 153934ebc..b3965b3d6 100644
--- a/spec/models/validations/property_validations_spec.rb
+++ b/spec/models/validations/property_validations_spec.rb
@@ -233,6 +233,17 @@ RSpec.describe Validations::PropertyValidations do
         property_validator.validate_rsnvac(record)
         expect(record.errors["rsnvac"]).to be_empty
       end
+
+      context "when the letting is not a renewal" do
+        it "validates that the reason for vacancy is not renewal" do
+          record.first_time_property_let_as_social_housing = 0
+          record.renewal = 0
+          record.rsnvac = 14
+          property_validator.validate_rsnvac(record)
+          expect(record.errors["rsnvac"])
+                .to include(match I18n.t("validations.property.rsnvac.not_a_renewal"))
+        end
+      end
     end
 
     context "when the property has been let before" do

From 978d72f7e04b6ce0b7952833a9156e44a74e617b Mon Sep 17 00:00:00 2001
From: James Rose <james@jbpr.net>
Date: Thu, 10 Nov 2022 14:52:39 +0000
Subject: [PATCH 5/9] Import locations that belong to now inactive schemes
 (#984)

We were previously blocking the import of locations that belonged to schemes where the end date was before the current date. This meant that we were unable to import logs that are associated with schemes that _were_ valid, but have since expired.
---
 .../imports/scheme_location_import_service.rb | 32 ++++++++-----------
 .../scheme_location_import_service_spec.rb    | 15 ---------
 2 files changed, 13 insertions(+), 34 deletions(-)

diff --git a/app/services/imports/scheme_location_import_service.rb b/app/services/imports/scheme_location_import_service.rb
index cdcd9448d..4214555cc 100644
--- a/app/services/imports/scheme_location_import_service.rb
+++ b/app/services/imports/scheme_location_import_service.rb
@@ -97,25 +97,19 @@ module Imports
     end
 
     def add_location(scheme, attributes)
-      if attributes["end_date"].nil? || attributes["end_date"] >= Time.zone.now
-        begin
-          Location.create!(
-            name: attributes["location_name"],
-            postcode: attributes["postcode"],
-            mobility_type: attributes["mobility_type"],
-            units: attributes["units"],
-            type_of_unit: attributes["type_of_unit"],
-            old_visible_id: attributes["location_old_visible_id"],
-            old_id: attributes["location_old_id"],
-            startdate: attributes["start_date"],
-            scheme:,
-          )
-        rescue ActiveRecord::RecordNotUnique
-          @logger.warn("Location is already present with legacy ID #{attributes['location_old_id']}, skipping")
-        end
-      else
-        @logger.warn("Location with legacy ID #{attributes['location_old_id']} is expired (#{attributes['end_date']}), skipping")
-      end
+      Location.create!(
+        name: attributes["location_name"],
+        postcode: attributes["postcode"],
+        mobility_type: attributes["mobility_type"],
+        units: attributes["units"],
+        type_of_unit: attributes["type_of_unit"],
+        old_visible_id: attributes["location_old_visible_id"],
+        old_id: attributes["location_old_id"],
+        startdate: attributes["start_date"],
+        scheme:,
+      )
+    rescue ActiveRecord::RecordNotUnique
+      @logger.warn("Location is already present with legacy ID #{attributes['location_old_id']}, skipping")
     end
 
     def find_scheme_to_merge(attributes)
diff --git a/spec/services/imports/scheme_location_import_service_spec.rb b/spec/services/imports/scheme_location_import_service_spec.rb
index 70ff039cd..8b93d4969 100644
--- a/spec/services/imports/scheme_location_import_service_spec.rb
+++ b/spec/services/imports/scheme_location_import_service_spec.rb
@@ -160,21 +160,6 @@ RSpec.describe Imports::SchemeLocationImportService do
       expect(location.scheme.confirmed).to be_truthy
     end
 
-    context "and the end date is before the current date" do
-      before do
-        Timecop.freeze(2022, 6, 1)
-        location_xml.at_xpath("//scheme:end-date").content = "2022-05-01"
-      end
-
-      after { Timecop.unfreeze }
-
-      it "does not create the location" do
-        expect(logger).to receive(:warn).with("Location with legacy ID 0ae7ad6dc0f1cf7ef33c18cc8c108bebc1b4923e is expired (2022-05-01 00:00:00 +0100), skipping")
-        expect { location_service.create_scheme_location(location_xml) }
-          .not_to change(Location, :count)
-      end
-    end
-
     context "and we import the same location twice" do
       before { location_service.create_scheme_location(location_xml) }
 

From 4a3172e82c496f78a86a7673131859b82329d662 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Thu, 10 Nov 2022 14:53:22 +0000
Subject: [PATCH 6/9] Cldc 1667 location details page (#976)

* Add show location page

* Add availability to the locations table

* Add active status

* Add an empty deactivate page and button

* styling

* lint

* Extract feature toggle

* Update route

* Move display_attributes

* update path

* update paths
---
 app/controllers/locations_controller.rb    |  6 ++++
 app/helpers/locations_helper.rb            | 14 ++++++++
 app/helpers/tag_helper.rb                  |  2 ++
 app/models/location.rb                     | 16 ++++-----
 app/views/locations/edit_name.html.erb     |  2 +-
 app/views/locations/index.html.erb         |  2 +-
 app/views/locations/show.html.erb          | 28 +++++++++++++++
 app/views/locations/toggle_active.html.erb |  0
 config/initializers/feature_toggle.rb      |  6 ++++
 config/routes.rb                           |  1 +
 spec/features/schemes_spec.rb              | 42 +++++++++++++++++++---
 spec/helpers/locations_helper_spec.rb      | 27 ++++++++++++++
 12 files changed, 131 insertions(+), 15 deletions(-)
 create mode 100644 app/views/locations/show.html.erb
 create mode 100644 app/views/locations/toggle_active.html.erb

diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb
index 0a857bb13..797e79544 100644
--- a/app/controllers/locations_controller.rb
+++ b/app/controllers/locations_controller.rb
@@ -18,6 +18,12 @@ class LocationsController < ApplicationController
     @location = Location.new
   end
 
+  def show; end
+
+  def deactivate
+    render "toggle_active", locals: { action: "deactivate" }
+  end
+
   def create
     if date_params_missing?(location_params) || valid_date_params?(location_params)
       @location = Location.new(location_params)
diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb
index e0fe9d18c..e21a4724e 100644
--- a/app/helpers/locations_helper.rb
+++ b/app/helpers/locations_helper.rb
@@ -22,4 +22,18 @@ module LocationsHelper
 
     resource.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) }
   end
+
+  def display_attributes(location)
+    [
+      { name: "Postcode", value: location.postcode },
+      { name: "Local authority", value: location.location_admin_district },
+      { name: "Location name", value: location.name, edit: true },
+      { name: "Total number of units at this location", value: location.units },
+      { 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: "Status", value: location.status },
+    ]
+  end
 end
diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb
index 1937394b0..08b6180b3 100644
--- a/app/helpers/tag_helper.rb
+++ b/app/helpers/tag_helper.rb
@@ -6,6 +6,7 @@ module TagHelper
     cannot_start_yet: "Cannot start yet",
     in_progress: "In progress",
     completed: "Completed",
+    active: "Active",
   }.freeze
 
   COLOUR = {
@@ -13,6 +14,7 @@ module TagHelper
     cannot_start_yet: "grey",
     in_progress: "blue",
     completed: "green",
+    active: "green",
   }.freeze
 
   def status_tag(status, classes = [])
diff --git a/app/models/location.rb b/app/models/location.rb
index 40a115d43..0fdc24a8e 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -359,14 +359,6 @@ class Location < ApplicationRecord
 
   enum type_of_unit: TYPE_OF_UNIT
 
-  def display_attributes
-    [
-      { name: "Location code ", value: location_code, suffix: false },
-      { name: "Postcode", value: postcode, suffix: county },
-      { name: "Type of unit", value: type_of_unit, suffix: false },
-    ]
-  end
-
   def postcode=(postcode)
     if postcode
       super UKPostcode.parse(postcode).to_s
@@ -375,6 +367,14 @@ class Location < ApplicationRecord
     end
   end
 
+  def available_from
+    startdate || created_at
+  end
+
+  def status
+    "active"
+  end
+
 private
 
   PIO = PostcodeService.new
diff --git a/app/views/locations/edit_name.html.erb b/app/views/locations/edit_name.html.erb
index ece4df866..7e75edf9d 100644
--- a/app/views/locations/edit_name.html.erb
+++ b/app/views/locations/edit_name.html.erb
@@ -3,7 +3,7 @@
 <% content_for :before_content do %>
   <%= govuk_back_link(
     text: "Back",
-    href: "/schemes/#{@scheme.id}/locations",
+    href: scheme_location_path(@scheme, @location),
   ) %>
 <% end %>
 
diff --git a/app/views/locations/index.html.erb b/app/views/locations/index.html.erb
index 703fd9fa5..5b2cbae47 100644
--- a/app/views/locations/index.html.erb
+++ b/app/views/locations/index.html.erb
@@ -52,7 +52,7 @@
     <%= table.body do |body| %>
       <%= body.row do |row| %>
         <% row.cell(text: location.id) %>
-        <% row.cell(text: simple_format(location_cell_postcode(location, "/schemes/#{@scheme.id}/locations/#{location.id}/edit-name"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %>
+        <% row.cell(text: simple_format(location_cell_postcode(location, "/schemes/#{@scheme.id}/locations/#{location.id}"), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %>
         <% row.cell(text: location.units) %>
         <% row.cell(text: simple_format("<span>#{location.type_of_unit}</span>")) %>
         <% row.cell(text: location.mobility_type) %>
diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb
new file mode 100644
index 000000000..fdceb77ad
--- /dev/null
+++ b/app/views/locations/show.html.erb
@@ -0,0 +1,28 @@
+<% title = @location.name %>
+<% content_for :title, title %>
+
+<% content_for :before_content do %>
+  <%= govuk_back_link(
+    text: "Back",
+    href: scheme_locations_path(@scheme),
+  ) %>
+<% end %>
+
+<%= render partial: "organisations/headings", locals: { main: @location.postcode, sub: @location.name } %>
+
+<div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds-from-desktop">
+        <%= govuk_summary_list do |summary_list| %>
+            <% display_attributes(@location).each do |attr| %>
+                <%= summary_list.row do |row| %>
+                <% row.key { attr[:name] } %>
+                <% row.value { attr[:name].eql?("Status") ? status_tag(attr[:value]) : details_html(attr) } %>
+                <% row.action(text: "Change", href: scheme_location_edit_name_path(scheme_id: @scheme.id, location_id: @location.id)) if attr[:edit] %>
+                <% end %>
+            <% end %>
+        <% end %>
+    </div>
+</div>
+<% if FeatureToggle.location_toggle_enabled? %>
+    <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %>
+<% end %>
diff --git a/app/views/locations/toggle_active.html.erb b/app/views/locations/toggle_active.html.erb
new file mode 100644
index 000000000..e69de29bb
diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb
index ca4315e9e..d3ab1db57 100644
--- a/config/initializers/feature_toggle.rb
+++ b/config/initializers/feature_toggle.rb
@@ -14,4 +14,10 @@ class FeatureToggle
 
     false
   end
+
+  def self.location_toggle_enabled?
+    return true unless Rails.env.production?
+
+    false
+  end
 end
diff --git a/config/routes.rb b/config/routes.rb
index eab3d3222..ad088fe11 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -53,6 +53,7 @@ Rails.application.routes.draw do
     resources :locations do
       get "edit-name", to: "locations#edit_name"
       get "edit-local-authority", to: "locations#edit_local_authority"
+      get "deactivate", to: "locations#deactivate"
     end
   end
 
diff --git a/spec/features/schemes_spec.rb b/spec/features/schemes_spec.rb
index 35a380fb5..48f386fa7 100644
--- a/spec/features/schemes_spec.rb
+++ b/spec/features/schemes_spec.rb
@@ -692,7 +692,7 @@ RSpec.describe "Schemes scheme Features" do
 
         context "when I click to see individual scheme" do
           let(:scheme) { schemes.first }
-          let!(:location) { FactoryBot.create(:location, scheme:) }
+          let!(:location) { FactoryBot.create(:location, startdate: Time.zone.local(2022, 4, 4), scheme:) }
 
           before do
             click_link(scheme.service_name)
@@ -757,11 +757,31 @@ RSpec.describe "Schemes scheme Features" do
                 click_link(location.postcode)
               end
 
-              it "shows available fields to edit" do
-                expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/edit-name")
+              it "displays details about the selected location" do
+                expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}")
+                expect(page).to have_content(location.postcode)
+                expect(page).to have_content(location.location_admin_district)
+                expect(page).to have_content(location.name)
+                expect(page).to have_content(location.units)
+                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")
+              end
+
+              it "only allows to edit the location name" do
+                assert_selector "a", text: "Change", count: 1
+
+                click_link("Change")
                 expect(page).to have_content "Location name for #{location.postcode}"
               end
 
+              it "allows to deactivate a location" do
+                click_link("Deactivate this location")
+                expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}/deactivate")
+              end
+
               context "when I press the back button" do
                 before do
                   click_link "Back"
@@ -775,15 +795,27 @@ RSpec.describe "Schemes scheme Features" do
 
               context "and I change the location name" do
                 before do
-                  fill_in "location-name-field", with: "NewName"
-                  click_button "Save and continue"
+                  click_link("Change")
                 end
 
                 it "returns to locations check your answers page and shows the new name" do
+                  fill_in "location-name-field", with: "NewName"
+                  click_button "Save and continue"
                   expect(page).to have_content location.id
                   expect(page).to have_content "NewName"
                   expect(page.current_url.split("/").last).to eq("check-answers#locations")
                 end
+
+                context "when I press the back button" do
+                  before do
+                    click_link "Back"
+                  end
+
+                  it "I see location details" do
+                    expect(page).to have_content location.name
+                    expect(page).to have_current_path("/schemes/#{scheme.id}/locations/#{location.id}")
+                  end
+                end
               end
             end
 
diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb
index 402772dec..9a4550912 100644
--- a/spec/helpers/locations_helper_spec.rb
+++ b/spec/helpers/locations_helper_spec.rb
@@ -46,4 +46,31 @@ RSpec.describe LocationsHelper do
       expect(selection_options(%w[example])).to eq([OpenStruct.new(id: "example", name: "Example")])
     end
   end
+
+  describe "display_attributes" do
+    let(:location) { FactoryBot.build(:location, startdate: Time.zone.local(2022, 8, 8)) }
+
+    it "returns correct display attributes" do
+      attributes = [
+        { name: "Postcode", value: location.postcode },
+        { name: "Local authority", value: location.location_admin_district },
+        { name: "Location name", value: location.name, edit: true },
+        { name: "Total number of units at this location", value: location.units },
+        { 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: "Status", value: "active" },
+      ]
+
+      expect(display_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_attributes(location).find { |x| x[:name] == "Availability" }[:value]
+
+      expect(availability_attribute).to eq("Available from #{location.created_at.to_formatted_s(:govuk_date)}")
+    end
+  end
 end

From 492ebb2d4bbdb66b98272d40c8cda3a60716c25e Mon Sep 17 00:00:00 2001
From: James Rose <james@jbpr.net>
Date: Fri, 11 Nov 2022 09:00:51 +0000
Subject: [PATCH 7/9] Add validation to dependent earnings and incfreq fields
 (#985)

We need to add a validation here so that we can clear both values on import. If we don't do this then the validation will continue to trigger and the importer will get stuck in an endless loop.
---
 app/models/validations/financial_validations.rb       | 2 ++
 spec/models/validations/financial_validations_spec.rb | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb
index b1bddd9cb..87c506b4d 100644
--- a/app/models/validations/financial_validations.rb
+++ b/app/models/validations/financial_validations.rb
@@ -34,10 +34,12 @@ module Validations::FinancialValidations
 
     if record.earnings.present? && record.incfreq.blank?
       record.errors.add :incfreq, I18n.t("validations.financial.earnings.freq_missing")
+      record.errors.add :earnings, I18n.t("validations.financial.earnings.freq_missing")
     end
 
     if record.incfreq.present? && record.earnings.blank?
       record.errors.add :earnings, I18n.t("validations.financial.earnings.earnings_missing")
+      record.errors.add :incfreq, I18n.t("validations.financial.earnings.earnings_missing")
     end
   end
 
diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb
index 0676e900b..0b88fdd76 100644
--- a/spec/models/validations/financial_validations_spec.rb
+++ b/spec/models/validations/financial_validations_spec.rb
@@ -18,6 +18,8 @@ RSpec.describe Validations::FinancialValidations do
       financial_validator.validate_net_income(record)
       expect(record.errors["incfreq"])
         .to include(match I18n.t("validations.financial.earnings.freq_missing"))
+      expect(record.errors["earnings"])
+        .to include(match I18n.t("validations.financial.earnings.freq_missing"))
     end
 
     it "when income frequency is provided it validates that earnings must be provided" do
@@ -26,6 +28,8 @@ RSpec.describe Validations::FinancialValidations do
       financial_validator.validate_net_income(record)
       expect(record.errors["earnings"])
         .to include(match I18n.t("validations.financial.earnings.earnings_missing"))
+      expect(record.errors["incfreq"])
+        .to include(match I18n.t("validations.financial.earnings.earnings_missing"))
     end
   end
 

From 1630c386196ad15a811d4edd76da62588f199165 Mon Sep 17 00:00:00 2001
From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com>
Date: Fri, 11 Nov 2022 10:31:08 +0000
Subject: [PATCH 8/9] Cldc 1668 deactivate location (#981)

* add deactivation_date to locations

* Change conditional question controller to accommodate all models

* UI spike

* Update toggle-active views and render them correctly

* Display 2 errors

* Update errors

* Extract text to translation file

* Add collection start date

* Add out of range validation

* Update affected logs label

* lint

* Add status method

* update the displayed status tags

* Keep deactivation_date_type selected if an error occurs

* refactor deactivation_date_type to use default and other as options instead of 1 and 2

* refactor

* refactor

* update lettings logs

* Add reactivate ocation button and path

* Fix controller and update deactivate confirm page

* Don't actually update the logs data when deactivating a location

* lint and typos

* update a path

* update current_collection_start_date

* Remove unused scope
---
 app/controllers/locations_controller.rb       |  65 +++++-
 .../conditional_question_controller.js        |   6 +-
 app/helpers/question_attribute_helper.rb      |  10 +-
 app/helpers/tag_helper.rb                     |   4 +
 app/models/form_handler.rb                    |   4 +
 app/models/location.rb                        |   7 +-
 app/views/locations/show.html.erb             |   6 +-
 app/views/locations/toggle_active.html.erb    |  39 ++++
 .../locations/toggle_active_confirm.html.erb  |  16 ++
 config/locales/en.yml                         |  16 ++
 config/routes.rb                              |   2 +
 ...2033_add_deactivation_date_to_locations.rb |   5 +
 db/schema.rb                                  |   3 +-
 spec/helpers/locations_helper_spec.rb         |   2 +-
 .../helpers/question_attribute_helper_spec.rb |   2 +-
 spec/models/form_handler_spec.rb              |   4 +
 spec/models/location_spec.rb                  |  32 +++
 spec/requests/locations_controller_spec.rb    | 185 ++++++++++++++++++
 18 files changed, 396 insertions(+), 12 deletions(-)
 create mode 100644 app/views/locations/toggle_active_confirm.html.erb
 create mode 100644 db/migrate/20221109122033_add_deactivation_date_to_locations.rb

diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb
index 797e79544..6cd40161f 100644
--- a/app/controllers/locations_controller.rb
+++ b/app/controllers/locations_controller.rb
@@ -21,7 +21,23 @@ class LocationsController < ApplicationController
   def show; end
 
   def deactivate
-    render "toggle_active", locals: { action: "deactivate" }
+    if params[:location].blank?
+      render "toggle_active", locals: { action: "deactivate" }
+    elsif params[:location][:confirm].present? && params[:location][:deactivation_date].present?
+      confirm_deactivation
+    else
+      deactivation_date_errors
+      if @location.errors.present?
+        @location.deactivation_date_type = params[:location][:deactivation_date_type]
+        render "toggle_active", locals: { action: "deactivate" }, status: :unprocessable_entity
+      else
+        render "toggle_active_confirm", locals: { action: "deactivate", deactivation_date: }
+      end
+    end
+  end
+
+  def reactivate
+    render "toggle_active", locals: { action: "reactivate" }
   end
 
   def create
@@ -128,7 +144,7 @@ private
   end
 
   def authenticate_action!
-    if %w[new edit update create index edit_name edit_local_authority].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?)
+    if %w[new edit update create index edit_name edit_local_authority deactivate].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?)
       render_not_found and return
     end
   end
@@ -146,4 +162,49 @@ private
   def valid_location_admin_district?(location_params)
     location_params["location_admin_district"] != "Select an option"
   end
+
+  def confirm_deactivation
+    if @location.update(deactivation_date: params[:location][:deactivation_date])
+      flash[:notice] = "#{@location.name || @location.postcode} has been deactivated"
+    end
+    redirect_to scheme_location_path(@scheme, @location)
+  end
+
+  def deactivation_date_errors
+    if params[:location][:deactivation_date].blank? && params[:location][:deactivation_date_type].blank?
+      @location.errors.add(:deactivation_date_type, message: I18n.t("validations.location.deactivation_date.not_selected"))
+    end
+
+    if params[:location][:deactivation_date_type] == "other"
+      day = params[:location]["deactivation_date(3i)"]
+      month = params[:location]["deactivation_date(2i)"]
+      year = params[:location]["deactivation_date(1i)"]
+
+      collection_start_date = FormHandler.instance.current_collection_start_date
+
+      if [day, month, year].any?(&:blank?)
+        { day:, month:, year: }.each do |period, value|
+          @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.not_entered", period: period.to_s)) if value.blank?
+        end
+      elsif !Date.valid_date?(year.to_i, month.to_i, day.to_i)
+        @location.errors.add(:deactivation_date, message: I18n.t("validations.location.deactivation_date.invalid"))
+      elsif !Date.new(year.to_i, month.to_i, day.to_i).between?(collection_start_date, Date.new(2200, 1, 1))
+        @location.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
+  end
+
+  def deactivation_date
+    return if params[:location].blank?
+
+    collection_start_date = FormHandler.instance.current_collection_start_date
+    return collection_start_date if params[:location][:deactivation_date_type] == "default"
+    return params[:location][:deactivation_date] if params[:location][:deactivation_date_type].blank?
+
+    day = params[:location]["deactivation_date(3i)"]
+    month = params[:location]["deactivation_date(2i)"]
+    year = params[:location]["deactivation_date(1i)"]
+
+    Date.new(year.to_i, month.to_i, day.to_i)
+  end
 end
diff --git a/app/frontend/controllers/conditional_question_controller.js b/app/frontend/controllers/conditional_question_controller.js
index fb52d07c1..d24d5c6c3 100644
--- a/app/frontend/controllers/conditional_question_controller.js
+++ b/app/frontend/controllers/conditional_question_controller.js
@@ -10,14 +10,14 @@ export default class extends Controller {
       const selectedValue = this.element.value
       const dataInfo = JSON.parse(this.element.dataset.info)
       const conditionalFor = dataInfo.conditional_questions
-      const logType = dataInfo.log_type
+      const type = dataInfo.type
 
       Object.entries(conditionalFor).forEach(([targetQuestion, conditions]) => {
         if (!conditions.map(String).includes(String(selectedValue))) {
-          const textNumericInput = document.getElementById(`${logType}-log-${targetQuestion.replaceAll('_', '-')}-field`)
+          const textNumericInput = document.getElementById(`${type}-${targetQuestion.replaceAll('_', '-')}-field`)
           if (textNumericInput == null) {
             const dateInputs = [1, 2, 3].map((idx) => {
-              return document.getElementById(`${logType}_log_${targetQuestion}_${idx}i`)
+              return document.getElementById(`${type.replaceAll('-', '_')}_${targetQuestion}_${idx}i`)
             })
             this.clearDateInputs(dateInputs)
           } else {
diff --git a/app/helpers/question_attribute_helper.rb b/app/helpers/question_attribute_helper.rb
index 857ce5eb1..020ea1909 100644
--- a/app/helpers/question_attribute_helper.rb
+++ b/app/helpers/question_attribute_helper.rb
@@ -7,6 +7,14 @@ module QuestionAttributeHelper
     merge_controller_attributes(*attribs)
   end
 
+  def basic_conditional_html_attributes(conditional_for, type)
+    {
+      "data-controller": "conditional-question",
+      "data-action": "click->conditional-question#displayConditional",
+      "data-info": { conditional_questions: conditional_for, type: type }.to_json,
+    }
+  end
+
 private
 
   def numeric_question_html_attributes(question)
@@ -27,7 +35,7 @@ private
     {
       "data-controller": "conditional-question",
       "data-action": "click->conditional-question#displayConditional",
-      "data-info": { conditional_questions: question.conditional_for, log_type: question.form.type }.to_json,
+      "data-info": { conditional_questions: question.conditional_for, type: "#{question.form.type}-log" }.to_json,
     }
   end
 end
diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb
index 08b6180b3..2ea23a86a 100644
--- a/app/helpers/tag_helper.rb
+++ b/app/helpers/tag_helper.rb
@@ -7,6 +7,8 @@ module TagHelper
     in_progress: "In progress",
     completed: "Completed",
     active: "Active",
+    deactivating_soon: "Deactivating soon",
+    deactivated: "Deactivated",
   }.freeze
 
   COLOUR = {
@@ -15,6 +17,8 @@ module TagHelper
     in_progress: "blue",
     completed: "green",
     active: "green",
+    deactivating_soon: "yellow",
+    deactivated: "grey",
   }.freeze
 
   def status_tag(status, classes = [])
diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb
index c6dde13ab..c080e6e9b 100644
--- a/app/models/form_handler.rb
+++ b/app/models/form_handler.rb
@@ -49,6 +49,10 @@ class FormHandler
     today < window_end_date ? today.year - 1 : today.year
   end
 
+  def current_collection_start_date
+    Time.utc(current_collection_start_year, 4, 1)
+  end
+
   def form_name_from_start_year(year, type)
     form_mappings = { 0 => "current_#{type}", 1 => "previous_#{type}", -1 => "next_#{type}" }
     form_mappings[current_collection_start_year - year]
diff --git a/app/models/location.rb b/app/models/location.rb
index 0fdc24a8e..ab1c3620a 100644
--- a/app/models/location.rb
+++ b/app/models/location.rb
@@ -2,6 +2,7 @@ class Location < ApplicationRecord
   validate :validate_postcode
   validates :units, :type_of_unit, :mobility_type, presence: true
   belongs_to :scheme
+  has_many :lettings_logs, class_name: "LettingsLog"
 
   has_paper_trail
 
@@ -9,7 +10,7 @@ class Location < ApplicationRecord
 
   auto_strip_attributes :name
 
-  attr_accessor :add_another_location
+  attr_accessor :add_another_location, :deactivation_date_type
 
   scope :search_by_postcode, ->(postcode) { where("REPLACE(postcode, ' ', '') ILIKE ?", "%#{postcode.delete(' ')}%") }
   scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") }
@@ -372,7 +373,9 @@ class Location < ApplicationRecord
   end
 
   def status
-    "active"
+    return :active if deactivation_date.blank?
+    return :deactivating_soon if Time.zone.now < deactivation_date
+    return :deactivated if Time.zone.now >= deactivation_date
   end
 
 private
diff --git a/app/views/locations/show.html.erb b/app/views/locations/show.html.erb
index fdceb77ad..ccc0163c2 100644
--- a/app/views/locations/show.html.erb
+++ b/app/views/locations/show.html.erb
@@ -24,5 +24,9 @@
     </div>
 </div>
 <% if FeatureToggle.location_toggle_enabled? %>
-    <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %>
+    <% if @location.status == :active %>
+        <%= govuk_button_link_to "Deactivate this location", scheme_location_deactivate_path(scheme_id: @scheme.id, location_id: @location.id), warning: true %>
+    <% else %>
+        <%= govuk_button_link_to "Reactivate this location", scheme_location_reactivate_path(scheme_id: @scheme.id, location_id: @location.id) %>
+    <% end %>
 <% end %>
diff --git a/app/views/locations/toggle_active.html.erb b/app/views/locations/toggle_active.html.erb
index e69de29bb..96cda6daa 100644
--- a/app/views/locations/toggle_active.html.erb
+++ b/app/views/locations/toggle_active.html.erb
@@ -0,0 +1,39 @@
+<% title = "#{action.humanize} #{@location.postcode}" %>
+<% content_for :title, title %>
+
+<% content_for :before_content do %>
+  <%= govuk_back_link(
+    text: "Back",
+    href: scheme_location_path(scheme_id: @location.scheme.id, id: @location.id),
+  ) %>
+<% end %>
+
+<%= form_with model: @location, url: scheme_location_deactivate_path(scheme_id: @location.scheme.id, location_id: @location.id), method: "patch", local: true do |f| %>
+  <div class="govuk-grid-row">
+    <div class="govuk-grid-column-two-thirds">
+    <% collection_start_date = FormHandler.instance.current_collection_start_date %>
+    <%= f.govuk_error_summary %>
+        <%= f.govuk_radio_buttons_fieldset :deactivation_date_type,
+                                            legend: { text: I18n.t("questions.location.deactivation.apply_from") },
+                                            caption: { text: "Deactivate #{@location.postcode}" },
+                                            hint: { text: I18n.t("hints.location.deactivation", date: collection_start_date.to_formatted_s(:govuk_date)) } do %>
+            <%= govuk_warning_text text: I18n.t("warnings.location.deactivation.existing_logs") %>
+            <%= f.govuk_radio_button :deactivation_date_type,
+                                        "default",
+                                        label: { text: "From the start of the current collection period (#{collection_start_date.to_formatted_s(:govuk_date)})" } %>
+
+            <%= f.govuk_radio_button :deactivation_date_type,
+                                        "other",
+                                        label: { text: "For tenancies starting after a certain date" },
+                                        **basic_conditional_html_attributes({ "deactivation_date" => %w[other] }, "location") do %>
+                                          <%= f.govuk_date_field :deactivation_date,
+                                                        legend: { text: "Date", size: "m" },
+                                                        hint: { text: "For example, 27 3 2008" },
+                                                        width: 20 %>
+                                                        <% end %>
+        <% end %>
+
+        <%= f.govuk_submit "Continue" %>
+    </div>
+  </div>
+<% end %>
diff --git a/app/views/locations/toggle_active_confirm.html.erb b/app/views/locations/toggle_active_confirm.html.erb
new file mode 100644
index 000000000..452d66e48
--- /dev/null
+++ b/app/views/locations/toggle_active_confirm.html.erb
@@ -0,0 +1,16 @@
+<%= form_with model: @location, url: scheme_location_deactivate_path(@location), method: "patch", local: true do |f| %>
+     <% content_for :before_content do %>
+       <%= govuk_back_link(href: :back) %>
+     <% end %>
+   <h1 class="govuk-heading-l">
+     <span class="govuk-caption-l"><%= @location.postcode %></span>
+     <%= "This change will affect #{@location.lettings_logs.count} logs" %>
+   </h1>
+   <%= govuk_warning_text text: I18n.t("warnings.location.deactivation.review_logs") %>
+   <%= f.hidden_field :confirm, value: true %>
+   <%= f.hidden_field :deactivation_date, value: deactivation_date %>
+   <div class="govuk-button-group">
+     <%= f.govuk_submit "Deactivate this location" %>
+     <%= govuk_button_link_to "Cancel", scheme_location_path(scheme_id: @scheme, id: @location.id), html: { method: :get }, secondary: true %>
+   </div>
+ <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 9a45d96dd..3c4808696 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -312,6 +312,13 @@ en:
     declaration:
       missing: "You must show the DLUHC privacy notice to the tenant before you can submit this log."
 
+    location:
+      deactivation_date:
+        not_selected: "Select one of the options"
+        not_entered: "Enter a %{period}"
+        invalid: "Enter a valid date"
+        out_of_range: "The date must be on or after the %{date}"
+
   soft_validations:
     net_income:
       title_text: "Net income is outside the expected range based on the lead tenant’s working situation"
@@ -362,6 +369,8 @@ en:
       startdate: "When did the first property in this location become available under this scheme? (optional)"
       add_another_location: "Do you want to add another location?"
       mobility_type: "What are the mobility standards for the majority of units in this location?"
+      deactivation:
+        apply_from: "When should this change apply?"
     descriptions:
       location:
         mobility_type:
@@ -374,6 +383,13 @@ en:
       postcode: "For example, SW1P 4DF."
       name: "This is how you refer to this location within your organisation"
       units: "A unit can be a bedroom in a shared house or flat, or a house with 4 bedrooms. Do not include bedrooms used for wardens, managers, volunteers or sleep-in staff."
+      deactivation: "If the date is before %{date}, select ‘From the start of the current collection period’ because the previous period has now closed."
+  
+  warnings:
+    location:
+      deactivation: 
+        existing_logs: "It will not be possible to add logs with this location if their tenancy start date is on or after the date you enter. Any existing logs may be affected."
+        review_logs: "Your data providers will need to review these logs and answer a few questions again. We’ll email each log creator with a list of logs that need updating."
 
   test:
     one_argument: "This is based on the tenant’s work situation: %{ecstat1}"
diff --git a/config/routes.rb b/config/routes.rb
index ad088fe11..8bd78d6e6 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -54,6 +54,8 @@ Rails.application.routes.draw do
       get "edit-name", to: "locations#edit_name"
       get "edit-local-authority", to: "locations#edit_local_authority"
       get "deactivate", to: "locations#deactivate"
+      get "reactivate", to: "locations#reactivate"
+      patch "deactivate", to: "locations#deactivate"
     end
   end
 
diff --git a/db/migrate/20221109122033_add_deactivation_date_to_locations.rb b/db/migrate/20221109122033_add_deactivation_date_to_locations.rb
new file mode 100644
index 000000000..3360bfc7c
--- /dev/null
+++ b/db/migrate/20221109122033_add_deactivation_date_to_locations.rb
@@ -0,0 +1,5 @@
+class AddDeactivationDateToLocations < ActiveRecord::Migration[7.0]
+  def change
+    add_column :locations, :deactivation_date, :datetime
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 43fc94775..ea4b59670 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_10_19_082625) do
+ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -260,6 +260,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_10_19_082625) 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/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb
index 9a4550912..f2a5a67a5 100644
--- a/spec/helpers/locations_helper_spec.rb
+++ b/spec/helpers/locations_helper_spec.rb
@@ -60,7 +60,7 @@ RSpec.describe LocationsHelper do
         { name: "Mobility type", value: location.mobility_type },
         { name: "Code", value: location.location_code },
         { name: "Availability", value: "Available from 8 August 2022" },
-        { name: "Status", value: "active" },
+        { name: "Status", value: :active },
       ]
 
       expect(display_attributes(location)).to eq(attributes)
diff --git a/spec/helpers/question_attribute_helper_spec.rb b/spec/helpers/question_attribute_helper_spec.rb
index 2be903535..9d769cdf6 100644
--- a/spec/helpers/question_attribute_helper_spec.rb
+++ b/spec/helpers/question_attribute_helper_spec.rb
@@ -48,7 +48,7 @@ RSpec.describe QuestionAttributeHelper do
           "data-action": "input->numeric-question#calculateFields click->conditional-question#displayConditional",
           "data-target": "lettings-log-#{question.result_field.to_s.dasherize}-field",
           "data-calculated": question.fields_to_add.to_json,
-          "data-info": { conditional_questions: question.conditional_for, log_type: "lettings" }.to_json,
+          "data-info": { conditional_questions: question.conditional_for, type: "lettings-log" }.to_json,
         }
       end
 
diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb
index 81a9b1c45..c6a47c107 100644
--- a/spec/models/form_handler_spec.rb
+++ b/spec/models/form_handler_spec.rb
@@ -118,6 +118,10 @@ RSpec.describe FormHandler do
       it "returns the correct next sales form name" do
         expect(form_handler.form_name_from_start_year(2023, "sales")).to eq("next_sales")
       end
+
+      it "returns the correct current start date" do
+        expect(form_handler.current_collection_start_date).to eq(Time.utc(2022, 4, 1))
+      end
     end
 
     context "with the date before 1st of April" do
diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb
index 856575932..8f9f452bc 100644
--- a/spec/models/location_spec.rb
+++ b/spec/models/location_spec.rb
@@ -111,4 +111,36 @@ RSpec.describe Location, type: :model do
       end
     end
   end
+
+  describe "status" do
+    let(:location) { FactoryBot.build(:location) }
+
+    before do
+      Timecop.freeze(2022, 6, 7)
+    end
+
+    it "returns active if the location is not deactivated" do
+      location.deactivation_date = nil
+      location.save!
+      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.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.save!
+      expect(location.status).to eq(:deactivated)
+    end
+
+    it "returns deactivated if deactivation_date is today" do
+      location.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 96b3d1b21..feeae2b99 100644
--- a/spec/requests/locations_controller_spec.rb
+++ b/spec/requests/locations_controller_spec.rb
@@ -1212,4 +1212,189 @@ RSpec.describe LocationsController, type: :request do
       end
     end
   end
+
+  describe "#deactivate" do
+    context "when not signed in" do
+      it "redirects to the sign in page" do
+        patch "/schemes/1/locations/1/deactivate"
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    context "when signed in as a data provider" do
+      let(:user) { FactoryBot.create(:user) }
+
+      before do
+        sign_in user
+        patch "/schemes/1/locations/1/deactivate"
+      end
+
+      it "returns 401 unauthorized" do
+        request
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context "when signed in as a data coordinator" 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) }
+
+      before do
+        Timecop.freeze(Time.utc(2022, 10, 10))
+        sign_in user
+        patch "/schemes/#{scheme.id}/locations/#{location.id}/deactivate", params:
+      end
+
+      context "with default date" do
+        let(:params) { { location: { deactivation_date_type: "default" } } }
+
+        it "renders the confirmation page" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_content("This change will affect #{location.lettings_logs.count} logs")
+        end
+      end
+
+      context "with other date" do
+        let(:params) { { location: { deactivation_date: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "10", "deactivation_date(1i)": "2022" } } }
+
+        it "renders the confirmation page" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_content("This change will affect #{location.lettings_logs.count} logs")
+        end
+      end
+
+      context "when confirming deactivation" do
+        let(:params) { { location: { deactivation_date:, confirm: true } } }
+
+        it "updates existing location with valid deactivation date and renders location page" do
+          follow_redirect!
+          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)
+        end
+      end
+
+      context "when the date is not selected" do
+        let(:params) { { location: { "deactivation_date": "" } } }
+
+        it "displays the new page with an error message" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_selected"))
+        end
+      end
+
+      context "when invalid date is entered" do
+        let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "44", "deactivation_date(1i)": "2022" } } }
+
+        it "displays the new page with an error message" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.location.deactivation_date.invalid"))
+        end
+      end
+
+      context "when the date is entered is before the beginning of current collection window" do
+        let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "10", "deactivation_date(2i)": "4", "deactivation_date(1i)": "2020" } } }
+
+        it "displays the new page with an error message" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.location.deactivation_date.out_of_range", date: "1 April 2022"))
+        end
+      end
+
+      context "when the day is not entered" do
+        let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "", "deactivation_date(2i)": "2", "deactivation_date(1i)": "2022" } } }
+
+        it "displays page with an error message" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "day"))
+        end
+      end
+
+      context "when the month is not entered" do
+        let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "2", "deactivation_date(2i)": "", "deactivation_date(1i)": "2022" } } }
+
+        it "displays page with an error message" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "month"))
+        end
+      end
+
+      context "when the year is not entered" do
+        let(:params) { { location: { deactivation_date_type: "other", "deactivation_date(3i)": "2", "deactivation_date(2i)": "2", "deactivation_date(1i)": "" } } }
+
+        it "displays page with an error message" do
+          expect(response).to have_http_status(:unprocessable_entity)
+          expect(page).to have_content(I18n.t("validations.location.deactivation_date.not_entered", period: "year"))
+        end
+      end
+    end
+  end
+
+  describe "#show" do
+    context "when not signed in" do
+      it "redirects to the sign in page" do
+        get "/schemes/1/locations/1"
+        expect(response).to redirect_to("/account/sign-in")
+      end
+    end
+
+    context "when signed in as a data provider" do
+      let(:user) { FactoryBot.create(:user) }
+
+      before do
+        sign_in user
+        get "/schemes/1/locations/1"
+      end
+
+      it "returns 401 unauthorized" do
+        request
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context "when signed in as a data coordinator" do
+      let(:user) { FactoryBot.create(:user, :data_coordinator) }
+      let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: user.organisation) }
+      let!(:location) { FactoryBot.create(:location, scheme:) }
+
+      before do
+        Timecop.freeze(Time.utc(2022, 10, 10))
+        sign_in user
+        location.deactivation_date = deactivation_date
+        location.save!
+        get "/schemes/#{scheme.id}/locations/#{location.id}"
+      end
+
+      context "with active location" do
+        let(:deactivation_date) { nil }
+
+        it "renders deactivate this location" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_link("Deactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/deactivate")
+        end
+      end
+
+      context "with deactivated location" do
+        let(:deactivation_date) { Time.utc(2022, 10, 9) }
+
+        it "renders reactivate this location" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_link("Reactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/reactivate")
+        end
+      end
+
+      context "with location that's deactivating soon" do
+        let(:deactivation_date) { Time.utc(2022, 10, 12) }
+
+        it "renders reactivate this location" do
+          expect(response).to have_http_status(:ok)
+          expect(page).to have_link("Reactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/reactivate")
+        end
+      end
+    end
+  end
 end

From 8f9886cbc8538a99ed334cbe22244c96d6cb7cf4 Mon Sep 17 00:00:00 2001
From: James Rose <james@jbpr.net>
Date: Fri, 11 Nov 2022 11:30:15 +0000
Subject: [PATCH 9/9] Change type of legacy old_visible_id fields to strings
 (#986)

These fields are set using IDs from the previous CORE service. There was an incorrect assumption that these fields were integers so we were casting them as such. We have discovered that these fields are strings (e.g., `027`) so this change adjusts our types.
---
 .../imports/lettings_logs_import_service.rb         |  6 +++---
 .../imports/scheme_location_import_service.rb       |  2 +-
 .../20221111102656_change_old_visible_id_type.rb    | 13 +++++++++++++
 db/schema.rb                                        |  8 ++++----
 spec/factories/location.rb                          |  2 +-
 .../logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml   |  2 +-
 .../6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml    |  2 +-
 .../imports/lettings_logs_import_service_spec.rb    | 10 +++++-----
 .../imports/organisation_import_service_spec.rb     | 10 +++++-----
 spec/services/imports/scheme_import_service_spec.rb |  4 ++--
 .../imports/scheme_location_import_service_spec.rb  |  2 +-
 11 files changed, 37 insertions(+), 24 deletions(-)
 create mode 100644 db/migrate/20221111102656_change_old_visible_id_type.rb

diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb
index 1aecc4f81..ab15e7f3d 100644
--- a/app/services/imports/lettings_logs_import_service.rb
+++ b/app/services/imports/lettings_logs_import_service.rb
@@ -205,8 +205,8 @@ module Imports
 
       # Supported Housing fields
       if attributes["needstype"] == GN_SH[:supported_housing]
-        location_old_visible_id = safe_string_as_integer(xml_doc, "_1cschemecode")
-        scheme_old_visible_id = safe_string_as_integer(xml_doc, "_1cmangroupcode")
+        location_old_visible_id = string_or_nil(xml_doc, "_1cschemecode")
+        scheme_old_visible_id = string_or_nil(xml_doc, "_1cmangroupcode")
 
         schemes = Scheme.where(old_visible_id: scheme_old_visible_id, owning_organisation_id: attributes["owning_organisation_id"])
         location = Location.find_by(old_visible_id: location_old_visible_id, scheme: schemes)
@@ -399,7 +399,7 @@ module Imports
     end
 
     def find_organisation_id(xml_doc, id_field)
-      old_visible_id = unsafe_string_as_integer(xml_doc, id_field)
+      old_visible_id = string_or_nil(xml_doc, id_field)
       organisation = Organisation.find_by(old_visible_id:)
       raise "Organisation not found with legacy ID #{old_visible_id}" if organisation.nil?
 
diff --git a/app/services/imports/scheme_location_import_service.rb b/app/services/imports/scheme_location_import_service.rb
index 4214555cc..837be0b49 100644
--- a/app/services/imports/scheme_location_import_service.rb
+++ b/app/services/imports/scheme_location_import_service.rb
@@ -91,7 +91,7 @@ module Imports
       attributes["units"] = safe_string_as_integer(xml_doc, "total-units")
       attributes["type_of_unit"] = safe_string_as_integer(xml_doc, "unit-type")
       attributes["location_old_id"] = string_or_nil(xml_doc, "id")
-      attributes["location_old_visible_id"] = safe_string_as_integer(xml_doc, "visible-id")
+      attributes["location_old_visible_id"] = string_or_nil(xml_doc, "visible-id")
       attributes["scheme_old_id"] = string_or_nil(xml_doc, "mgmtgroup")
       attributes
     end
diff --git a/db/migrate/20221111102656_change_old_visible_id_type.rb b/db/migrate/20221111102656_change_old_visible_id_type.rb
new file mode 100644
index 000000000..d4c568bb7
--- /dev/null
+++ b/db/migrate/20221111102656_change_old_visible_id_type.rb
@@ -0,0 +1,13 @@
+class ChangeOldVisibleIdType < ActiveRecord::Migration[7.0]
+  def up
+    change_column :organisations, :old_visible_id, :string
+    change_column :schemes, :old_visible_id, :string
+    change_column :locations, :old_visible_id, :string
+  end
+
+  def down
+    change_column :organisations, :old_visible_id, :integer
+    change_column :schemes, :old_visible_id, :integer
+    change_column :locations, :old_visible_id, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ea4b59670..5faff28ba 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_09_122033) do
+ActiveRecord::Schema[7.0].define(version: 2022_11_11_102656) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -255,7 +255,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do
     t.integer "units"
     t.integer "type_of_unit"
     t.string "old_id"
-    t.integer "old_visible_id"
+    t.string "old_visible_id"
     t.string "mobility_type"
     t.datetime "startdate", precision: nil
     t.string "location_admin_district"
@@ -316,7 +316,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do
     t.integer "supported_housing_units"
     t.integer "unspecified_units"
     t.string "old_org_id"
-    t.integer "old_visible_id"
+    t.string "old_visible_id"
     t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true
   end
 
@@ -395,7 +395,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_11_09_122033) do
     t.bigint "managing_organisation_id"
     t.string "arrangement_type"
     t.string "old_id"
-    t.integer "old_visible_id"
+    t.string "old_visible_id"
     t.integer "total_units"
     t.boolean "confirmed"
     t.index ["managing_organisation_id"], name: "index_schemes_on_managing_organisation_id"
diff --git a/spec/factories/location.rb b/spec/factories/location.rb
index a8418f8ba..3359f64cd 100644
--- a/spec/factories/location.rb
+++ b/spec/factories/location.rb
@@ -17,7 +17,7 @@ FactoryBot.define do
       units { 20 }
       mobility_type { "A" }
       scheme { FactoryBot.create(:scheme, :export) }
-      old_visible_id { 111 }
+      old_visible_id { "111" }
     end
   end
 end
diff --git a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml
index 59c1eada7..3faacf28b 100644
--- a/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml
+++ b/spec/fixtures/imports/logs/0b4a68df-30cc-474a-93c0-a56ce8fdad3b.xml
@@ -19,7 +19,7 @@
     <FORM>123456</FORM>
     <Landlord source-value="2">2 Local Authority</Landlord>
     <Group>
-      <_1cmangroupcode>123</_1cmangroupcode>
+      <_1cmangroupcode>0123</_1cmangroupcode>
       <_1cschemecode>10</_1cschemecode>
       <schPC/>
       <Q1e>3 No</Q1e>
diff --git a/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml b/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml
index 5a7b23b5b..712d3f6e5 100644
--- a/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml
+++ b/spec/fixtures/imports/mgmtgroups/6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d.xml
@@ -5,5 +5,5 @@
     <mgmtgroup:agent>456</mgmtgroup:agent>
     <mgmtgroup:institution>7c5bd5fb549c09z2c55d9cb90d7ba84927e64618</mgmtgroup:institution>
     <mgmtgroup:status>Approved</mgmtgroup:status>
-    <mgmtgroup:visible-id>123</mgmtgroup:visible-id>
+    <mgmtgroup:visible-id>0123</mgmtgroup:visible-id>
 </mgmtgroup:mgmtgroup>
diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb
index 576249971..453ee7e30 100644
--- a/spec/services/imports/lettings_logs_import_service_spec.rb
+++ b/spec/services/imports/lettings_logs_import_service_spec.rb
@@ -11,8 +11,8 @@ RSpec.describe Imports::LettingsLogsImportService do
   let(:fixture_directory) { "spec/fixtures/imports/logs" }
 
   let(:organisation) { FactoryBot.create(:organisation, old_visible_id: "1", provider_type: "PRP") }
-  let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: 123, owning_organisation: organisation) }
-  let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: 456, owning_organisation: organisation) }
+  let(:scheme1) { FactoryBot.create(:scheme, old_visible_id: "0123", owning_organisation: organisation) }
+  let(:scheme2) { FactoryBot.create(:scheme, old_visible_id: "456", owning_organisation: organisation) }
 
   def open_file(directory, filename)
     File.open("#{directory}/#{filename}.xml")
@@ -23,16 +23,16 @@ RSpec.describe Imports::LettingsLogsImportService do
            .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Westminster","codes":{"admin_district":"E08000035"}}}', headers: {})
 
     allow(Organisation).to receive(:find_by).and_return(nil)
-    allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id.to_i).and_return(organisation)
+    allow(Organisation).to receive(:find_by).with(old_visible_id: organisation.old_visible_id).and_return(organisation)
 
     # Created by users
     FactoryBot.create(:user, old_user_id: "c3061a2e6ea0b702e6f6210d5c52d2a92612d2aa", organisation:)
     FactoryBot.create(:user, old_user_id: "e29c492473446dca4d50224f2bb7cf965a261d6f", organisation:)
 
     # Location setup
-    FactoryBot.create(:location, old_visible_id: 10, postcode: "LS166FT", scheme_id: scheme1.id, mobility_type: "W")
+    FactoryBot.create(:location, old_visible_id: "10", postcode: "LS166FT", scheme_id: scheme1.id, mobility_type: "W")
     FactoryBot.create(:location, scheme_id: scheme1.id)
-    FactoryBot.create(:location, old_visible_id: 10, postcode: "LS166FT", scheme_id: scheme2.id, mobility_type: "W")
+    FactoryBot.create(:location, old_visible_id: "10", postcode: "LS166FT", scheme_id: scheme2.id, mobility_type: "W")
 
     # Stub the form handler to use the real form
     allow(FormHandler.instance).to receive(:get_form).with("previous_lettings").and_return(real_2021_2022_form)
diff --git a/spec/services/imports/organisation_import_service_spec.rb b/spec/services/imports/organisation_import_service_spec.rb
index 48a0d72bb..c8fcd4829 100644
--- a/spec/services/imports/organisation_import_service_spec.rb
+++ b/spec/services/imports/organisation_import_service_spec.rb
@@ -32,7 +32,7 @@ RSpec.describe Imports::OrganisationImportService do
     it "successfully create an organisation with the expected data" do
       import_service.create_organisations(folder_name)
 
-      organisation = Organisation.find_by(old_visible_id: 1)
+      organisation = Organisation.find_by(old_visible_id: "1")
       expect(organisation.name).to eq("HA Ltd")
       expect(organisation.provider_type).to eq("PRP")
       expect(organisation.phone).to eq("xxxxxxxx")
@@ -53,7 +53,7 @@ RSpec.describe Imports::OrganisationImportService do
       expect(organisation.unspecified_units).to eq(0)
       expect(organisation.unspecified_units).to eq(0)
       expect(organisation.old_org_id).to eq("7c5bd5fb549c09z2c55d9cb90d7ba84927e64618")
-      expect(organisation.old_visible_id).to eq(1)
+      expect(organisation.old_visible_id).to eq("1")
     end
 
     it "successfully create multiple organisations" do
@@ -62,8 +62,8 @@ RSpec.describe Imports::OrganisationImportService do
       expect(storage_service).to receive(:get_file_io).with(filenames[1]).ordered
 
       expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(2)
-      expect(Organisation).to exist(old_visible_id: 1)
-      expect(Organisation).to exist(old_visible_id: 2)
+      expect(Organisation).to exist(old_visible_id: "1")
+      expect(Organisation).to exist(old_visible_id: "2")
     end
   end
 
@@ -86,7 +86,7 @@ RSpec.describe Imports::OrganisationImportService do
       expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(1)
       expect { import_service.create_organisations(folder_name) }.to change(Organisation, :count).by(0)
 
-      expect(Organisation).to exist(old_visible_id: 1, name: "HA Ltd")
+      expect(Organisation).to exist(old_visible_id: "1", name: "HA Ltd")
     end
   end
 end
diff --git a/spec/services/imports/scheme_import_service_spec.rb b/spec/services/imports/scheme_import_service_spec.rb
index efba81a1c..97e9359b5 100644
--- a/spec/services/imports/scheme_import_service_spec.rb
+++ b/spec/services/imports/scheme_import_service_spec.rb
@@ -10,7 +10,7 @@ RSpec.describe Imports::SchemeImportService do
   let(:scheme_id) { "6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d" }
 
   let!(:owning_org) { FactoryBot.create(:organisation, old_org_id: "7c5bd5fb549c09z2c55d9cb90d7ba84927e64618") }
-  let!(:managing_org) { FactoryBot.create(:organisation, old_visible_id: 456) }
+  let!(:managing_org) { FactoryBot.create(:organisation, old_visible_id: "456") }
 
   def open_file(directory, filename)
     File.open("#{directory}/#{filename}.xml")
@@ -46,7 +46,7 @@ RSpec.describe Imports::SchemeImportService do
       expect(scheme.owning_organisation).to eq(owning_org)
       expect(scheme.managing_organisation).to eq(managing_org)
       expect(scheme.old_id).to eq("6d6d7618b58affe2a150a5ef2e9f4765fa6cd05d")
-      expect(scheme.old_visible_id).to eq(123)
+      expect(scheme.old_visible_id).to eq("0123")
       expect(scheme.service_name).to eq("Management Group")
       expect(scheme.arrangement_type).to eq("Another organisation")
     end
diff --git a/spec/services/imports/scheme_location_import_service_spec.rb b/spec/services/imports/scheme_location_import_service_spec.rb
index 8b93d4969..c11c2ca2d 100644
--- a/spec/services/imports/scheme_location_import_service_spec.rb
+++ b/spec/services/imports/scheme_location_import_service_spec.rb
@@ -142,7 +142,7 @@ RSpec.describe Imports::SchemeLocationImportService do
       expect(location.mobility_type).to eq("Fitted with equipment and adaptations")
       expect(location.type_of_unit).to eq("Bungalow")
       expect(location.old_id).to eq(first_location_id)
-      expect(location.old_visible_id).to eq(10)
+      expect(location.old_visible_id).to eq("10")
       expect(location.startdate).to eq("1900-01-01")
       expect(location.scheme).to eq(scheme)
     end