From e212612ee6f5e4a70b51cf7791efcaf8d289d6d3 Mon Sep 17 00:00:00 2001
From: baarkerlounger <db@slothlife.xyz>
Date: Thu, 21 Apr 2022 17:02:46 +0100
Subject: [PATCH 1/5] Add table

---
 app/models/organisation.rb                          |  1 +
 app/models/organisation_rent_period.rb              |  3 +++
 .../20220421140410_organisation_rent_period.rb      | 10 ++++++++++
 db/schema.rb                                        | 10 +++++++++-
 spec/factories/organisation.rb                      |  7 +++++++
 .../ebd22326d33e389e9f1bfd546979d2c05f9e68d6.xml    |  5 +++++
 spec/models/organisation_spec.rb                    | 13 ++++++++++++-
 7 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 app/models/organisation_rent_period.rb
 create mode 100644 db/migrate/20220421140410_organisation_rent_period.rb
 create mode 100644 spec/fixtures/softwire_imports/organisation_rent_periods/ebd22326d33e389e9f1bfd546979d2c05f9e68d6.xml

diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index 85cf76028..e5c26eec4 100644
--- a/app/models/organisation.rb
+++ b/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
 
diff --git a/app/models/organisation_rent_period.rb b/app/models/organisation_rent_period.rb
new file mode 100644
index 000000000..292ac75b2
--- /dev/null
+++ b/app/models/organisation_rent_period.rb
@@ -0,0 +1,3 @@
+class OrganisationRentPeriod < ApplicationRecord
+  belongs_to :organisation
+end
diff --git a/db/migrate/20220421140410_organisation_rent_period.rb b/db/migrate/20220421140410_organisation_rent_period.rb
new file mode 100644
index 000000000..54e090647
--- /dev/null
+++ b/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
diff --git a/db/schema.rb b/db/schema.rb
index beff46c57..dcd9e007a 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_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"
diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb
index 2bb293968..7da708cae 100644
--- a/spec/factories/organisation.rb
+++ b/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 { 1 }
+    created_at { Time.zone.now }
+    updated_at { Time.zone.now }
+  end
 end
diff --git a/spec/fixtures/softwire_imports/organisation_rent_periods/ebd22326d33e389e9f1bfd546979d2c05f9e68d6.xml b/spec/fixtures/softwire_imports/organisation_rent_periods/ebd22326d33e389e9f1bfd546979d2c05f9e68d6.xml
new file mode 100644
index 000000000..7f6e4fb99
--- /dev/null
+++ b/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>
diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb
index 4766c35fa..a60d5574a 100644
--- a/spec/models/organisation_spec.rb
+++ b/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,17 @@ RSpec.describe Organisation, type: :model do
       end
     end
 
+    context "when the organisation only uses specific rent periods" do
+      before do
+        FactoryBot.create(:organisation_rent_period, organisation:, rent_period: 1)
+        FactoryBot.create(:organisation_rent_period, organisation:, rent_period: 2)
+      end
+
+      it "has rent periods associated" do
+        expect(organisation.organisation_rent_periods.pluck("rent_period")).to eq([1, 2])
+      end
+    end
+
     context "with case logs" do
       let(:other_organisation) { FactoryBot.create(:organisation) }
       let!(:owned_case_log) do

From 1e335f494820c64219a1252c12440e701a3c3aad Mon Sep 17 00:00:00 2001
From: baarkerlounger <db@slothlife.xyz>
Date: Fri, 22 Apr 2022 10:45:03 +0100
Subject: [PATCH 2/5] Display Org rent periods

---
 app/models/organisation.rb       | 11 ++++++++++-
 app/models/rent_period.rb        |  5 +++++
 spec/models/organisation_spec.rb | 13 +++++++++++--
 spec/models/rent_period_spec.rb  | 16 ++++++++++++++++
 4 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100644 app/models/rent_period.rb
 create mode 100644 spec/models/rent_period_spec.rb

diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index e5c26eec4..b55f371bd 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -38,13 +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] }
   end
 
+  def rent_periods
+    organisation_rent_periods.pluck(:rent_period)
+  end
+
+  def rent_period_labels
+    rent_periods.map { |period| RentPeriod.rent_period_mappings[period.to_s]["value"] }
+  end
+
   def display_attributes
     [
       { name: "name", value: name, editable: true },
@@ -52,6 +60,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 },
diff --git a/app/models/rent_period.rb b/app/models/rent_period.rb
new file mode 100644
index 000000000..77e154f4c
--- /dev/null
+++ b/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
diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb
index a60d5574a..1a64145b1 100644
--- a/spec/models/organisation_spec.rb
+++ b/spec/models/organisation_spec.rb
@@ -58,13 +58,22 @@ RSpec.describe Organisation, type: :model do
     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: 1)
         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.organisation_rent_periods.pluck("rent_period")).to eq([1, 2])
+        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
 
diff --git a/spec/models/rent_period_spec.rb b/spec/models/rent_period_spec.rb
new file mode 100644
index 000000000..55252977a
--- /dev/null
+++ b/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

From 58f4431dd7f8d035750a26da8eaa12ecde86f7ac Mon Sep 17 00:00:00 2001
From: baarkerlounger <db@slothlife.xyz>
Date: Fri, 22 Apr 2022 10:48:29 +0100
Subject: [PATCH 3/5] Default to dislpaying all

---
 app/models/organisation.rb       |  6 ++++--
 spec/models/organisation_spec.rb | 12 ++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index b55f371bd..e3e7f05c2 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -42,7 +42,8 @@ class Organisation < ApplicationRecord
   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.present? ? names : ["All"]
   end
 
   def rent_periods
@@ -50,7 +51,8 @@ class Organisation < ApplicationRecord
   end
 
   def rent_period_labels
-    rent_periods.map { |period| RentPeriod.rent_period_mappings[period.to_s]["value"] }
+    labels = rent_periods.map { |period| RentPeriod.rent_period_mappings[period.to_s]["value"] }
+    labels.present? ? labels : ["All"]
   end
 
   def display_attributes
diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb
index 1a64145b1..f94782b1d 100644
--- a/spec/models/organisation_spec.rb
+++ b/spec/models/organisation_spec.rb
@@ -57,6 +57,12 @@ 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" } }
@@ -77,6 +83,12 @@ RSpec.describe Organisation, type: :model do
       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

From e6e2c00abd166f5b30c7b9067fa0d195cc9eb616 Mon Sep 17 00:00:00 2001
From: baarkerlounger <db@slothlife.xyz>
Date: Fri, 22 Apr 2022 11:17:08 +0100
Subject: [PATCH 4/5] Add validation

---
 app/models/organisation.rb                    |  4 ++--
 .../validations/financial_validations.rb      | 11 +++++++++
 config/locales/en.yml                         |  2 ++
 spec/factories/organisation.rb                |  2 +-
 spec/models/case_log_spec.rb                  |  4 ++++
 .../validations/financial_validations_spec.rb | 24 ++++++++++++++++++-
 6 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/app/models/organisation.rb b/app/models/organisation.rb
index e3e7f05c2..95f2b4a48 100644
--- a/app/models/organisation.rb
+++ b/app/models/organisation.rb
@@ -43,7 +43,7 @@ class Organisation < ApplicationRecord
 
   def local_authority_names
     names = local_authorities.map { |ons_code| LocalAuthority.ons_code_mappings[ons_code] }
-    names.present? ? names : ["All"]
+    names.presence || %w[All]
   end
 
   def rent_periods
@@ -52,7 +52,7 @@ class Organisation < ApplicationRecord
 
   def rent_period_labels
     labels = rent_periods.map { |period| RentPeriod.rent_period_mappings[period.to_s]["value"] }
-    labels.present? ? labels : ["All"]
+    labels.presence || %w[All]
   end
 
   def display_attributes
diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb
index a95cf25c1..857ceb45a 100644
--- a/app/models/validations/financial_validations.rb
+++ b/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 = {
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 2d66ac916..ab1c95918 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -138,6 +138,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:
diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb
index 7da708cae..981217736 100644
--- a/spec/factories/organisation.rb
+++ b/spec/factories/organisation.rb
@@ -18,7 +18,7 @@ FactoryBot.define do
 
   factory :organisation_rent_period do
     organisation
-    rent_period { 1 }
+    rent_period { 2 }
     created_at { Time.zone.now }
     updated_at { Time.zone.now }
   end
diff --git a/spec/models/case_log_spec.rb b/spec/models/case_log_spec.rb
index 3633fba51..852b2eaec 100644
--- a/spec/models/case_log_spec.rb
+++ b/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
diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb
index 68b5f4629..7f4eb438c 100644
--- a/spec/models/validations/financial_validations_spec.rb
+++ b/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

From 48ac937df79f0e12d872cfe9574c8e96d31915e5 Mon Sep 17 00:00:00 2001
From: baarkerlounger <db@slothlife.xyz>
Date: Fri, 22 Apr 2022 12:09:25 +0100
Subject: [PATCH 5/5] Add import

---
 ...organisation_rent_period_import_service.rb | 22 ++++++++++
 lib/tasks/data_import.rake                    |  2 +
 spec/lib/tasks/data_import_spec.rb            | 18 ++++++++
 ...isation_rent_period_import_service_spec.rb | 41 +++++++++++++++++++
 4 files changed, 83 insertions(+)
 create mode 100644 app/services/imports/organisation_rent_period_import_service.rb
 create mode 100644 spec/services/imports/organisation_rent_period_import_service_spec.rb

diff --git a/app/services/imports/organisation_rent_period_import_service.rb b/app/services/imports/organisation_rent_period_import_service.rb
new file mode 100644
index 000000000..74b20b830
--- /dev/null
+++ b/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
diff --git a/lib/tasks/data_import.rake b/lib/tasks/data_import.rake
index eb4554083..ad15b766e 100644
--- a/lib/tasks/data_import.rake
+++ b/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
diff --git a/spec/lib/tasks/data_import_spec.rb b/spec/lib/tasks/data_import_spec.rb
index eae18ff33..4e225bfe3 100644
--- a/spec/lib/tasks/data_import_spec.rb
+++ b/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
diff --git a/spec/services/imports/organisation_rent_period_import_service_spec.rb b/spec/services/imports/organisation_rent_period_import_service_spec.rb
new file mode 100644
index 000000000..a0ee2e6fe
--- /dev/null
+++ b/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