From 2996a34a9512a8d7cee19e825f1fda5e743aff9c Mon Sep 17 00:00:00 2001 From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com> Date: Thu, 8 Dec 2022 11:03:50 +0000 Subject: [PATCH] 1628: a scheme's owning organisation must hold stock (#1027) * feat: start adding scheme validation for when owning org doesn't hold stock * feat: don't set owning_organisation = nil in validate_owning_organisation * style: line code * test: remove redundant test for missing owning org * test: add test for invalid owning org id (invalid as doesn't hold stock) * test: add test that including owning org does nothing when user is coordinator * test: standardise missing params test descriptions in schemes_controller_spec * test: remove owning org id from required params list for data coordinators * test: write "support user", not just "support" * test: fix incorrect test descriptions * test: test validation raised when updating model with non-stock-owning org * config: set dummy_org in seeds as holding own stock * test: test for actual validation string, not the validation lookup * refactor: change owning_organisation_id: organisation.id to owning_organisation: organisation * test: move scheme params def onto 1 line * style: move "validate :validate_owning_organisation" onto new line --- app/models/scheme.rb | 7 ++++++ config/locales/en.yml | 2 ++ db/seeds.rb | 2 +- spec/models/scheme_spec.rb | 12 +++++++++ spec/requests/schemes_controller_spec.rb | 32 ++++++++++++++++-------- 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/app/models/scheme.rb b/app/models/scheme.rb index 67b577ed2..a4dd21418 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -22,6 +22,7 @@ class Scheme < ApplicationRecord scope :order_by_service_name, -> { order(service_name: :asc) } validate :validate_confirmed + validate :validate_owning_organisation auto_strip_attributes :service_name @@ -212,6 +213,12 @@ class Scheme < ApplicationRecord end end + def validate_owning_organisation + unless owning_organisation.holds_own_stock? + errors.add(:owning_organisation_id, :does_not_own_stock, message: I18n.t("validations.scheme.owning_organisation.does_not_own_stock")) + end + end + def available_from FormHandler.instance.collection_start_date(created_at) end diff --git a/config/locales/en.yml b/config/locales/en.yml index 2463041f9..6c2d7db46 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -341,6 +341,8 @@ en: before_deactivation: "This scheme was deactivated on %{date}. The reactivation date must be on or after deactivation date" deactivation: during_deactivated_period: "The scheme is already deactivated during this date, please enter a different date" + owning_organisation: + does_not_own_stock: "Enter an organisation that owns housing stock" diff --git a/db/seeds.rb b/db/seeds.rb index 5ac6bbad4..20975feb8 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -186,7 +186,7 @@ unless Rails.env.test? address_line1: "Higher Kingston", address_line2: "Yeovil", postcode: "BA21 4AT", - holds_own_stock: false, + holds_own_stock: true, other_stock_owners: "None", managing_agents_label: "None", provider_type: "LA", diff --git a/spec/models/scheme_spec.rb b/spec/models/scheme_spec.rb index 2f65ba442..f59fe56c5 100644 --- a/spec/models/scheme_spec.rb +++ b/spec/models/scheme_spec.rb @@ -209,4 +209,16 @@ RSpec.describe Scheme, type: :model do end end end + + describe "owning organisation" do + let(:stock_owning_org) { FactoryBot.create(:organisation, holds_own_stock: true) } + let(:non_stock_owning_org) { FactoryBot.create(:organisation, holds_own_stock: false) } + let(:scheme) { FactoryBot.create(:scheme, owning_organisation_id: stock_owning_org.id) } + + context "when the owning organisation is set as a non-stock-owning organisation" do + it "throws the correct validation error" do + expect { scheme.update!({ owning_organisation: non_stock_owning_org }) }.to raise_error(ActiveRecord::RecordInvalid, /Enter an organisation that owns housing stock/) + end + end + end end diff --git a/spec/requests/schemes_controller_spec.rb b/spec/requests/schemes_controller_spec.rb index 9ea9de205..5530600ff 100644 --- a/spec/requests/schemes_controller_spec.rb +++ b/spec/requests/schemes_controller_spec.rb @@ -471,7 +471,7 @@ RSpec.describe SchemesController, type: :request do end end - context "when missing required scheme params" do + context "when required params are missing" do let(:params) do { scheme: { service_name: "", scheme_type: "", @@ -489,6 +489,16 @@ RSpec.describe SchemesController, type: :request do expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.service_name.invalid")) end end + + context "when the organisation id param is included" do + let(:organisation) { FactoryBot.create(:organisation) } + let(:params) { { scheme: { owning_organisation: organisation } } } + + it "sets the owning organisation correctly" do + post "/schemes", params: params + expect(Scheme.last.owning_organisation_id).to eq(user.organisation_id) + end + end end context "when signed in as a support user" do @@ -566,7 +576,7 @@ RSpec.describe SchemesController, type: :request do end end - context "when missing required scheme params" do + context "when required params are missing" do let(:params) do { scheme: { service_name: "", scheme_type: "", @@ -586,13 +596,14 @@ RSpec.describe SchemesController, type: :request do end end - context "when required organisation id param is missing" do - let(:params) { { "scheme" => { "service_name" => "qweqwer", "sensitive" => "Yes", "owning_organisation_id" => "", "scheme_type" => "Foyer", "registered_under_care_act" => "Yes – part registered as a care home" } } } + context "when organisation id param refers to a non-stock-owning organisation" do + let(:organisation_which_does_not_own_stock) { FactoryBot.create(:organisation, holds_own_stock: false) } + let(:params) { { scheme: { owning_organisation_id: organisation_which_does_not_own_stock.id } } } it "displays the new page with an error message" do post "/schemes", params: params expect(response).to have_http_status(:unprocessable_entity) - expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.owning_organisation_id.invalid")) + expect(page).to have_content("Enter an organisation that owns housing stock") end end end @@ -638,12 +649,11 @@ RSpec.describe SchemesController, type: :request do end end - context "when params are missing" do + context "when required params are missing" do let(:params) do { scheme: { service_name: "", managing_organisation_id: "", - owning_organisation_id: "", primary_client_group: "", secondary_client_group: "", scheme_type: "", @@ -656,7 +666,7 @@ RSpec.describe SchemesController, type: :request do } } end - it "renders primary client group after successful update" do + it "renders the same page with error message" do expect(response).to have_http_status(:unprocessable_entity) expect(page).to have_content("Create a new supported housing scheme") expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.service_name.invalid")) @@ -923,7 +933,7 @@ RSpec.describe SchemesController, type: :request do end end - context "when signed in as a support" do + context "when signed in as a support user" do let(:user) { FactoryBot.create(:user, :support) } let(:scheme_to_update) { FactoryBot.create(:scheme, owning_organisation: user.organisation, confirmed: nil) } @@ -934,7 +944,7 @@ RSpec.describe SchemesController, type: :request do patch "/schemes/#{scheme_to_update.id}", params: end - context "when params are missing" do + context "when required params are missing" do let(:params) do { scheme: { service_name: "", @@ -952,7 +962,7 @@ RSpec.describe SchemesController, type: :request do } } end - it "renders primary client group after successful update" do + it "renders the same page with error message" do expect(response).to have_http_status(:unprocessable_entity) expect(page).to have_content("Create a new supported housing scheme") expect(page).to have_content(I18n.t("activerecord.errors.models.scheme.attributes.owning_organisation_id.invalid"))