From a5f020bfee07295c08ede4c3dcef13087a4a053d Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 10 Aug 2023 14:12:41 +0100 Subject: [PATCH] CLDC-2108 Update sales owning organisation dropdown (#1821) * Display absorbed orgs/absorbed org stock owners * Only display orgs merged during open collection period * split guard clauses back * Do not show stock owners in sales logs owning org options * Update routed to method * Display active dates for support --- app/models/form/sales/pages/organisation.rb | 17 +-- .../sales/questions/owning_organisation_id.rb | 35 +++-- app/models/organisation.rb | 1 + .../form/sales/pages/organisation_spec.rb | 32 ++--- .../questions/owning_organisation_id_spec.rb | 132 +++++++++++++++--- 5 files changed, 158 insertions(+), 59 deletions(-) diff --git a/app/models/form/sales/pages/organisation.rb b/app/models/form/sales/pages/organisation.rb index 8f6728821..405cf51c1 100644 --- a/app/models/form/sales/pages/organisation.rb +++ b/app/models/form/sales/pages/organisation.rb @@ -10,24 +10,17 @@ class Form::Sales::Pages::Organisation < ::Form::Page ] end - def routed_to?(log, current_user) + def routed_to?(_log, current_user) return false unless current_user return true if current_user.support? - stock_owners = current_user.organisation.stock_owners + current_user.organisation.absorbed_organisations.where(holds_own_stock: true) + absorbed_stock_owners = current_user.organisation.absorbed_organisations.where(holds_own_stock: true) if current_user.organisation.holds_own_stock? - if current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) - return true - end - return true if stock_owners.count >= 1 - - log.update!(owning_organisation: current_user.organisation) + return true if absorbed_stock_owners.count >= 1 else - return false if stock_owners.count.zero? - return true if stock_owners.count > 1 - - log.update!(owning_organisation: stock_owners.first) + return false if absorbed_stock_owners.count.zero? + return true if absorbed_stock_owners.count > 1 end false diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index 0db5ec1a0..a09fe05ed 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -15,20 +15,35 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question return answer_opts unless log if log.owning_organisation_id.present? - answer_opts = answer_opts.merge({ log.owning_organisation.id => log.owning_organisation.name }) + answer_opts[log.owning_organisation.id] = log.owning_organisation.name end + recently_absorbed_organisations = user.organisation.absorbed_organisations.merged_during_open_collection_period if !user.support? && user.organisation.holds_own_stock? - answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)" + answer_opts[user.organisation.id] = if recently_absorbed_organisations.exists? + "#{user.organisation.name} (Your organisation, active as of #{user.organisation.created_at.to_fs(:govuk_date)})" + else + "#{user.organisation.name} (Your organisation)" + end end - user_answer_options = if user.support? - Organisation.where(holds_own_stock: true) - else - user.organisation.stock_owners + user.organisation.absorbed_organisations.where(holds_own_stock: true) - end.pluck(:id, :name).to_h + if user.support? + Organisation.where(holds_own_stock: true).find_each do |org| + if org.merge_date.present? + answer_opts[org.id] = "#{org.name} (inactive as of #{org.merge_date.to_fs(:govuk_date)})" if org.merge_date >= FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period + elsif org.absorbed_organisations.merged_during_open_collection_period.exists? + answer_opts[org.id] = "#{org.name} (active as of #{org.created_at.to_fs(:govuk_date)})" + else + answer_opts[org.id] = org.name + end + end + else + recently_absorbed_organisations.each do |absorbed_org| + answer_opts[absorbed_org.id] = merged_organisation_label(absorbed_org.name, absorbed_org.merge_date) if absorbed_org.holds_own_stock? + end + end - answer_opts.merge(user_answer_options) + answer_opts end def displayed_answer_options(log, user = nil) @@ -66,4 +81,8 @@ private def selected_answer_option_is_derived?(_log) true end + + def merged_organisation_label(name, merge_date) + "#{name} (inactive as of #{merge_date.to_fs(:govuk_date)})" + end end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 1c048703f..dac2b0ff5 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -32,6 +32,7 @@ class Organisation < ApplicationRecord scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by, ->(param) { search_by_name(param) } + scope :merged_during_open_collection_period, -> { where("merge_date >= ?", FormHandler.instance.start_date_of_earliest_open_for_editing_collection_period) } has_paper_trail diff --git a/spec/models/form/sales/pages/organisation_spec.rb b/spec/models/form/sales/pages/organisation_spec.rb index 710edbecc..8f160ca53 100644 --- a/spec/models/form/sales/pages/organisation_spec.rb +++ b/spec/models/form/sales/pages/organisation_spec.rb @@ -88,8 +88,8 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do expect(page.routed_to?(log, user)).to eq(false) end - it "updates owning_organisation_id" do - expect { page.routed_to?(log, user) }.to change(log.reload, :owning_organisation).from(nil).to(stock_owner) + it "does not update owning_organisation_id" do + expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation) end end @@ -111,11 +111,7 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do end it "is not shown" do - expect(page.routed_to?(log, user)).to eq(true) - end - - it "updates owning_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation) + expect(page.routed_to?(log, user)).to eq(false) end end end @@ -129,12 +125,6 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do it "is not shown" do expect(page.routed_to?(log, user)).to eq(false) end - - it "updates owning_organisation_id to user organisation" do - expect { - page.routed_to?(log, user) - }.to change(log.reload, :owning_organisation).from(nil).to(user.organisation) - end end context "with >0 stock_owners" do @@ -143,12 +133,20 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do create(:organisation_relationship, child_organisation: user.organisation) end - it "is shown" do - expect(page.routed_to?(log, user)).to eq(true) + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) end + end - it "does not update owning_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation).from(nil) + context "with merged organisations" do + let(:merged_org) { create(:organisation) } + + before do + merged_org.update!(absorbing_organisation: user.organisation, merge_date: Time.zone.now) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) end end end diff --git a/spec/models/form/sales/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb index 815a42319..9cfefbf8b 100644 --- a/spec/models/form/sales/questions/owning_organisation_id_spec.rb +++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb @@ -70,53 +70,99 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model 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 - 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 + it "does not show stock owner" do user.organisation.update!(holds_own_stock: true) expect(question.displayed_answer_options(log, user)).to eq(options) end + 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 + context "when user's org doesn't own stock" do + let(:options) do + { + "" => "Select an option", + owning_org_1.id => "Owning org 1", + } + 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 + it "shows current log owner" do + user.organisation.update!(holds_own_stock: false) + expect(question.displayed_answer_options(log, user)).to eq(options) end end - context "when user's org doesn't own stock" do + context "when user's org has recently absorbed other orgs" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } let(:options) do { "" => "Select an option", + user.organisation.id => "User org (Your organisation, active as of 2 February 2021)", owning_org_1.id => "Owning org 1", - owning_org_2.id => "Owning org 2", + merged_organisation.id => "Merged org (inactive as of 2 February 2023)", } 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) + before do + merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation) + user.organisation.update!(created_at: Time.zone.local(2021, 2, 2)) + end + + it "shows merged organisation as an option" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when user's org has recently absorbed other orgs with parent organisations" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:options) do + { + "" => "Select an option", + user.organisation.id => "User org (Your organisation, active as of 2 February 2021)", + owning_org_1.id => "Owning org 1", + merged_organisation.id => "Merged org (inactive as of 2 February 2023)", + } + end + + before do + org_rel.update!(child_organisation: merged_organisation) + merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation) + user.organisation.update!(created_at: Time.zone.local(2021, 2, 2)) + end + + it "does not show merged organisations stock owners as options" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when user's org has absorbed other orgs with parent organisations during closed collection periods" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:options) do + { + "" => "Select an option", + user.organisation.id => "User org (Your organisation)", + owning_org_1.id => "Owning org 1", + } + end + + before do + Timecop.freeze(Time.zone.local(2023, 4, 2)) + org_rel.update!(child_organisation: merged_organisation) + merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation) + user.organisation.update!(created_at: Time.zone.local(2021, 2, 2)) + end + + it "shows merged organisation as an option" do expect(question.displayed_answer_options(log, user)).to eq(options) end end end context "when user is support" do - let(:user) { create(:user, :support) } + let(:user) { create(:user, :support, organisation: organisation_1) } - let(:log) { create(:lettings_log) } + let(:log) { create(:lettings_log, created_by: user) } let(:non_stock_organisation) { create(:organisation, holds_own_stock: false) } let(:expected_opts) do @@ -130,6 +176,48 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do expect(question.displayed_answer_options(log, user)).to eq(expected_opts) expect(question.displayed_answer_options(log, user)).not_to include(non_stock_organisation.id) end + + context "when an org has recently absorbed other orgs" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:options) do + { + "" => "Select an option", + organisation_1.id => "first test org (active as of 2 February 2021)", + organisation_2.id => "second test org", + merged_organisation.id => "Merged org (inactive as of 2 February 2023)", + } + end + + before do + merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: organisation_1) + organisation_1.update!(created_at: Time.zone.local(2021, 2, 2)) + end + + it "shows merged organisation as an option" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when user's org has absorbed other orgs during closed collection periods" do + let(:merged_organisation) { create(:organisation, name: "Merged org") } + let(:options) do + { + "" => "Select an option", + organisation_1.id => "first test org", + organisation_2.id => "second test org", + } + end + + before do + Timecop.freeze(Time.zone.local(2023, 4, 2)) + merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation) + user.organisation.update!(created_at: Time.zone.local(2021, 2, 2)) + end + + it "shows merged organisation as an option" do + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end end end