From 92a00e43054245d4b415bc3c99c39a3cf918abe1 Mon Sep 17 00:00:00 2001 From: SamSeed-Softwire <63662292+SamSeed-Softwire@users.noreply.github.com> Date: Tue, 24 Jan 2023 10:11:46 +0000 Subject: [PATCH] CLDC-1723 Overhaul letting log owning & managing org questions & tests (#1140) * test: check managing org not gone from answer opts when relationship deleted * feat: add current managing org to answer opts * feat: check if managing org exists before trying to show it * wip * test: improve managing orgs opts test when not support * test: improve managing orgs opts tests when support * test: make relationship deletion test consistent with other tests * test: add "(with hint)" to managing org opts test descriptions * test: refactor managing orgs opts tests for support user case * fix: don't call user in get_answer_label in CYA component * style: reorder instance vars and remove old comments in managing_organisation.rb * refactor: ensure label_from_value always accepts log & nil as args * lint * test: pass in log and user in housing provider opts test for support user * test: update housing provider opts tests for non-support user * feat: update housing provider answer opts to include current HP in db * style: add space after user definition * test: make context definition more human-readable * test: refactor housing providers opts tests (not support user) * test: check housing prov. still selectable after deleting relationship * fix: define log and current_user instance vars in label_from_value (housing prov.) * lint * test: update lettings log feature tests to differentiate between different numbers of stock owners when acting as a data coordinator * test: check owning & managing orgs set correctly when a log is created * test: add line breaks and start context descriptions with and (not if) * test: artificially reference org_rel2 to avoid lint offense * feat: don't set log owning org as user's org if that org doesn't hold stock * test: improve test context descriptions in lettings_log_spec * test: finish overhauling owning and managing org tests in lettings_log_spec * test: change let! to let where possible in spec/features/lettings_log_spec.rb * test: change let! to let where possible in spec/models/form/lettings/questions/managing_organisation_spec.rb * test: change let! to let where possible in spec/models/form/lettings/questions/stock_owner_spec.rb * test: remove if statement from "coordinator user's org doesn't hold stock" managing org test * test: remove if statement from "coordinator user's org does hold stock" no managing orgs managing org test * test: remove if statement from "coordinator user's org does hold stock" >=1 managing orgs managing org test plus refactor previous test * test: explicitly reference org rels in "coordinator user's org doesn't hold stock" managing org test * test: don't create vars inside other vars (for tests edited/created in this branch) * chore: save schema changes after migration Co-authored-by: Phil Lee --- ...eck_answers_summary_list_card_component.rb | 2 +- app/controllers/logs_controller.rb | 3 +- app/helpers/check_answers_helper.rb | 2 +- .../form/common/questions/created_by_id.rb | 2 +- .../questions/owning_organisation_id.rb | 2 +- .../form/lettings/questions/created_by_id.rb | 2 +- .../questions/managing_organisation.rb | 10 +- .../form/lettings/questions/stock_owner.rb | 11 +- app/models/form/question.rb | 6 +- db/schema.rb | 10 +- spec/features/lettings_log_spec.rb | 239 +++++++++++++++++- .../questions/managing_organisation_spec.rb | 117 +++++---- .../lettings/questions/stock_owner_spec.rb | 74 ++++-- .../requests/lettings_logs_controller_spec.rb | 58 +++++ 14 files changed, 439 insertions(+), 99 deletions(-) diff --git a/app/components/check_answers_summary_list_card_component.rb b/app/components/check_answers_summary_list_card_component.rb index 246173f48..113303ba0 100644 --- a/app/components/check_answers_summary_list_card_component.rb +++ b/app/components/check_answers_summary_list_card_component.rb @@ -13,7 +13,7 @@ class CheckAnswersSummaryListCardComponent < ViewComponent::Base end def get_answer_label(question) - question.answer_label(log).presence || "You didn’t answer this question".html_safe + question.answer_label(log, user).presence || "You didn’t answer this question".html_safe end def check_answers_card_title(question) diff --git a/app/controllers/logs_controller.rb b/app/controllers/logs_controller.rb index 3982d4a54..083d40178 100644 --- a/app/controllers/logs_controller.rb +++ b/app/controllers/logs_controller.rb @@ -60,8 +60,9 @@ private end def org_params + owning_organisation_id = current_user.organisation.holds_own_stock? ? current_user.organisation.id : nil { - "owning_organisation_id" => current_user.organisation.id, + "owning_organisation_id" => owning_organisation_id, "managing_organisation_id" => current_user.organisation.id, "created_by_id" => current_user.id, } diff --git a/app/helpers/check_answers_helper.rb b/app/helpers/check_answers_helper.rb index 709bbacd6..66a833859 100644 --- a/app/helpers/check_answers_helper.rb +++ b/app/helpers/check_answers_helper.rb @@ -43,6 +43,6 @@ private end def get_answer_label(question, lettings_log) - question.answer_label(lettings_log).presence || "You didn’t answer this question".html_safe + question.answer_label(lettings_log, current_user).presence || "You didn’t answer this question".html_safe end end diff --git a/app/models/form/common/questions/created_by_id.rb b/app/models/form/common/questions/created_by_id.rb index 5b6631868..ee2767a4f 100644 --- a/app/models/form/common/questions/created_by_id.rb +++ b/app/models/form/common/questions/created_by_id.rb @@ -24,7 +24,7 @@ class Form::Common::Questions::CreatedById < ::Form::Question answer_options.select { |k, _v| user_ids.include?(k) } end - def label_from_value(value) + def label_from_value(value, _log = nil, _user = nil) return unless value answer_options[value] diff --git a/app/models/form/common/questions/owning_organisation_id.rb b/app/models/form/common/questions/owning_organisation_id.rb index c09968ef9..14e262f58 100644 --- a/app/models/form/common/questions/owning_organisation_id.rb +++ b/app/models/form/common/questions/owning_organisation_id.rb @@ -21,7 +21,7 @@ class Form::Common::Questions::OwningOrganisationId < ::Form::Question answer_options end - def label_from_value(value) + def label_from_value(value, _log = nil, _user = nil) return unless value answer_options[value] diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb index d61be8882..6402f14e0 100644 --- a/app/models/form/lettings/questions/created_by_id.rb +++ b/app/models/form/lettings/questions/created_by_id.rb @@ -27,7 +27,7 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question answer_options.select { |k, _v| user_ids.include?(k) } end - def label_from_value(value) + def label_from_value(value, _log = nil, _user = nil) return unless value answer_options[value] diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index bd4f57fe8..e2e3b3e3d 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -12,10 +12,15 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question def answer_options opts = { "" => "Select an option" } + return opts unless ActiveRecord::Base.connected? return opts unless current_user return opts unless log + if log.managing_organisation.present? + opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name }) + end + if current_user.support? if log.owning_organisation.holds_own_stock? opts[log.owning_organisation.id] = "#{log.owning_organisation.name} (Owning organisation)" @@ -34,7 +39,10 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question answer_options end - def label_from_value(value) + def label_from_value(value, log = nil, user = nil) + @log = log + @current_user = user + return unless value answer_options[value] diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index de82258a6..e2d2a633d 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/app/models/form/lettings/questions/stock_owner.rb @@ -11,8 +11,14 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question def answer_options answer_opts = { "" => "Select an option" } + return answer_opts unless ActiveRecord::Base.connected? return answer_opts unless current_user + return answer_opts unless log + + if log.owning_organisation_id.present? + answer_opts = answer_opts.merge({ log.owning_organisation.id => log.owning_organisation.name }) + end if !current_user.support? && current_user.organisation.holds_own_stock? answer_opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)" @@ -28,7 +34,10 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question answer_options end - def label_from_value(value) + def label_from_value(value, log = nil, user = nil) + @log = log + @current_user = user + return unless value answer_options[value] diff --git a/app/models/form/question.rb b/app/models/form/question.rb index ebc7b1fc6..cbfc6aae2 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -46,11 +46,11 @@ class Form::Question delegate :subsection, to: :page delegate :form, to: :subsection - def answer_label(log) + def answer_label(log, user = nil) return checkbox_answer_label(log) if type == "checkbox" return log[id]&.to_formatted_s(:govuk_date).to_s if type == "date" - answer = label_from_value(log[id]) if log[id].present? + answer = label_from_value(log[id], log, user) if log[id].present? answer_label = [prefix, format_value(answer), suffix_label(log)].join("") if answer inferred_answer_value(log) || answer_label @@ -151,7 +151,7 @@ class Form::Question end end - def label_from_value(value) + def label_from_value(value, _log = nil, _user = nil) return unless value label = case type diff --git a/db/schema.rb b/db/schema.rb index f79d25275..e05334946 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -490,16 +490,16 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_16_151942) do t.integer "mortgagelender" t.string "mortgagelenderother" t.integer "mortlen" - t.integer "extrabor" - t.integer "hhmemb" - t.integer "totadult" - t.integer "totchild" - t.integer "hhtype" t.string "pcode1" t.string "pcode2" t.integer "pcodenk" t.string "postcode_full" t.boolean "is_la_inferred" + t.integer "extrabor" + t.integer "hhmemb" + t.integer "totadult" + t.integer "totchild" + t.integer "hhtype" t.integer "hodate_check" t.bigint "bulk_upload_id" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index 725be4744..f145f9e7c 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -58,7 +58,8 @@ RSpec.describe "Lettings Log Features" do end context "when the signed is user is a Support user" do - let(:support_user) { create(:user, :support, last_sign_in_at: Time.zone.now) } + let(:organisation) { create(:organisation, name: "User org") } + let(:support_user) { create(:user, :support, last_sign_in_at: Time.zone.now, organisation:) } let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } let(:mfa_template_id) { User::MFA_TEMPLATE_ID } @@ -78,7 +79,7 @@ RSpec.describe "Lettings Log Features" do end context "when completing the setup lettings log section", :aggregate_failure do - it "includes the organisation and created by questions" do + it "includes the owning organisation and created by questions" do visit("/lettings-logs") click_button("Create a new lettings log") click_link("Set up this lettings log") @@ -88,14 +89,116 @@ RSpec.describe "Lettings Log Features" do click_button("Save and continue") log_id = page.current_path.scan(/\d/).join visit("lettings-logs/#{log_id}/setup/check-answers") - expect(page).to have_content("Stock owner #{support_user.organisation.name}") + expect(page).to have_content("Stock owner User org") expect(page).to have_content("You have answered 2 of 8 questions") end end + + context "when the owning organisation question isn't answered" do + it "doesn't show the managing agent question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + click_link("Skip for now") + expect(page).not_to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + end + end + + context "when the owning organisation question is answered" do + context "and the owning organisation doesn't hold stock" do + let(:managing_org) { create(:organisation, name: "Managing org") } + let!(:org_rel) { create(:organisation_relationship, parent_organisation: support_user.organisation, child_organisation: managing_org) } + + before do + support_user.organisation.update!(holds_own_stock: false) + end + + it "shows the managing organisation question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + select(managing_org.name, from: "lettings-log-managing-organisation-id-field") + click_button("Save and continue") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Managing agent Managing org") + expect(support_user.organisation.managing_agents).to eq([org_rel.child_organisation]) + end + end + + context "and the owning organisation does hold stock" do + before do + support_user.organisation.update!(holds_own_stock: true) + end + + context "and the owning organisation has no managing agents" do + it "doesn't show the managing organisation question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).not_to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).not_to have_content("Managing agent ") + end + end + + context "and the owning organisation has 1 or more managing agents" do + let(:managing_org1) { create(:organisation, name: "Managing org 1") } + let!(:org_rel1) { create(:organisation_relationship, parent_organisation: support_user.organisation, child_organisation: managing_org1) } + + it "does show the managing organisation question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + select(managing_org1.name, from: "lettings-log-managing-organisation-id-field") + click_button("Save and continue") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Managing agent Managing org 1") + end + + context "and the owning organisation has 2 or more managing agents" do + let(:managing_org2) { create(:organisation, name: "Managing org 2") } + let!(:org_rel2) { create(:organisation_relationship, parent_organisation: support_user.organisation, child_organisation: managing_org2) } + + context "and the organisation relationship for the selected managing agent is deleted" do + it "doesn't change the CYA page text to be 'You didn't answer this question'" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + select(support_user.organisation.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + select(managing_org1.name, from: "lettings-log-managing-organisation-id-field") + click_button("Save and continue") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Managing agent Managing org 1") + org_rel1.destroy! + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Managing agent Managing org 1") + expect(support_user.organisation.managing_agents).to eq([org_rel2.child_organisation]) + end + end + end + end + end + end end context "when the signed is user is not a Support user" do - let(:user) { create(:user) } + let(:organisation) { create(:organisation, name: "User org") } + let(:user) { create(:user, :data_coordinator, name: "User name", organisation:) } let(:devise_notify_mailer) { DeviseNotifyMailer.new } let(:notify_client) { instance_double(Notifications::Client) } @@ -110,15 +213,125 @@ RSpec.describe "Lettings Log Features" do end context "when completing the setup log section" do - it "does not include the organisation and created by questions" do - visit("/lettings-logs") - click_button("Create a new lettings log") - click_link("Set up this lettings log") - log_id = page.current_path.scan(/\d/).join - expect(page).to have_current_path("/lettings-logs/#{log_id}/needs-type") - visit("lettings-logs/#{log_id}/setup/check-answers") - expect(page).not_to have_content("Owning organisation #{user.organisation.name}") - expect(page).not_to have_content("Log owner #{user.name}") + context "and there is at most 1 potential stock owner" do + it "does not include the owning organisation and created by questions" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + expect(page).to have_current_path("/lettings-logs/#{log_id}/needs-type") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).not_to have_content("Stock owner ") + expect(page).not_to have_content("Log owner ") + end + end + + context "and there are 2 or more potential stock owners" do + let(:owning_org1) { create(:organisation, name: "Owning org 1") } + let!(:org_rel1) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org1) } + + it "does include the owning organisation question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Stock owner User org") + end + + context "and there are 3 or more potential stock owners" do + let(:owning_org2) { create(:organisation, name: "Owning org 2") } + let!(:org_rel2) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org2) } + + context "and the organisation relationship for the selected stock owner is deleted" do + it "doesn't change the CYA page text to be 'You didn't answer this question'" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner") + select(owning_org1.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Stock owner Owning org 1") + org_rel1.destroy! + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Stock owner Owning org 1") + expect(user.organisation.stock_owners).to eq([org_rel2.parent_organisation]) + end + end + end + end + + context "when the current user's organisation doesn't hold stock" do + let(:owning_org1) { create(:organisation, name: "Owning org 1") } + let(:owning_org2) { create(:organisation, name: "Owning org 2") } + let!(:org_rel1) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org1) } + let!(:org_rel2) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org2) } + + it "shows the managing organisation question" do + user.organisation.update!(holds_own_stock: false) + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner") + select(owning_org1.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + select(user.organisation.name, from: "lettings-log-managing-organisation-id-field") + click_button("Save and continue") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Managing agent User org") + expect(user.organisation.stock_owners).to eq([org_rel1.parent_organisation, org_rel2.parent_organisation]) + end + end + + context "when the current user's organisation does hold stock" do + let!(:owning_org) { create(:organisation, name: "Owning org") } + let!(:org_rel1) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org) } + + before do + user.organisation.update!(holds_own_stock: true) + end + + context "and the user's organisation has no managing agents" do + it "doesn't show the managing organisation question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner") + select(owning_org.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).not_to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).not_to have_content("Managing agent ") + expect(user.organisation.stock_owners).to eq([org_rel1.parent_organisation]) + end + end + + context "and the user's organisation has 1 or more managing agents" do + let(:managing_org) { create(:organisation, name: "Managing org") } + let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: managing_org) } + + it "does show the managing organisation question" do + visit("/lettings-logs") + click_button("Create a new lettings log") + click_link("Set up this lettings log") + log_id = page.current_path.scan(/\d/).join + expect(page).to have_current_path("/lettings-logs/#{log_id}/stock-owner") + select(user.organisation.name, from: "lettings-log-owning-organisation-id-field") + click_button("Save and continue") + expect(page).to have_current_path("/lettings-logs/#{log_id}/managing-organisation") + select(managing_org.name, from: "lettings-log-managing-organisation-id-field") + click_button("Save and continue") + visit("lettings-logs/#{log_id}/setup/check-answers") + expect(page).to have_content("Managing agent Managing org") + expect(user.organisation.managing_agents).to eq([org_rel2.child_organisation]) + end + end end end diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb index 9977f07be..9d289d455 100644 --- a/spec/models/form/lettings/questions/managing_organisation_spec.rb +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -52,86 +52,97 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do end end - context "when user not support and owns own stock" do - let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) } + context "when user is not support" do + let(:user_org) { create(:organisation, name: "User org") } + let(:user) { create(:user, :data_coordinator, organisation: user_org) } - let(:log) { create(:lettings_log) } - let!(:org_rel1) { create(:organisation_relationship, parent_organisation: user.organisation) } - let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation) } + let(:managing_org1) { create(:organisation, name: "Managing org 1") } + let(:managing_org2) { create(:organisation, name: "Managing org 2") } + let(:managing_org3) { create(:organisation, name: "Managing org 3") } - let(:options) do - { - "" => "Select an option", - user.organisation.id => "#{user.organisation.name} (Your organisation)", - org_rel1.child_organisation.id => org_rel1.child_organisation.name, - org_rel2.child_organisation.id => org_rel2.child_organisation.name, - } - end - - it "shows managing agents with own org at the top" do - expect(question.displayed_answer_options(log, user)).to eq(options) - end - end - - context "when user not support and does not own stock" do - let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) } - - let(:log) { create(:lettings_log) } - let!(:org_rel1) { create(:organisation_relationship, parent_organisation: user.organisation) } - let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation) } + let(:log) { create(:lettings_log, managing_organisation: managing_org1) } + let!(:org_rel1) { create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: managing_org2) } + let!(:org_rel2) { create(:organisation_relationship, parent_organisation: user.organisation, child_organisation: managing_org3) } let(:options) do { "" => "Select an option", - user.organisation.id => "#{user.organisation.name} (Your organisation)", - org_rel1.child_organisation.id => org_rel1.child_organisation.name, - org_rel2.child_organisation.id => org_rel2.child_organisation.name, + log.managing_organisation.id => "Managing org 1", + user.organisation.id => "User org (Your organisation)", + org_rel1.child_organisation.id => "Managing org 2", + org_rel2.child_organisation.id => "Managing org 3", } end - it "shows managing agents with own org at the top" do + it "shows current managing agent at top, followed by user's org (with hint), followed by the managing agents of the user's org" do expect(question.displayed_answer_options(log, user)).to eq(options) end end - context "when support user and org does not own own stock" do + context "when user is support" do let(:user) { create(:user, :support) } - let(:log_owning_org) { create(:organisation, holds_own_stock: false) } - let(:log) { create(:lettings_log, owning_organisation: log_owning_org) } - let!(:org_rel1) { create(:organisation_relationship, parent_organisation: log_owning_org) } - let!(:org_rel2) { create(:organisation_relationship, parent_organisation: log_owning_org) } - - let(:options) do - { - "" => "Select an option", - org_rel1.child_organisation.id => org_rel1.child_organisation.name, - org_rel2.child_organisation.id => org_rel2.child_organisation.name, - } + let(:log_owning_org) { create(:organisation, name: "Owning org") } + + let(:managing_org1) { create(:organisation, name: "Managing org 1") } + let(:managing_org2) { create(:organisation, name: "Managing org 2") } + let(:managing_org3) { create(:organisation, name: "Managing org 3") } + + let(:log) { create(:lettings_log, owning_organisation: log_owning_org, managing_organisation: managing_org1, created_by: nil) } + let!(:org_rel1) { create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: managing_org2) } + let!(:org_rel2) { create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: managing_org3) } + + context "when org owns stock" do + let(:options) do + { + "" => "Select an option", + log.managing_organisation.id => "Managing org 1", + log_owning_org.id => "Owning org (Owning organisation)", + org_rel1.child_organisation.id => "Managing org 2", + org_rel2.child_organisation.id => "Managing org 3", + } + end + + it "shows current managing agent at top, followed by the current owning organisation (with hint), followed by the managing agents of the current owning organisation" do + log_owning_org.update!(holds_own_stock: true) + expect(question.displayed_answer_options(log, user)).to eq(options) + end end - it "shows owning org managing agents with hint" do - expect(question.displayed_answer_options(log, user)).to eq(options) + context "when org does not own stock" do + let(:options) do + { + "" => "Select an option", + log.managing_organisation.id => "Managing org 1", + org_rel1.child_organisation.id => "Managing org 2", + org_rel2.child_organisation.id => "Managing org 3", + } + end + + it "shows current managing agent at top, followed by the managing agents of the current owning organisation" do + log_owning_org.update!(holds_own_stock: false) + expect(question.displayed_answer_options(log, user)).to eq(options) + end end end - context "when support user and org does own stock" do + context "when the owning-managing organisation relationship is deleted" do let(:user) { create(:user, :support) } - let(:log_owning_org) { create(:organisation, holds_own_stock: true) } - let(:log) { create(:lettings_log, owning_organisation: log_owning_org) } - let!(:org_rel1) { create(:organisation_relationship, parent_organisation: log_owning_org) } - let!(:org_rel2) { create(:organisation_relationship, parent_organisation: log_owning_org) } + + let(:owning_org) { create(:organisation, name: "Owning org", holds_own_stock: true) } + let(:managing_org) { create(:organisation, name: "Managing org", holds_own_stock: false) } + let(:org_rel) { create(:organisation_relationship, parent_organisation: owning_org, child_organisation: managing_org) } + let(:log) { create(:lettings_log, owning_organisation: owning_org, managing_organisation: managing_org, created_by: nil) } let(:options) do { "" => "Select an option", - log_owning_org.id => "#{log_owning_org.name} (Owning organisation)", - org_rel1.child_organisation.id => org_rel1.child_organisation.name, - org_rel2.child_organisation.id => org_rel2.child_organisation.name, + owning_org.id => "Owning org (Owning organisation)", + managing_org.id => "Managing org", } end - it "shows owning org managing agents - " do + it "doesn't remove the managing org from the list of allowed managing orgs" do + org_rel.destroy! expect(question.displayed_answer_options(log, user)).to eq(options) end end diff --git a/spec/models/form/lettings/questions/stock_owner_spec.rb b/spec/models/form/lettings/questions/stock_owner_spec.rb index f08318f02..41aebed89 100644 --- a/spec/models/form/lettings/questions/stock_owner_spec.rb +++ b/spec/models/form/lettings/questions/stock_owner_spec.rb @@ -42,28 +42,68 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do end end - context "when user not support and owns own stock" do - let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) } - let(:options) do - { - "" => "Select an option", - user.organisation.id => "#{user.organisation.name} (Your organisation)", - } - end + context "when user is not support" do + let(:user_org) { create(:organisation, name: "User org") } + let(:user) { create(:user, :data_coordinator, organisation: user_org) } + + let(:owning_org_1) { create(:organisation, name: "Owning org 1") } + let(:owning_org_2) { create(:organisation, name: "Owning org 2") } + let!(:org_rel) { create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) } + let(:log) { create(:lettings_log, owning_organisation: owning_org_1) } + + context "when user's org owns stock" do + let(:options) do + { + "" => "Select an option", + owning_org_1.id => "Owning org 1", + user.organisation.id => "User org (Your organisation)", + owning_org_2.id => "Owning org 2", + } + end - before do - question.current_user = user + it "shows current stock owner at top, followed by user's org (with hint), followed by the stock owners of the user's org" do + user.organisation.update!(holds_own_stock: true) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + + context "when the owning-managing organisation relationship is deleted" do + let(:options) do + { + "" => "Select an option", + user.organisation.id => "User org (Your organisation)", + owning_org_2.id => "Owning org 2", + } + end + + it "doesn't remove the housing provider from the list of allowed housing providers" do + log.update!(owning_organisation: owning_org_2) + expect(question.displayed_answer_options(log, user)).to eq(options) + org_rel.destroy! + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end end - it "shows stock owners with own org at the top" do - expect(question.answer_options).to eq(options) + context "when user's org doesn't own stock" do + let(:options) do + { + "" => "Select an option", + owning_org_1.id => "Owning org 1", + owning_org_2.id => "Owning org 2", + } + end + + it "shows current stock owner at top, followed by the stock owners of the user's org" do + user.organisation.update!(holds_own_stock: false) + expect(question.displayed_answer_options(log, user)).to eq(options) + end end end - context "when user support" do - before do - question.current_user = create(:user, :support) - end + context "when user is support" do + let(:user) { create(:user, :support) } + + let(:log) { create(:lettings_log) } let(:expected_opts) do Organisation.all.each_with_object(options) do |organisation, hsh| @@ -73,7 +113,7 @@ RSpec.describe Form::Lettings::Questions::StockOwner, type: :model do end it "shows all orgs" do - expect(question.answer_options).to eq(expected_opts) + expect(question.displayed_answer_options(log, user)).to eq(expected_opts) end end end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 862f82fa1..dc009a16d 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -143,6 +143,64 @@ RSpec.describe LettingsLogsController, type: :request do expect(whodunnit_actor).to be_a(User) expect(whodunnit_actor.id).to eq(user.id) end + + context "when creating a new log" do + context "when the user is support" do + let(:organisation) { FactoryBot.create(:organisation) } + let(:support_user) { FactoryBot.create(:user, :support, organisation:) } + + before do + allow(support_user).to receive(:need_two_factor_authentication?).and_return(false) + sign_in support_user + post "/lettings-logs", headers: + end + + it "sets the managing org and stock-owning org as nil" do + created_id = response.location.match(/[0-9]+/)[0] + lettings_log = LettingsLog.find_by(id: created_id) + expect(lettings_log.owning_organisation).to eq(nil) + expect(lettings_log.managing_organisation).to eq(nil) + end + end + + context "when the user is not support" do + context "when the user's org holds stock" do + let(:organisation) { FactoryBot.create(:organisation, name: "User org", holds_own_stock: true) } + let(:user) { FactoryBot.create(:user, :data_coordinator, organisation:) } + + before do + RequestHelper.stub_http_requests + sign_in user + post "/lettings-logs", headers: + end + + it "sets the managing org and stock-owning org as the user's org" do + created_id = response.location.match(/[0-9]+/)[0] + lettings_log = LettingsLog.find_by(id: created_id) + expect(lettings_log.owning_organisation.name).to eq("User org") + expect(lettings_log.managing_organisation.name).to eq("User org") + end + end + + context "when the user's org doesn't hold stock" do + let(:organisation) { FactoryBot.create(:organisation, name: "User org", holds_own_stock: false) } + let(:user) { FactoryBot.create(:user, :data_coordinator, organisation:) } + + before do + RequestHelper.stub_http_requests + sign_in user + post "/lettings-logs", headers: + end + + it "sets the managing org as the user's org but the stock-owning org as nil" do + created_id = response.location.match(/[0-9]+/)[0] + lettings_log = LettingsLog.find_by(id: created_id) + expect(lettings_log.owning_organisation).to eq(nil) + expect(lettings_log.managing_organisation.name).to eq("User org") + end + end + end + end end end