diff --git a/app/models/derived_variables/lettings_log_variables.rb b/app/models/derived_variables/lettings_log_variables.rb index c76d5c2d9..1b1817ec3 100644 --- a/app/models/derived_variables/lettings_log_variables.rb +++ b/app/models/derived_variables/lettings_log_variables.rb @@ -49,8 +49,8 @@ module DerivedVariables::LettingsLogVariables self.waityear = 2 if is_general_needs? # fixed term - self.prevten = 32 if managing_organisation.provider_type == "PRP" - self.prevten = 30 if managing_organisation.provider_type == "LA" + self.prevten = 32 if managing_organisation&.provider_type == "PRP" + self.prevten = 30 if managing_organisation&.provider_type == "LA" end end diff --git a/app/models/form/lettings/pages/managing_organisation.rb b/app/models/form/lettings/pages/managing_organisation.rb index fc2ef5e36..bc8534999 100644 --- a/app/models/form/lettings/pages/managing_organisation.rb +++ b/app/models/form/lettings/pages/managing_organisation.rb @@ -25,13 +25,6 @@ class Form::Lettings::Pages::ManagingOrganisation < ::Form::Page return false unless organisation return true unless organisation.holds_own_stock? - managing_agents = organisation.managing_agents - - return false if managing_agents.count.zero? - return true if managing_agents.count > 1 - - log.update!(managing_organisation: managing_agents.first) - - false + organisation.managing_agents.count >= 1 end end diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb index a120e72a8..d5921e935 100644 --- a/app/models/form/lettings/questions/created_by_id.rb +++ b/app/models/form/lettings/questions/created_by_id.rb @@ -35,8 +35,8 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question answer_options[value] end - def hidden_in_check_answers?(_log, current_user) - !current_user.support? + def hidden_in_check_answers?(_log, _current_user) + true end def derived? diff --git a/app/models/form/lettings/questions/housing_provider.rb b/app/models/form/lettings/questions/housing_provider.rb index 8d8db24b6..f46ea1632 100644 --- a/app/models/form/lettings/questions/housing_provider.rb +++ b/app/models/form/lettings/questions/housing_provider.rb @@ -42,10 +42,15 @@ class Form::Lettings::Questions::HousingProvider < ::Form::Question def hidden_in_check_answers?(_log, user = nil) @current_user = user - return false unless @current_user - return false if @current_user.support? + return false if current_user.support? - housing_providers_answer_options.count < 2 + housing_providers = current_user.organisation.housing_providers + + if current_user.organisation.holds_own_stock? + housing_providers.count.zero? + else + housing_providers.count <= 1 + end end def enabled diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index 42662202c..82ae2aabd 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -21,7 +21,7 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question if log.owning_organisation.holds_own_stock? opts[log.owning_organisation.id] = "#{log.owning_organisation.name} (Owning organisation)" end - elsif current_user.organisation.holds_own_stock? + else opts[current_user.organisation.id] = "#{current_user.organisation.name} (Your organisation)" end @@ -47,11 +47,7 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question def hidden_in_check_answers?(_log, user = nil) @current_user = user - - return false unless @current_user - return false if @current_user.support? - - managing_organisations_answer_options.count < 2 + @current_user.nil? end def enabled diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index 7836713e0..6d09b205a 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -89,8 +89,7 @@ RSpec.describe "Lettings Log Features" do log_id = page.current_path.scan(/\d/).join visit("lettings-logs/#{log_id}/setup/check-answers") expect(page).to have_content("Housing provider #{support_user.organisation.name}") - expect(page).to have_content("Log owner #{support_user.name}") - expect(page).to have_content("You have answered 3 of 9 questions") + expect(page).to have_content("You have answered 2 of 8 questions") end end end @@ -119,8 +118,7 @@ RSpec.describe "Lettings Log Features" do 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("User #{user.name}") - expect(page).to have_content("You have answered 0 of 6 questions") + expect(page).not_to have_content("Log owner #{user.name}") end end diff --git a/spec/models/form/lettings/pages/managing_organisation_spec.rb b/spec/models/form/lettings/pages/managing_organisation_spec.rb index 4a44aa25d..65c2589c6 100644 --- a/spec/models/form/lettings/pages/managing_organisation_spec.rb +++ b/spec/models/form/lettings/pages/managing_organisation_spec.rb @@ -40,10 +40,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is not shown" do expect(page.routed_to?(log, nil)).to eq(false) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, nil) }.not_to change(log.reload, :managing_organisation) - end end context "when support" do @@ -56,10 +52,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is shown" do expect(page.routed_to?(log, user)).to eq(true) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "when owning_organisation not set" do @@ -69,10 +61,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is not shown" do expect(page.routed_to?(log, user)).to eq(false) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "when holds own stock" do @@ -84,10 +72,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is not shown" do expect(page.routed_to?(log, user)).to eq(false) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "with >1 managing_agents" do @@ -99,10 +83,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is shown" do expect(page.routed_to?(log, user)).to eq(true) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "with 1 managing_agents" do @@ -117,12 +97,8 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do ) end - it "is not shown" do - expect(page.routed_to?(log, user)).to eq(false) - end - - it "updates managing_organisation_id" do - expect { page.routed_to?(log, user) }.to change(log.reload, :managing_organisation).to(managing_agent) + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) end end end @@ -137,10 +113,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is shown" do expect(page.routed_to?(log, user)).to eq(true) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "when holds own stock" do @@ -152,10 +124,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is not shown" do expect(page.routed_to?(log, user)).to eq(false) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "with >1 managing_agents" do @@ -167,10 +135,6 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do it "is shown" do expect(page.routed_to?(log, user)).to eq(true) end - - it "does not update managing_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :managing_organisation) - end end context "with 1 managing_agents" do @@ -185,12 +149,8 @@ RSpec.describe Form::Lettings::Pages::ManagingOrganisation, type: :model do ) end - it "is not shown" do - expect(page.routed_to?(log, user)).to eq(false) - end - - it "updates managing_organisation_id" do - expect { page.routed_to?(log, user) }.to change(log.reload, :managing_organisation).to(managing_agent) + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) end end end diff --git a/spec/models/form/lettings/questions/created_by_id_spec.rb b/spec/models/form/lettings/questions/created_by_id_spec.rb index 04fac20fb..a5e60ad2b 100644 --- a/spec/models/form/lettings/questions/created_by_id_spec.rb +++ b/spec/models/form/lettings/questions/created_by_id_spec.rb @@ -51,20 +51,8 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do expect(question.derived?).to be true end - context "when the current user is support" do - let(:support_user) { build(:user, :support) } - - it "is shown in check answers" do - expect(question.hidden_in_check_answers?(nil, support_user)).to be false - end - end - - context "when the current user is not support" do - let(:user) { build(:user) } - - it "is not shown in check answers" do - expect(question.hidden_in_check_answers?(nil, user)).to be true - end + it "is not shown in check answers" do + expect(question.hidden_in_check_answers?(nil, user_1)).to be true end context "when the owning organisation is already set" do diff --git a/spec/models/form/lettings/questions/housing_provider_spec.rb b/spec/models/form/lettings/questions/housing_provider_spec.rb index 073c59e37..5363a0227 100644 --- a/spec/models/form/lettings/questions/housing_provider_spec.rb +++ b/spec/models/form/lettings/questions/housing_provider_spec.rb @@ -83,34 +83,64 @@ RSpec.describe Form::Lettings::Questions::HousingProvider, type: :model do end describe "#hidden_in_check_answers?" do - context "when housing providers < 2" do - context "when not support user" do - let(:user) { create(:user) } + context "when support" do + let(:user) { create(:user, :support) } + + it "is not hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false + end + end + + context "when org holds own stock", :aggregate_failures do + let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) } + + context "when housing providers == 0" do + before do + user.organisation.housing_providers.delete_all + end it "is hidden in check answers" do + expect(user.organisation.housing_providers.count).to eq(0) expect(question.hidden_in_check_answers?(nil, user)).to be true end end - context "when support" do - let(:user) { create(:user, :support) } + context "when housing providers != 0" do + before do + create(:organisation_relationship, :owning, child_organisation: user.organisation) + end - it "is not hiddes in check answers" do + it "is visible in check answers" do + expect(user.organisation.housing_providers.count).to eq(1) expect(question.hidden_in_check_answers?(nil, user)).to be false end end end - context "when housing providers >= 2" do - let(:user) { create(:user) } + context "when org does not hold own stock", :aggregate_failures do + let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) } - before do - create(:organisation_relationship, :owning, child_organisation: user.organisation) - create(:organisation_relationship, :owning, child_organisation: user.organisation) + context "when housing providers <= 1" do + before do + create(:organisation_relationship, :owning, child_organisation: user.organisation) + end + + it "is hidden in check answers" do + expect(user.organisation.housing_providers.count).to eq(1) + expect(question.hidden_in_check_answers?(nil, user)).to be true + end end - it "is not hidden in check answers" do - expect(question.hidden_in_check_answers?(nil, user)).to be false + context "when housing providers >= 2" do + before do + create(:organisation_relationship, :owning, child_organisation: user.organisation) + create(:organisation_relationship, :owning, child_organisation: user.organisation) + end + + it "is visible in check answers" do + expect(user.organisation.housing_providers.count).to eq(2) + expect(question.hidden_in_check_answers?(nil, user)).to be false + 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 f80099d67..e045601bf 100644 --- a/spec/models/form/lettings/questions/managing_organisation_spec.rb +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -73,6 +73,27 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do 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, :managing, parent_organisation: user.organisation) } + let!(:org_rel2) { create(:organisation_relationship, :managing, parent_organisation: user.organisation) } + + 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 support user and org does not own own stock" do let(:user) { create(:user, :support) } let(:log_owning_org) { create(:organisation, holds_own_stock: false) } @@ -121,34 +142,19 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do end describe "#hidden_in_check_answers?" do - context "when housing providers < 2" do - context "when not support user" do - let(:user) { create(:user) } - - it "is hidden in check answers" do - expect(question.hidden_in_check_answers?(nil, user)).to be true - end - end - - context "when support" do - let(:user) { create(:user, :support) } + context "when user present" do + let(:user) { create(:user) } - it "is not hiddes in check answers" do - expect(question.hidden_in_check_answers?(nil, user)).to be false - end + it "is hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false end end - context "when managing agents >= 2" do - let(:user) { create(:user) } - - before do - create(:organisation_relationship, :managing, parent_organisation: user.organisation) - create(:organisation_relationship, :managing, parent_organisation: user.organisation) - end + context "when user not provided" do + let(:user) { create(:user, :support) } it "is not hidden in check answers" do - expect(question.hidden_in_check_answers?(nil, user)).to be false + expect(question.hidden_in_check_answers?(nil)).to be true end end end