From ebe0e2a5eecbf0df6cbe51dfde8a4c3c64b62bc7 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 17 Mar 2023 10:59:50 +0000 Subject: [PATCH] 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 * Update app/helpers/locations_helper.rb Co-authored-by: James Rose * Refactor * Seed review app * Change dates format * Update the local authority link data * Typo * update mapping in Lettings logs --------- Co-authored-by: James Rose --- app/helpers/locations_helper.rb | 17 +++++-- app/models/lettings_log.rb | 2 +- app/models/local_authority.rb | 3 ++ app/models/local_authority_link.rb | 4 ++ app/models/location.rb | 7 +++ app/models/log.rb | 12 ++--- .../imports/local_authority_links_service.rb | 22 +++++++++ .../local_authority_links_2023.csv | 35 ++++++++++++++ ...20230309145740_add_local_authority_link.rb | 12 +++++ db/schema.rb | 13 +++++- db/seeds.rb | 8 ++++ lib/tasks/local_authority_links.rake | 13 ++++++ .../files/local_authority_links_2023.csv | 6 +++ spec/helpers/locations_helper_spec.rb | 46 +++++++++++++++++++ .../local_authority_links_import_spec.rb | 40 ++++++++++++++++ spec/models/lettings_log_spec.rb | 34 ++++++++++++++ spec/models/location_spec.rb | 14 ++++++ spec/models/sales_log_spec.rb | 6 +-- 18 files changed, 280 insertions(+), 14 deletions(-) create mode 100644 app/models/local_authority_link.rb create mode 100644 app/services/imports/local_authority_links_service.rb create mode 100644 config/local_authorities_data/local_authority_links_2023.csv create mode 100644 db/migrate/20230309145740_add_local_authority_link.rb create mode 100644 lib/tasks/local_authority_links.rake create mode 100644 spec/fixtures/files/local_authority_links_2023.csv create mode 100644 spec/lib/tasks/local_authority_links_import_spec.rb diff --git a/app/helpers/locations_helper.rb b/app/helpers/locations_helper.rb index 8d6a46c26..5c51fc88c 100644 --- a/app/helpers/locations_helper.rb +++ b/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 diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index 57766a405..764b23f9a 100644 --- a/app/models/lettings_log.rb +++ b/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 diff --git a/app/models/local_authority.rb b/app/models/local_authority.rb index 22786ce7c..fe11adf04 100644 --- a/app/models/local_authority.rb +++ b/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 diff --git a/app/models/local_authority_link.rb b/app/models/local_authority_link.rb new file mode 100644 index 000000000..dcd339e76 --- /dev/null +++ b/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 diff --git a/app/models/location.rb b/app/models/location.rb index 0f90aedf5..7002ce5a9 100644 --- a/app/models/location.rb +++ b/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 diff --git a/app/models/log.rb b/app/models/log.rb index 256db3612..34c920bd5 100644 --- a/app/models/log.rb +++ b/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 diff --git a/app/services/imports/local_authority_links_service.rb b/app/services/imports/local_authority_links_service.rb new file mode 100644 index 000000000..bdcf8731c --- /dev/null +++ b/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 diff --git a/config/local_authorities_data/local_authority_links_2023.csv b/config/local_authorities_data/local_authority_links_2023.csv new file mode 100644 index 000000000..d388a36c1 --- /dev/null +++ b/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 diff --git a/db/migrate/20230309145740_add_local_authority_link.rb b/db/migrate/20230309145740_add_local_authority_link.rb new file mode 100644 index 000000000..6c1b0a079 --- /dev/null +++ b/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 diff --git a/db/schema.rb b/db/schema.rb index 3f21ceb23..daf421930 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: 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" diff --git a/db/seeds.rb b/db/seeds.rb index e677ad50e..f57bb85d7 100644 --- a/db/seeds.rb +++ b/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") diff --git a/lib/tasks/local_authority_links.rake b/lib/tasks/local_authority_links.rake new file mode 100644 index 000000000..0345d3dd2 --- /dev/null +++ b/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 diff --git a/spec/fixtures/files/local_authority_links_2023.csv b/spec/fixtures/files/local_authority_links_2023.csv new file mode 100644 index 000000000..5cf6f4798 --- /dev/null +++ b/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 diff --git a/spec/helpers/locations_helper_spec.rb b/spec/helpers/locations_helper_spec.rb index 4d58a7e8f..ae63c2e84 100644 --- a/spec/helpers/locations_helper_spec.rb +++ b/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 diff --git a/spec/lib/tasks/local_authority_links_import_spec.rb b/spec/lib/tasks/local_authority_links_import_spec.rb new file mode 100644 index 000000000..443a622cf --- /dev/null +++ b/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 diff --git a/spec/models/lettings_log_spec.rb b/spec/models/lettings_log_spec.rb index d233ecba9..b35ab5bb1 100644 --- a/spec/models/lettings_log_spec.rb +++ b/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 diff --git a/spec/models/location_spec.rb b/spec/models/location_spec.rb index 8d1284fa5..ceb87121f 100644 --- a/spec/models/location_spec.rb +++ b/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 diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 267de2246..7f6c74fc4 100644 --- a/spec/models/sales_log_spec.rb +++ b/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