CLDC-1999 Add local authority links and select correct LA from location (#1402)

* Add local authority links

* Display all local authorities for location

* set correct LA for log based on year

* Format local authorities for locations

* Rename variable

* Add import task for la links

* look at form start date because log start date might not be given

* Update app/models/lettings_log.rb

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

* Update app/helpers/locations_helper.rb

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

* Refactor

* Seed review app

* Change dates format

* Update the local authority link data

* Typo

* update mapping in Lettings logs

---------

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

app/helpers/locations_helper.rb

@@ -27,11 +27,11 @@ module LocationsHelper
     base_attributes = [
       { name: "Postcode", value: location.postcode, attribute: "postcode" },
       { name: "Location name", value: location.name, attribute: "name" },
-      { name: "Local authority", value: location.location_admin_district, attribute: "local_authority" },
+      { name: "Local authority", value: formatted_local_authority_timeline(location, "name"), attribute: "local_authority" },
       { name: "Number of units", value: location.units, attribute: "units" },
       { name: "Most common unit", value: location.type_of_unit, attribute: "type_of_unit" },
       { name: "Mobility standards", value: location.mobility_type, attribute: "mobility_standards" },
-      { name: "Location code", value: location.location_code, attribute: "location_code" },
+      { name: "Location code", value: formatted_local_authority_timeline(location, "code"), attribute: "location_code" },
       { name: "Availability", value: location_availability(location), attribute: "availability" },
     ]
 
@@ -46,7 +46,7 @@ module LocationsHelper
     [
       { name: "Postcode", value: location.postcode, attribute: "postcode" },
       { name: "Location name", value: location.name, attribute: "name" },
-      { name: "Local authority", value: location.location_admin_district, attribute: "local_authority" },
+      { name: "Local authority", value: formatted_local_authority_timeline(location, "name"), attribute: "local_authority" },
       { name: "Number of units", value: location.units, attribute: "units" },
       { name: "Most common unit", value: location.type_of_unit, attribute: "type_of_unit" },
       { name: "Mobility standards", value: location.mobility_type, attribute: "mobility_standards" },
@@ -107,4 +107,15 @@ private
 
     [inner.deactivation_date, inner.reactivation_date].all? { |date| date.between?(outer.deactivation_date, outer.reactivation_date) }
   end
+
+  def formatted_local_authority_timeline(location, field)
+    sorted_linked_authorities = location.linked_local_authorities.sort_by(&:start_date)
+    return sorted_linked_authorities.first[field] if sorted_linked_authorities.count == 1
+
+    sorted_linked_authorities.map { |linked_local_authority|
+      formatted_start_date = linked_local_authority.start_date.year == 2021 ? "until" : "#{linked_local_authority.start_date&.to_formatted_s(:govuk_date)} -"
+      formatted_end_date = linked_local_authority.end_date&.to_formatted_s(:govuk_date) || "present"
+      "#{linked_local_authority[field]} (#{formatted_start_date} #{formatted_end_date})"
+    }.join("\n")
+  end
 end

app/models/lettings_log.rb

@@ -84,7 +84,7 @@ class LettingsLog < Log
 
   def la
     if location
-      location.location_code
+      location.linked_local_authorities.active(form.start_date).first&.code
     else
       super
     end

app/models/local_authority.rb

@@ -1,4 +1,7 @@
 class LocalAuthority < ApplicationRecord
+  has_many :local_authority_links, dependent: :destroy
+  has_many :linked_local_authorities, class_name: "LocalAuthority", through: :local_authority_links
+
   scope :active, ->(date) { where("start_date <= ? AND (end_date IS NULL OR end_date >= ?)", date, date) }
   scope :england, -> { where("code LIKE ?", "E%") }
 end

app/models/local_authority_link.rb

@@ -0,0 +1,4 @@
+class LocalAuthorityLink < ApplicationRecord
+  belongs_to :local_authority, class_name: "LocalAuthority"
+  belongs_to :linked_local_authority, class_name: "LocalAuthority"
+end

app/models/location.rb

@@ -130,6 +130,13 @@ class Location < ApplicationRecord
     self.confirmed = [postcode, location_admin_district, location_code, units, type_of_unit, mobility_type].all?(&:present?)
   end
 
+  def linked_local_authorities
+    la = LocalAuthority.find_by(code: location_code)
+    return [] unless la
+
+    LocalAuthority.where(id: [la.id] + la.linked_local_authority_ids)
+  end
+
 private
 
   PIO = PostcodeService.new

app/models/log.rb

@@ -154,12 +154,12 @@ private
   end
 
   LA_CHANGES = {
-    "E07000027" => "E06000063", # Barrow-in-Furness => Cumberland
-    "E07000030" => "E06000063", # Eden => Cumberland
-    "E07000031" => "E06000063", # South Lakeland => Cumberland
-    "E07000026" => "E06000064", # Allerdale => Westmorland and Furness
-    "E07000028" => "E06000064", # Carlisle => Westmorland and Furness
-    "E07000029" => "E06000064", # Copeland => Westmorland and Furness
+    "E07000027" => "E06000064", # Barrow-in-Furness => Westmorland and Furness
+    "E07000030" => "E06000064", # Eden => Westmorland and Furness
+    "E07000031" => "E06000064", # South Lakeland => Westmorland and Furness
+    "E07000026" => "E06000063", # Allerdale => Cumberland
+    "E07000028" => "E06000063", # Carlisle => Cumberland
+    "E07000029" => "E06000063", # Copeland => Cumberland
     "E07000163" => "E06000065", # Craven => North Yorkshire
     "E07000164" => "E06000065", # Hambleton => North Yorkshire
     "E07000165" => "E06000065", # Harrogate => North Yorkshire

app/services/imports/local_authority_links_service.rb

@@ -0,0 +1,22 @@
+require "csv"
+
+module Imports
+  class LocalAuthorityLinksService
+    attr_reader :path, :count
+
+    def initialize(path:)
+      @path = path
+      @count = 0
+    end
+
+    def call
+      CSV.foreach(path, headers: true) do |row|
+        LocalAuthorityLink.upsert(
+          { local_authority_id: LocalAuthority.find_by(code: row["local_authority_code"]).id,
+            linked_local_authority_id: LocalAuthority.find_by(code: row["linked_local_authority_code"]).id },
+        )
+        @count += 1
+      end
+    end
+  end
+end

config/local_authorities_data/local_authority_links_2023.csv

@@ -0,0 +1,35 @@
+local_authority_code,linked_local_authority_code
+E06000064,E07000027
+E06000064,E07000030
+E06000064,E07000031
+E07000027,E06000064
+E07000030,E06000064
+E07000031,E06000064
+E06000063,E07000026
+E06000063,E07000028
+E06000063,E07000029
+E07000026,E06000063
+E07000028,E06000063
+E07000029,E06000063
+E06000065,E07000163
+E06000065,E07000164
+E06000065,E07000165
+E06000065,E07000166
+E06000065,E07000167
+E06000065,E07000168
+E06000065,E07000169
+E07000163,E06000065
+E07000164,E06000065
+E07000165,E06000065
+E07000166,E06000065
+E07000167,E06000065
+E07000168,E06000065
+E07000169,E06000065
+E06000066,E07000187
+E06000066,E07000188
+E06000066,E07000246
+E06000066,E07000189
+E07000187,E06000066
+E07000188,E06000066
+E07000246,E06000066
+E07000189,E06000066

db/migrate/20230309145740_add_local_authority_link.rb

@@ -0,0 +1,12 @@
+class AddLocalAuthorityLink < ActiveRecord::Migration[7.0]
+  def change
+    create_table :local_authority_links do |t|
+      t.references :local_authority
+      t.references :linked_local_authority
+
+      t.timestamps
+    end
+    add_foreign_key :local_authority_links, :local_authorities, column: :local_authority_id
+    add_foreign_key :local_authority_links, :local_authorities, column: :linked_local_authority_id
+  end
+end

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: 2023_03_08_101826) do
+ActiveRecord::Schema[7.0].define(version: 2023_03_09_145740) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -306,6 +306,15 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_08_101826) do
     t.index ["code"], name: "index_local_authority_code", unique: true
   end
 
+  create_table "local_authority_links", force: :cascade do |t|
+    t.bigint "local_authority_id"
+    t.bigint "linked_local_authority_id"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["linked_local_authority_id"], name: "index_local_authority_links_on_linked_local_authority_id"
+    t.index ["local_authority_id"], name: "index_local_authority_links_on_local_authority_id"
+  end
+
   create_table "location_deactivation_periods", force: :cascade do |t|
     t.datetime "deactivation_date"
     t.datetime "reactivation_date"
@@ -655,6 +664,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_03_08_101826) do
   add_foreign_key "lettings_logs", "locations"
   add_foreign_key "lettings_logs", "organisations", column: "owning_organisation_id", on_delete: :cascade
   add_foreign_key "lettings_logs", "schemes"
+  add_foreign_key "local_authority_links", "local_authorities"
+  add_foreign_key "local_authority_links", "local_authorities", column: "linked_local_authority_id"
   add_foreign_key "locations", "schemes"
   add_foreign_key "organisation_relationships", "organisations", column: "child_organisation_id"
   add_foreign_key "organisation_relationships", "organisations", column: "parent_organisation_id"

db/seeds.rb

@@ -309,6 +309,14 @@ unless Rails.env.test?
     pp "Seeded 3 dummy schemes"
   end
 
+  if (Rails.env.development? || Rails.env.review?) && LocalAuthorityLink.count.zero?
+    links_data_path = "config/local_authorities_data/local_authority_links_2023.csv"
+    service = Imports::LocalAuthorityLinksService.new(path: links_data_path)
+    service.call
+
+    pp "Seeded local authority links"
+  end
+
   if LaRentRange.count.zero?
     Dir.glob("config/rent_range_data/*.csv").each do |path|
       start_year = File.basename(path, ".csv")

lib/tasks/local_authority_links.rake

@@ -0,0 +1,13 @@
+namespace :data_import do
+  desc "Import local authority links data"
+  task :local_authority_links, %i[path] => :environment do |_task, args|
+    path = args[:path]
+
+    raise "Usage: rake data_import:local_authority_links['path/to/csv_file']" if path.blank?
+
+    service = Imports::LocalAuthorityLinksService.new(path:)
+    service.call
+
+    pp "Created/updated #{service.count} local authority link records" unless Rails.env.test?
+  end
+end

spec/fixtures/files/local_authority_links_2023.csv

@@ -0,0 +1,6 @@
+local_authority_code,linked_local_authority_code
+E06000063,E07000027
+E06000063,E07000030
+E06000063,E07000031
+E07000027,E06000063
+E07000030,E06000063

spec/helpers/locations_helper_spec.rb

@@ -152,6 +152,52 @@ RSpec.describe LocationsHelper do
       expect(display_location_attributes(location)).to eq(attributes)
     end
 
+    context "when location has different local authorities for different years" do
+      before do
+        LocalAuthorityLink.create!(local_authority_id: LocalAuthority.find_by(code: "E07000030").id, linked_local_authority_id: LocalAuthority.find_by(code: "E06000063").id)
+        location.update!(location_code: "E07000030")
+      end
+
+      it "returns correct display attributes" do
+        attributes = [
+          { attribute: "postcode", name: "Postcode", value: location.postcode },
+          { attribute: "name", name: "Location name", value: location.name },
+          { attribute: "local_authority", name: "Local authority", value: "Eden (until 31 March 2023)\nCumberland (1 April 2023 - present)" },
+          { attribute: "units", name: "Number of units", value: location.units },
+          { attribute: "type_of_unit", name: "Most common unit", value: location.type_of_unit },
+          { attribute: "mobility_standards", name: "Mobility standards", value: location.mobility_type },
+          { attribute: "location_code", name: "Location code", value: "E07000030 (until 31 March 2023)\nE06000063 (1 April 2023 - present)" },
+          { attribute: "availability", name: "Availability", value: "Active from 1 April 2022" },
+          { attribute: "status", name: "Status", value: :active },
+        ]
+
+        expect(display_location_attributes(location)).to eq(attributes)
+      end
+    end
+
+    context "when location has no local authority" do
+      before do
+        LocalAuthorityLink.create!(local_authority_id: LocalAuthority.find_by(code: "E07000030").id, linked_local_authority_id: LocalAuthority.find_by(code: "E06000063").id)
+        location.update!(location_code: nil)
+      end
+
+      it "returns correct display attributes" do
+        attributes = [
+          { attribute: "postcode", name: "Postcode", value: location.postcode },
+          { attribute: "name", name: "Location name", value: location.name },
+          { attribute: "local_authority", name: "Local authority", value: "" },
+          { attribute: "units", name: "Number of units", value: location.units },
+          { attribute: "type_of_unit", name: "Most common unit", value: location.type_of_unit },
+          { attribute: "mobility_standards", name: "Mobility standards", value: location.mobility_type },
+          { attribute: "location_code", name: "Location code", value: "" },
+          { attribute: "availability", name: "Availability", value: "Active from 1 April 2022" },
+          { attribute: "status", name: "Status", value: :incomplete },
+        ]
+
+        expect(display_location_attributes(location)).to eq(attributes)
+      end
+    end
+
     context "when viewing availability" do
       context "with no deactivations" do
         it "displays previous collection start date as availability date if created_at is earlier than collection start date" do

spec/lib/tasks/local_authority_links_import_spec.rb

@@ -0,0 +1,40 @@
+require "rails_helper"
+require "rake"
+
+RSpec.describe "data_import" do
+  describe ":local_authority_links", type: :task do
+    subject(:task) { Rake::Task["data_import:local_authority_links"] }
+
+    before do
+      LocalAuthorityLink.destroy_all
+      Rake.application.rake_require("tasks/local_authority_links")
+      Rake::Task.define_task(:environment)
+      task.reenable
+    end
+
+    context "when the rake task is run" do
+      let(:local_authority_links_file_path) { "./spec/fixtures/files/local_authority_links_2023.csv" }
+      let(:wrong_file_path) { "/test/no_csv_here.csv" }
+
+      it "creates new local authority links records" do
+        expect { task.invoke(local_authority_links_file_path) }.to change(LocalAuthorityLink, :count).by(5)
+        expect(LocalAuthorityLink.where(local_authority_id: LocalAuthority.find_by(code: "E06000063").id).exists?).to be true
+      end
+
+      it "raises an error when no path is given" do
+        expect { task.invoke(nil) }.to raise_error(RuntimeError, "Usage: rake data_import:local_authority_links['path/to/csv_file']")
+      end
+
+      it "raises an error when no file exists at the given path" do
+        expect { task.invoke(wrong_file_path) }.to raise_error(Errno::ENOENT)
+      end
+
+      context "when a record already exists with a matching ids" do
+        it "does not create a new link" do
+          task.invoke(local_authority_links_file_path)
+          expect { task.invoke(local_authority_links_file_path) }.to change(LocalAuthorityLink, :count).by(0)
+        end
+      end
+    end
+  end
+end

spec/models/lettings_log_spec.rb

@@ -1870,6 +1870,40 @@ RSpec.describe LettingsLog do
           expect(record_from_db["location_id"]).to eq(location.id)
           expect(lettings_log["location_id"]).to eq(location.id)
         end
+
+        context "and the location has multiple local authorities for different years" do
+          before do
+            LocalAuthorityLink.create!(local_authority_id: LocalAuthority.find_by(code: "E07000030").id, linked_local_authority_id: LocalAuthority.find_by(code: "E06000063").id)
+            location.update!(location_code: "E07000030")
+            Timecop.freeze(startdate)
+            Singleton.__init__(FormHandler)
+            lettings_log.update!(startdate:)
+            lettings_log.reload
+          end
+
+          after do
+            Timecop.unfreeze
+            Singleton.__init__(FormHandler)
+          end
+
+          context "with 22/23" do
+            let(:startdate) { Time.zone.local(2022, 4, 2) }
+
+            it "returns the correct la" do
+              expect(lettings_log["location_id"]).to eq(location.id)
+              expect(lettings_log.la).to eq("E07000030")
+            end
+          end
+
+          context "with 23/24" do
+            let(:startdate) { Time.zone.local(2023, 4, 2) }
+
+            it "returns the correct la" do
+              expect(lettings_log["location_id"]).to eq(location.id)
+              expect(lettings_log.la).to eq("E06000063")
+            end
+          end
+        end
       end
 
       context "and not renewal" do

spec/models/location_spec.rb

@@ -1,12 +1,19 @@
 require "rails_helper"
 
 RSpec.describe Location, type: :model do
+  before do
+    LocalAuthorityLink.create(local_authority_id: LocalAuthority.find_by(code: "E07000030").id, linked_local_authority_id: LocalAuthority.find_by(code: "E06000063").id)
+  end
+
   describe "#new" do
     let(:location) { FactoryBot.build(:location) }
 
     before do
       stub_request(:get, /api.postcodes.io/)
         .to_return(status: 200, body: "{\"status\":200,\"result\":{\"admin_district\":\"Manchester\",\"codes\":{\"admin_district\": \"E08000003\"}}}", headers: {})
+
+      stub_request(:get, /api.postcodes.io\/postcodes\/CA101AA/)
+      .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Eden","codes":{"admin_district":"E07000030"}}}', headers: {})
     end
 
     it "belongs to an organisation" do
@@ -18,6 +25,13 @@ RSpec.describe Location, type: :model do
       location.save!
       expect(location.location_code).to eq("E08000003")
     end
+
+    it "infers and returns the list of local authorities" do
+      location.update!(postcode: "CA10 1AA")
+      expect(location.linked_local_authorities.count).to eq(2)
+      expect(location.linked_local_authorities.active(Time.zone.local(2022, 4, 1)).first.code).to eq("E07000030")
+      expect(location.linked_local_authorities.active(Time.zone.local(2023, 4, 1)).first.code).to eq("E06000063")
+    end
   end
 
   describe "#postcode" do

spec/models/sales_log_spec.rb

@@ -264,7 +264,7 @@ RSpec.describe SalesLog, type: :model do
 
       before do
         WebMock.stub_request(:get, /api.postcodes.io\/postcodes\/CA101AA/)
-               .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Cumberland","codes":{"admin_district":"E06000063"}}}', headers: {})
+               .to_return(status: 200, body: '{"status":200,"result":{"admin_district":"Cumberland","codes":{"admin_district":"E06000064"}}}', headers: {})
 
         Timecop.freeze(2023, 5, 1)
         Singleton.__init__(FormHandler)
@@ -306,8 +306,8 @@ RSpec.describe SalesLog, type: :model do
 
       it "correctly infers new la" do
         record_from_db = ActiveRecord::Base.connection.execute("select la from sales_logs where id=#{address_sales_log_23_24.id}").to_a[0]
-        expect(address_sales_log_23_24.la).to eq("E06000063")
-        expect(record_from_db["la"]).to eq("E06000063")
+        expect(address_sales_log_23_24.la).to eq("E06000064")
+        expect(record_from_db["la"]).to eq("E06000064")
       end
     end