Merge pull request #487 from communitiesuk/import_organisation_rent_periods

CLDC-1133: Some organisations only use specific rent periods

app/models/organisation.rb

@@ -4,6 +4,7 @@ class Organisation < ApplicationRecord
   has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id"
   has_many :data_protection_confirmations
   has_many :organisation_las
+  has_many :organisation_rent_periods
 
   has_paper_trail
 
@@ -37,11 +38,21 @@ class Organisation < ApplicationRecord
   end
 
   def local_authorities
-    organisation_las.pluck(:ons_code).map { |ons_code| ons_code }
+    organisation_las.pluck(:ons_code)
   end
 
   def local_authority_names
-    local_authorities.map { |ons_code| LocalAuthority.ons_code_mappings[ons_code] }
+    names = local_authorities.map { |ons_code| LocalAuthority.ons_code_mappings[ons_code] }
+    names.presence || %w[All]
+  end
+
+  def rent_periods
+    organisation_rent_periods.pluck(:rent_period)
+  end
+
+  def rent_period_labels
+    labels = rent_periods.map { |period| RentPeriod.rent_period_mappings[period.to_s]["value"] }
+    labels.presence || %w[All]
   end
 
   def display_attributes
@@ -51,6 +62,7 @@ class Organisation < ApplicationRecord
       { name: "telephone_number", value: phone, editable: true },
       { name: "type", value: "Org type", editable: false },
       { name: "local_authorities_operated_in", value: local_authority_names, editable: false, format: :bullet },
+      { name: "rent_periods", value: rent_period_labels, editable: false, format: :bullet },
       { name: "holds_own_stock", value: holds_own_stock.to_s.humanize, editable: false },
       { name: "other_stock_owners", value: other_stock_owners, editable: false },
       { name: "managing_agents", value: managing_agents, editable: false },

app/models/organisation_rent_period.rb

@@ -0,0 +1,3 @@
+class OrganisationRentPeriod < ApplicationRecord
+  belongs_to :organisation
+end

app/models/rent_period.rb

@@ -0,0 +1,5 @@
+class RentPeriod
+  def self.rent_period_mappings
+    FormHandler.instance.current_form.get_question("period", nil).answer_options
+  end
+end

app/models/validations/financial_validations.rb

@@ -80,6 +80,17 @@ module Validations::FinancialValidations
     validate_rent_range(record)
   end
 
+  def validate_rent_period(record)
+    if record.owning_organisation.present? && record.owning_organisation.rent_periods.present? &&
+        record.period && !record.owning_organisation.rent_periods.include?(record.period)
+      record.errors.add :period, I18n.t(
+        "validations.financial.rent_period.invalid_for_org",
+        org_name: record.owning_organisation.name,
+        rent_period: record.form.get_question("period", record).label_from_value(record.period).downcase,
+      )
+    end
+  end
+
 private
 
   CHARGE_MAXIMUMS = {

app/services/imports/organisation_rent_period_import_service.rb

@@ -0,0 +1,22 @@
+module Imports
+  class OrganisationRentPeriodImportService < ImportService
+    def create_organisation_rent_periods(folder)
+      import_from(folder, :create_organisation_rent_period)
+    end
+
+  private
+
+    def create_organisation_rent_period(xml_document)
+      organisation = Organisation.find_by(old_org_id: record_field_value(xml_document, "institution"))
+
+      OrganisationRentPeriod.create!(
+        organisation:,
+        rent_period: Integer(record_field_value(xml_document, "period")),
+      )
+    end
+
+    def record_field_value(xml_document, field)
+      field_value(xml_document, "rent-period", field)
+    end
+  end
+end

config/locales/en.yml

@@ -136,6 +136,8 @@ en:
         complete_1_of_3: "Answer either the ‘household rent and charges’ question or 'is this accommodation a care home', or select ‘no’ for ‘does the household pay rent or charges for the accommodation?’"
       tcharge:
         under_10: "Total charge must be at least £10 per week"
+      rent_period:
+        invalid_for_org: "%{org_name} does not charge rent %{rent_period}"
 
     household:
       reasonpref:

db/migrate/20220421140410_organisation_rent_period.rb

@@ -0,0 +1,10 @@
+class OrganisationRentPeriod < ActiveRecord::Migration[7.0]
+  def change
+    create_table :organisation_rent_periods do |t|
+      t.belongs_to :organisation
+      t.column :rent_period, :integer
+
+      t.timestamps
+    end
+  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: 2022_04_20_165451) do
+ActiveRecord::Schema[7.0].define(version: 2022_04_21_140410) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -264,6 +264,14 @@ ActiveRecord::Schema[7.0].define(version: 2022_04_20_165451) do
     t.index ["organisation_id"], name: "index_organisation_las_on_organisation_id"
   end
 
+  create_table "organisation_rent_periods", force: :cascade do |t|
+    t.bigint "organisation_id"
+    t.integer "rent_period"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["organisation_id"], name: "index_organisation_rent_periods_on_organisation_id"
+  end
+
   create_table "organisations", force: :cascade do |t|
     t.string "name"
     t.string "phone"

lib/tasks/data_import.rake

@@ -16,6 +16,8 @@ namespace :core do
       Imports::DataProtectionConfirmationImportService.new(storage_service).create_data_protection_confirmations(path)
     when "organisation-las"
       Imports::OrganisationLaImportService.new(storage_service).create_organisation_las(path)
+    when "organisation-rent-periods"
+      Imports::OrganisationRentPeriodImportService.new(storage_service).create_organisation_rent_periods(path)
     else
       raise "Type #{type} is not supported by data_import"
     end

spec/factories/organisation.rb

@@ -15,4 +15,11 @@ FactoryBot.define do
     created_at { Time.zone.now }
     updated_at { Time.zone.now }
   end
+
+  factory :organisation_rent_period do
+    organisation
+    rent_period { 2 }
+    created_at { Time.zone.now }
+    updated_at { Time.zone.now }
+  end
 end

spec/fixtures/softwire_imports/organisation_rent_periods/ebd22326d33e389e9f1bfd546979d2c05f9e68d6.xml

@@ -0,0 +1,5 @@
+<rent-period:rent-period xmlns:rent-period="dclg:rent-period">
+  <rent-period:id>ebd22326d33e389e9f1bfd546979d2c05f9e68d6</rent-period:id>
+  <rent-period:institution>44026acc7ed5c29516b26f2a5deb639e5e37966d</rent-period:institution>
+  <rent-period:period>1</rent-period:period>
+</rent-period:rent-period>

spec/lib/tasks/data_import_spec.rb

@@ -90,6 +90,24 @@ describe "rake core:data_import", type: :task do
     end
   end
 
+  context "when importing organisation rent period data" do
+    let(:type) { "organisation-rent-periods" }
+    let(:import_service) { instance_double(Imports::OrganisationRentPeriodImportService) }
+    let(:fixture_path) { "spec/fixtures/softwire_imports/organisation_rent_periods" }
+
+    before do
+      allow(Imports::OrganisationRentPeriodImportService).to receive(:new).and_return(import_service)
+    end
+
+    it "creates an organisation la from the given XML file" do
+      expect(StorageService).to receive(:new).with(paas_config_service, instance_name)
+      expect(Imports::OrganisationRentPeriodImportService).to receive(:new).with(storage_service)
+      expect(import_service).to receive(:create_organisation_rent_periods).with(fixture_path)
+
+      task.invoke(type, fixture_path)
+    end
+  end
+
   it "raises an exception if no parameters are provided" do
     expect { task.invoke }.to raise_error(/Usage/)
   end

spec/models/case_log_spec.rb

@@ -125,6 +125,10 @@ RSpec.describe CaseLog do
       expect(validator).to receive(:validate_outstanding_rent_amount)
     end
 
+    it "validates the rent period" do
+      expect(validator).to receive(:validate_rent_period)
+    end
+
     it "validates housing benefit rent shortfall" do
       expect(validator).to receive(:validate_tshortfall)
     end

spec/models/organisation_spec.rb

@@ -44,7 +44,7 @@ RSpec.describe Organisation, type: :model do
       end
 
       before do
-        FactoryBot.create(:organisation_la, organisation_id: organisation.id, ons_code: "E07000178")
+        FactoryBot.create(:organisation_la, organisation:, ons_code: "E07000178")
         allow(LocalAuthority).to receive(:ons_code_mappings).and_return(ons_code_mappings)
       end
 
@@ -57,6 +57,38 @@ RSpec.describe Organisation, type: :model do
       end
     end
 
+    context "when the organisation has not specified which local authorities it operates in" do
+      it "displays `all`" do
+        expect(organisation.local_authority_names).to eq(%w[All])
+      end
+    end
+
+    context "when the organisation only uses specific rent periods" do
+      let(:rent_period_mappings) do
+        { "2" => { "value" => "Weekly for 52 weeks" }, "3" => { "value" => "Every 2 weeks" } }
+      end
+
+      before do
+        FactoryBot.create(:organisation_rent_period, organisation:, rent_period: 2)
+        FactoryBot.create(:organisation_rent_period, organisation:, rent_period: 3)
+        allow(RentPeriod).to receive(:rent_period_mappings).and_return(rent_period_mappings)
+      end
+
+      it "has rent periods associated" do
+        expect(organisation.rent_periods).to eq([2, 3])
+      end
+
+      it "maps the rent periods to display values" do
+        expect(organisation.rent_period_labels).to eq(["Weekly for 52 weeks", "Every 2 weeks"])
+      end
+    end
+
+    context "when the organisation has not specified which rent periods it uses" do
+      it "displays `all`" do
+        expect(organisation.rent_period_labels).to eq(%w[All])
+      end
+    end
+
     context "with case logs" do
       let(:other_organisation) { FactoryBot.create(:organisation) }
       let!(:owned_case_log) do

spec/models/rent_period_spec.rb

@@ -0,0 +1,16 @@
+require "rails_helper"
+
+RSpec.describe RentPeriod, type: :model do
+  describe "rent period mapping" do
+    let(:form) { Form.new("spec/fixtures/forms/2021_2022.json", "2021_2022") }
+
+    before do
+      allow(FormHandler.instance).to receive(:current_form).and_return(form)
+    end
+
+    it "maps rent period id to display names" do
+      expect(described_class.rent_period_mappings).to be_a(Hash)
+      expect(described_class.rent_period_mappings["2"]).to eq({ "value" => "Weekly for 52 weeks" })
+    end
+  end
+end

spec/models/validations/financial_validations_spec.rb

@@ -86,6 +86,28 @@ RSpec.describe Validations::FinancialValidations do
     end
   end
 
+  describe "rent period validations" do
+    let(:organisation) { FactoryBot.create(:organisation) }
+    let(:record) { FactoryBot.create(:case_log, owning_organisation: organisation) }
+
+    before do
+      FactoryBot.create(:organisation_rent_period, organisation:, rent_period: 2)
+    end
+
+    context "when the organisation only uses specific rent periods" do
+      it "validates that the selected rent period is used by the organisation" do
+        record.period = 3
+        financial_validator.validate_rent_period(record)
+        expect(record.errors["period"])
+          .to include(match I18n.t(
+            "validations.financial.rent_period.invalid_for_org",
+            org_name: organisation.name,
+            rent_period: "every 2 weeks",
+          ))
+      end
+    end
+  end
+
   describe "housing benefit rent shortfall validations" do
     context "when shortfall is yes" do
       it "validates that housing benefit is not none" do
@@ -121,7 +143,7 @@ RSpec.describe Validations::FinancialValidations do
     end
   end
 
-  describe "Net income validations" do
+  describe "net income validations" do
     it "validates that the net income is within the expected range for the tenant's employment status" do
       record.earnings = 200
       record.incfreq = 1

spec/services/imports/organisation_rent_period_import_service_spec.rb

@@ -0,0 +1,41 @@
+require "rails_helper"
+
+RSpec.describe Imports::OrganisationRentPeriodImportService do
+  let(:fixture_directory) { "spec/fixtures/softwire_imports/organisation_rent_periods" }
+  let(:old_org_id) { "44026acc7ed5c29516b26f2a5deb639e5e37966d" }
+  let(:old_id) { "ebd22326d33e389e9f1bfd546979d2c05f9e68d6" }
+  let(:import_file) { File.open("#{fixture_directory}/#{old_id}.xml") }
+  let(:storage_service) { instance_double(StorageService) }
+
+  context "when importing data protection confirmations" do
+    subject(:import_service) { described_class.new(storage_service) }
+
+    before do
+      allow(storage_service)
+        .to receive(:list_files)
+        .and_return(["organisation_rent_period_directory/#{old_id}.xml"])
+      allow(storage_service)
+        .to receive(:get_file_io)
+        .with("organisation_rent_period_directory/#{old_id}.xml")
+        .and_return(import_file)
+    end
+
+    context "when the organisation in the import file doesn't exist in the system" do
+      it "does not create an organisation rent period record" do
+        expect { import_service.create_organisation_rent_periods("organisation_rent_period_directory") }
+          .to raise_error(ActiveRecord::RecordInvalid, /Organisation must exist/)
+      end
+    end
+
+    context "when the organisation does exist" do
+      before do
+        FactoryBot.create(:organisation, old_org_id:)
+      end
+
+      it "successfully create an organisation rent period record with the expected data" do
+        import_service.create_organisation_rent_periods("organisation_rent_period_directory")
+        expect(Organisation.find_by(old_org_id:).organisation_rent_periods.pluck("rent_period")).to eq([1])
+      end
+    end
+  end
+end