Browse Source

Remove organisation LAs since validation is no longer required (#637)

pull/644/head
baarkerlounger 3 years ago committed by GitHub
parent
commit
d406acd136
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 5
      app/models/local_authority.rb
  2. 11
      app/models/organisation.rb
  3. 3
      app/models/organisation_la.rb
  4. 11
      app/models/validations/local_authority_validations.rb
  5. 23
      app/services/imports/organisation_la_import_service.rb
  6. 14
      db/migrate/20220606082639_drop_organisations_las.rb
  7. 10
      db/schema.rb
  8. 6
      spec/fixtures/softwire_imports/organisation_las/00013f30e159d7f72a3abe9ea93fb5b685d311e4.xml
  9. 18
      spec/lib/tasks/data_import_spec.rb
  10. 4
      spec/models/case_log_spec.rb
  11. 16
      spec/models/local_authority_spec.rb
  12. 34
      spec/models/organisation_spec.rb
  13. 51
      spec/models/validations/local_authority_validations_spec.rb
  14. 42
      spec/services/imports/organisation_la_import_service_spec.rb

5
app/models/local_authority.rb

@ -1,5 +0,0 @@
class LocalAuthority
def self.ons_code_mappings
FormHandler.instance.current_form.get_question("la", nil).answer_options
end
end

11
app/models/organisation.rb

@ -3,7 +3,6 @@ class Organisation < ApplicationRecord
has_many :owned_case_logs, class_name: "CaseLog", foreign_key: "owning_organisation_id" has_many :owned_case_logs, class_name: "CaseLog", foreign_key: "owning_organisation_id"
has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id" has_many :managed_case_logs, class_name: "CaseLog", foreign_key: "managing_organisation_id"
has_many :data_protection_confirmations has_many :data_protection_confirmations
has_many :organisation_las
has_many :organisation_rent_periods has_many :organisation_rent_periods
scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") }
@ -36,15 +35,6 @@ class Organisation < ApplicationRecord
%i[address_line1 address_line2 postcode].map { |field| public_send(field) }.join("\n") %i[address_line1 address_line2 postcode].map { |field| public_send(field) }.join("\n")
end end
def local_authorities
organisation_las.pluck(:ons_code)
end
def local_authority_names
names = local_authorities.map { |ons_code| LocalAuthority.ons_code_mappings[ons_code] }
names.presence || %w[All]
end
def rent_periods def rent_periods
organisation_rent_periods.pluck(:rent_period) organisation_rent_periods.pluck(:rent_period)
end end
@ -74,7 +64,6 @@ class Organisation < ApplicationRecord
{ name: "address", value: address_string, editable: true }, { name: "address", value: address_string, editable: true },
{ name: "telephone_number", value: phone, editable: true }, { name: "telephone_number", value: phone, editable: true },
{ name: "type", value: display_provider_type, editable: false }, { name: "type", value: display_provider_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: "rent_periods", value: rent_period_labels, editable: false, format: :bullet },
{ name: "holds_own_stock", value: holds_own_stock.to_s.humanize, editable: false }, { name: "holds_own_stock", value: holds_own_stock.to_s.humanize, editable: false },
{ name: "other_stock_owners", value: other_stock_owners, editable: false }, { name: "other_stock_owners", value: other_stock_owners, editable: false },

3
app/models/organisation_la.rb

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

11
app/models/validations/local_authority_validations.rb

@ -8,15 +8,4 @@ module Validations::LocalAuthorityValidations
record.errors.add :ppostcode_full, error_message record.errors.add :ppostcode_full, error_message
end end
end end
def validate_la(record)
if record.owning_organisation && record.owning_organisation.local_authorities.present? &&
record.la && !record.owning_organisation.local_authorities.include?(record.la)
la_name = record.form.get_question("la", record).label_from_value(record.la)
org_name = record.owning_organisation.name
postcode = UKPostcode.parse(record.postcode_full) if record.postcode_full
record.errors.add :la, I18n.t("validations.property.la.la_invalid_for_org", org_name:, la_name:)
record.errors.add :postcode_known, I18n.t("validations.property.la.postcode_invalid_for_org", org_name:, postcode:)
end
end
end end

23
app/services/imports/organisation_la_import_service.rb

@ -1,23 +0,0 @@
module Imports
class OrganisationLaImportService < ImportService
def create_organisation_las(folder)
import_from(folder, :create_organisation_la)
end
private
def create_organisation_la(xml_document)
xml_document.remove_namespaces!
organisation = Organisation.find_by(old_org_id: record_field_value(xml_document, "InstitutionId"))
OrganisationLa.create!(
organisation:,
ons_code: record_field_value(xml_document, "ONSCode"),
)
end
def record_field_value(xml_document, field)
xml_document.at_xpath("//#{field}")&.text
end
end
end

14
db/migrate/20220606082639_drop_organisations_las.rb

@ -0,0 +1,14 @@
class DropOrganisationsLas < ActiveRecord::Migration[7.0]
def up
drop_table :organisation_las
end
def down
create_table :organisation_las do |t|
t.belongs_to :organisation
t.column :ons_code, :string
t.timestamps
end
end
end

10
db/schema.rb

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2022_05_25_130518) do ActiveRecord::Schema[7.0].define(version: 2022_06_06_082639) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -238,14 +238,6 @@ ActiveRecord::Schema[7.0].define(version: 2022_05_25_130518) do
t.boolean "empty_export", default: false, null: false t.boolean "empty_export", default: false, null: false
end end
create_table "organisation_las", force: :cascade do |t|
t.bigint "organisation_id"
t.string "ons_code"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["organisation_id"], name: "index_organisation_las_on_organisation_id"
end
create_table "organisation_rent_periods", force: :cascade do |t| create_table "organisation_rent_periods", force: :cascade do |t|
t.bigint "organisation_id" t.bigint "organisation_id"
t.integer "rent_period" t.integer "rent_period"

6
spec/fixtures/softwire_imports/organisation_las/00013f30e159d7f72a3abe9ea93fb5b685d311e4.xml vendored

@ -1,6 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<institution xmlns="http://www.ons.gov.uk/institutions-and-ons-la">
<InstitutionId>44026acc7ed5c29516b26f2a5deb639e5e37966d</InstitutionId>
<AssociationId>7076</AssociationId>
<ONSCode>E07000041</ONSCode>
</institution>

18
spec/lib/tasks/data_import_spec.rb

@ -73,24 +73,6 @@ describe "rake core:data_import", type: :task do
end end
end end
context "when importing organisation local authority data" do
let(:type) { "organisation-las" }
let(:import_service) { instance_double(Imports::OrganisationLaImportService) }
let(:fixture_path) { "spec/fixtures/softwire_imports/organisation_las" }
before do
allow(Imports::OrganisationLaImportService).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::OrganisationLaImportService).to receive(:new).with(storage_service)
expect(import_service).to receive(:create_organisation_las).with(fixture_path)
task.invoke(type, fixture_path)
end
end
context "when importing organisation rent period data" do context "when importing organisation rent period data" do
let(:type) { "organisation-rent-periods" } let(:type) { "organisation-rent-periods" }
let(:import_service) { instance_double(Imports::OrganisationRentPeriodImportService) } let(:import_service) { instance_double(Imports::OrganisationRentPeriodImportService) }

4
spec/models/case_log_spec.rb

@ -115,10 +115,6 @@ RSpec.describe CaseLog do
expect(validator).to receive(:validate_property_void_date) expect(validator).to receive(:validate_property_void_date)
end end
it "validates local authority" do
expect(validator).to receive(:validate_la)
end
it "validates benefits as proportion of income" do it "validates benefits as proportion of income" do
expect(validator).to receive(:validate_net_income_uc_proportion) expect(validator).to receive(:validate_net_income_uc_proportion)
end end

16
spec/models/local_authority_spec.rb

@ -1,16 +0,0 @@
require "rails_helper"
RSpec.describe LocalAuthority, type: :model do
describe "ons code 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 ONS code to local authority names" do
expect(described_class.ons_code_mappings).to be_a(Hash)
expect(described_class.ons_code_mappings["E07000178"]).to eq("Oxford")
end
end
end

34
spec/models/organisation_spec.rb

@ -29,40 +29,6 @@ RSpec.describe Organisation, type: :model do
end end
end end
context "when the organisation only operates in specific local authorities" do
let(:ons_code_mappings) do
{
"" => "Select an option",
"E07000223" => "Adur",
"E09000023" => "Lewisham",
"E08000003" => "Manchester",
"E07000178" => "Oxford",
"E07000114" => "Thanet",
"E09000033" => "Westminster",
"E06000014" => "York",
}
end
before do
FactoryBot.create(:organisation_la, organisation:, ons_code: "E07000178")
allow(LocalAuthority).to receive(:ons_code_mappings).and_return(ons_code_mappings)
end
it "has local authorities associated" do
expect(organisation.local_authorities).to eq(%w[E07000178])
end
it "maps the ons codes to LA names for display" do
expect(organisation.local_authority_names).to eq(%w[Oxford])
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 context "when the organisation only uses specific rent periods" do
let(:rent_period_mappings) do let(:rent_period_mappings) do
{ "2" => { "value" => "Weekly for 52 weeks" }, "3" => { "value" => "Every 2 weeks" } } { "2" => { "value" => "Weekly for 52 weeks" }, "3" => { "value" => "Every 2 weeks" } }

51
spec/models/validations/local_authority_validations_spec.rb

@ -35,55 +35,4 @@ RSpec.describe Validations::LocalAuthorityValidations do
expect(record.errors["ppostcode_full"]).to include(match I18n.t("validations.postcode")) expect(record.errors["ppostcode_full"]).to include(match I18n.t("validations.postcode"))
end end
end end
describe "#validate_la" do
context "when the organisation only operates in specific local authorities" do
let(:organisation) { FactoryBot.create(:organisation) }
let(:record) { FactoryBot.create(:case_log, owning_organisation: organisation) }
before do
FactoryBot.create(:organisation_la, organisation:, ons_code: "E07000178")
FactoryBot.create(:organisation_la, organisation:, ons_code: "E09000033")
end
it "validates that the local authority is one the owning organisation operates in" do
record.la = "E06000014"
local_auth_validator.validate_la(record)
expect(record.errors["la"])
.to include(match I18n.t(
"validations.property.la.la_invalid_for_org",
org_name: organisation.name,
la_name: "York",
))
end
it "expects that the local authority can be one that the owning organisation operates in" do
record.la = "E07000178"
local_auth_validator.validate_la(record)
expect(record.errors["la"]).to be_empty
end
context "when a postcode is given" do
it "validates that it matches a local authority the organisation operates in" do
record.postcode_full = "YO10 3RR"
record.la = "E06000014"
local_auth_validator.validate_la(record)
expect(record.errors["postcode_known"])
.to include(match I18n.t(
"validations.property.la.postcode_invalid_for_org",
org_name: organisation.name,
postcode: "YO10 3RR",
))
end
end
end
context "when the organisation has not listed specific local authorities it operates in" do
it "does not validate the local authority for the organisation" do
record.la = "E06000014"
local_auth_validator.validate_la(record)
expect(record.errors["la"]).to be_empty
end
end
end
end end

42
spec/services/imports/organisation_la_import_service_spec.rb

@ -1,42 +0,0 @@
require "rails_helper"
RSpec.describe Imports::OrganisationLaImportService do
let(:fixture_directory) { "spec/fixtures/softwire_imports/organisation_las" }
let(:old_org_id) { "44026acc7ed5c29516b26f2a5deb639e5e37966d" }
let(:old_id) { "00013f30e159d7f72a3abe9ea93fb5b685d311e4" }
let(:import_file) { File.open("#{fixture_directory}/#{old_id}.xml") }
let(:storage_service) { instance_double(StorageService) }
let(:logger) { instance_double(ActiveSupport::Logger) }
context "when importing organisation las" do
subject(:import_service) { described_class.new(storage_service, logger) }
before do
allow(storage_service)
.to receive(:list_files)
.and_return(["organisation_la_directory/#{old_id}.xml"])
allow(storage_service)
.to receive(:get_file_io)
.with("organisation_la_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 la record" do
expect(logger).to receive(:error).with(/Organisation must exist/)
import_service.create_organisation_las("organisation_la_directory")
end
end
context "when the organisation does exist" do
before do
FactoryBot.create(:organisation, old_org_id:)
end
it "successfully create an organisation la record with the expected data" do
import_service.create_organisation_las("organisation_la_directory")
expect(Organisation.find_by(old_org_id:).organisation_las.pluck("ons_code")).to eq(%w[E07000041])
end
end
end
end
Loading…
Cancel
Save