diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 7ac1277bd..5b88655ad 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -52,13 +52,13 @@ module FiltersHelper JSON.parse(session[session_name_for(filter_type)])[filter] || "" end - def owning_organisations_filter_options(user) - organisation_options = user.support? ? Organisation.all : [user.organisation] + user.organisation.stock_owners + def owning_organisation_filter_options(user) + organisation_options = user.support? ? Organisation.all : ([user.organisation] + user.organisation.stock_owners + user.organisation.absorbed_organisations).uniq [OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) } end def assigned_to_filter_options(user) - user_options = user.support? ? User.all : (user.organisation.users + user.organisation.managing_agents.flat_map(&:users) + user.organisation.stock_owners.flat_map(&:users)) + user_options = user.support? ? User.all : (user.organisation.users + user.organisation.managing_agents.flat_map(&:users) + user.organisation.stock_owners.flat_map(&:users)).uniq [OpenStruct.new(id: "", name: "Select an option")] + user_options.map { |user_option| OpenStruct.new(id: user_option.id, name: user_option.name) } end @@ -77,7 +77,7 @@ module FiltersHelper end def managing_organisation_filter_options(user) - organisation_options = user.support? ? Organisation.all : [user.organisation] + user.organisation.managing_agents + organisation_options = user.support? ? Organisation.all : ([user.organisation] + user.organisation.managing_agents + user.organisation.absorbed_organisations).uniq [OpenStruct.new(id: "", name: "Select an option")] + organisation_options.map { |org| OpenStruct.new(id: org.id, name: org.name) } end diff --git a/app/models/form/lettings/questions/created_by_id.rb b/app/models/form/lettings/questions/created_by_id.rb index 4ed1efdd5..b2516808e 100644 --- a/app/models/form/lettings/questions/created_by_id.rb +++ b/app/models/form/lettings/questions/created_by_id.rb @@ -19,8 +19,14 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question users = [] users += if current_user.support? [ - (log.owning_organisation&.users if log.owning_organisation), - (log.managing_organisation&.users if log.managing_organisation), + ( + if log.owning_organisation + log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users : log.owning_organisation&.users + end), + ( + if log.managing_organisation + log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users : log.managing_organisation.users + end), ].flatten else current_user.organisation.users diff --git a/app/models/form/lettings/questions/managing_organisation.rb b/app/models/form/lettings/questions/managing_organisation.rb index 7e4d318fe..00e817ac3 100644 --- a/app/models/form/lettings/questions/managing_organisation.rb +++ b/app/models/form/lettings/questions/managing_organisation.rb @@ -28,10 +28,10 @@ class Form::Lettings::Questions::ManagingOrganisation < ::Form::Question end orgs = if user.support? - log.owning_organisation + log.owning_organisation.managing_agents else - user.organisation - end.managing_agents.pluck(:id, :name).to_h + user.organisation.managing_agents + user.organisation.absorbed_organisations + end.pluck(:id, :name).to_h opts.merge(orgs) end diff --git a/app/models/form/lettings/questions/stock_owner.rb b/app/models/form/lettings/questions/stock_owner.rb index 4294f440f..dc6179b21 100644 --- a/app/models/form/lettings/questions/stock_owner.rb +++ b/app/models/form/lettings/questions/stock_owner.rb @@ -23,13 +23,13 @@ class Form::Lettings::Questions::StockOwner < ::Form::Question answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)" end - stock_owners_answer_options = if user.support? - Organisation.where(holds_own_stock: true) - else - user.organisation.stock_owners - end.pluck(:id, :name).to_h + 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 - answer_opts.merge(stock_owners_answer_options) + answer_opts.merge(user_answer_options) end def displayed_answer_options(log, user = nil) diff --git a/app/models/form/common/pages/organisation.rb b/app/models/form/sales/pages/organisation.rb similarity index 61% rename from app/models/form/common/pages/organisation.rb rename to app/models/form/sales/pages/organisation.rb index eb799d1e9..1d61b86ac 100644 --- a/app/models/form/common/pages/organisation.rb +++ b/app/models/form/sales/pages/organisation.rb @@ -1,4 +1,4 @@ -class Form::Common::Pages::Organisation < ::Form::Page +class Form::Sales::Pages::Organisation < ::Form::Page def initialize(id, hsh, subsection) super @id = "organisation" @@ -6,7 +6,7 @@ class Form::Common::Pages::Organisation < ::Form::Page def questions @questions ||= [ - Form::Common::Questions::OwningOrganisationId.new(nil, nil, self), + Form::Sales::Questions::OwningOrganisationId.new(nil, nil, self), ] end diff --git a/app/models/form/sales/questions/created_by_id.rb b/app/models/form/sales/questions/created_by_id.rb index 44b64184e..e48b4f9e5 100644 --- a/app/models/form/sales/questions/created_by_id.rb +++ b/app/models/form/sales/questions/created_by_id.rb @@ -17,8 +17,17 @@ class Form::Sales::Questions::CreatedById < ::Form::Question return ANSWER_OPTS unless log.owning_organisation return ANSWER_OPTS unless current_user - users = current_user.support? ? log.owning_organisation.users : current_user.organisation.users - + users = [] + users += if current_user.support? + [ + ( + if log.owning_organisation + log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users : log.owning_organisation&.users + end), + ].flatten + else + current_user.organisation.users + end.uniq.compact users.each_with_object(ANSWER_OPTS.dup) do |user, hsh| hsh[user.id] = present_user(user) hsh diff --git a/app/models/form/common/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb similarity index 92% rename from app/models/form/common/questions/owning_organisation_id.rb rename to app/models/form/sales/questions/owning_organisation_id.rb index 14e262f58..fa838f744 100644 --- a/app/models/form/common/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -1,4 +1,4 @@ -class Form::Common::Questions::OwningOrganisationId < ::Form::Question +class Form::Sales::Questions::OwningOrganisationId < ::Form::Question def initialize(id, hsh, page) super @id = "owning_organisation_id" diff --git a/app/models/form/sales/subsections/setup.rb b/app/models/form/sales/subsections/setup.rb index 92a28db3e..1fbc2ac9e 100644 --- a/app/models/form/sales/subsections/setup.rb +++ b/app/models/form/sales/subsections/setup.rb @@ -7,7 +7,7 @@ class Form::Sales::Subsections::Setup < ::Form::Subsection def pages @pages ||= [ - Form::Common::Pages::Organisation.new(nil, nil, self), + Form::Sales::Pages::Organisation.new(nil, nil, self), Form::Sales::Pages::CreatedBy.new(nil, nil, self), Form::Sales::Pages::SaleDate.new(nil, nil, self), Form::Sales::Pages::PurchaserCode.new(nil, nil, self), diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index fb7f8f2ad..0ac0e4bf5 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -54,7 +54,7 @@ class LettingsLog < Log scope :unresolved, -> { where(unresolved: true) } scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } - scope :filter_by_owning_organisation, ->(org, _user = nil) { where(owning_organisation: org) } + scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } scope :filter_by_managing_organisation, ->(managing_organisation, _user = nil) { where(managing_organisation:) } scope :duplicate_logs, lambda { |log| diff --git a/app/models/organisation.rb b/app/models/organisation.rb index c050fef06..eea36f754 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -1,9 +1,6 @@ class Organisation < ApplicationRecord has_many :users, dependent: :delete_all has_many :data_protection_officers, -> { where(is_dpo: true) }, class_name: "User" - has_many :owned_lettings_logs, class_name: "LettingsLog", foreign_key: "owning_organisation_id", dependent: :delete_all - has_many :managed_lettings_logs, class_name: "LettingsLog", foreign_key: "managing_organisation_id" - has_many :owned_sales_logs, class_name: "SalesLog", foreign_key: "owning_organisation_id", dependent: :delete_all has_one :data_protection_confirmation has_many :organisation_rent_periods has_many :owned_schemes, class_name: "Scheme", foreign_key: "owning_organisation_id", dependent: :delete_all @@ -18,6 +15,9 @@ class Organisation < ApplicationRecord has_many :managing_agent_relationships, foreign_key: :parent_organisation_id, class_name: "OrganisationRelationship" has_many :managing_agents, through: :managing_agent_relationships, source: :child_organisation + belongs_to :absorbing_organisation, class_name: "Organisation", optional: true + has_many :absorbed_organisations, class_name: "Organisation", foreign_key: "absorbing_organisation_id" + def affiliated_stock_owners ids = [] @@ -64,11 +64,19 @@ class Organisation < ApplicationRecord end def lettings_logs - LettingsLog.filter_by_organisation(self) + LettingsLog.filter_by_organisation(absorbed_organisations + [self]) end def sales_logs - SalesLog.filter_by_organisation(self) + SalesLog.filter_by_owning_organisation(absorbed_organisations + [self]) + end + + def owned_lettings_logs + LettingsLog.filter_by_owning_organisation(absorbed_organisations + [self]) + end + + def managed_lettings_logs + LettingsLog.filter_by_managing_organisation(absorbed_organisations + [self]) end def address_string diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 47a3ef9f9..fc57f3ba3 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -40,8 +40,8 @@ class SalesLog < Log .or(filter_by_postcode(param)) .or(filter_by_id(param)) } - scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org) } - scope :filter_by_owning_organisation, ->(org, _user = nil) { where(owning_organisation: org) } + scope :filter_by_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } + scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } scope :duplicate_logs, lambda { |log| visible.where(log.slice(*DUPLICATE_LOG_ATTRIBUTES)) .where.not(id: log.id) diff --git a/app/models/user.rb b/app/models/user.rb index 2ca5a3db6..48177d635 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -7,9 +7,6 @@ class User < ApplicationRecord # Marked as optional because we validate organisation_id below instead so that # the error message is linked to the right field on the form belongs_to :organisation, optional: true - has_many :owned_lettings_logs, through: :organisation - has_many :managed_lettings_logs, through: :organisation - has_many :owned_sales_logs, through: :organisation has_many :legacy_users has_many :bulk_uploads @@ -82,7 +79,7 @@ class User < ApplicationRecord if support? LettingsLog.all else - LettingsLog.filter_by_organisation(organisation) + LettingsLog.filter_by_organisation(organisation.absorbed_organisations + [organisation]) end end @@ -90,10 +87,18 @@ class User < ApplicationRecord if support? SalesLog.all else - SalesLog.filter_by_organisation(organisation) + SalesLog.filter_by_owning_organisation(organisation.absorbed_organisations + [organisation]) end end + def owned_lettings_logs + LettingsLog.filter_by_owning_organisation(organisation.absorbed_organisations + [organisation]) + end + + def managed_lettings_logs + LettingsLog.filter_by_managing_organisation(organisation.absorbed_organisations + [organisation]) + end + def is_key_contact? is_key_contact end diff --git a/app/models/validations/setup_validations.rb b/app/models/validations/setup_validations.rb index 3e3569a55..055923536 100644 --- a/app/models/validations/setup_validations.rb +++ b/app/models/validations/setup_validations.rb @@ -41,7 +41,7 @@ module Validations::SetupValidations def validate_organisation(record) created_by, managing_organisation, owning_organisation = record.values_at("created_by", "managing_organisation", "owning_organisation") - unless [created_by, managing_organisation, owning_organisation].any?(&:blank?) || created_by.organisation == managing_organisation || created_by.organisation == owning_organisation + unless [created_by, managing_organisation, owning_organisation].any?(&:blank?) || ((created_by.organisation.absorbed_organisations + [created_by.organisation]) & [managing_organisation, owning_organisation]).present? record.errors.add :created_by, I18n.t("validations.setup.created_by.invalid") record.errors.add :owning_organisation_id, I18n.t("validations.setup.owning_organisation.invalid") record.errors.add :managing_organisation_id, I18n.t("validations.setup.managing_organisation.invalid") diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb index 8f73a12a3..cc3e3b4fb 100644 --- a/app/views/logs/_log_filters.html.erb +++ b/app/views/logs/_log_filters.html.erb @@ -74,7 +74,7 @@ type: "select", label: "Owning Organisation", category: "owning_organisation", - options: owning_organisations_filter_options(@current_user), + options: owning_organisation_filter_options(@current_user), }, }, }, diff --git a/db/migrate/20230719150610_add_absorbing_organisation_id_to_organisations.rb b/db/migrate/20230719150610_add_absorbing_organisation_id_to_organisations.rb new file mode 100644 index 000000000..a1946986f --- /dev/null +++ b/db/migrate/20230719150610_add_absorbing_organisation_id_to_organisations.rb @@ -0,0 +1,5 @@ +class AddAbsorbingOrganisationIdToOrganisations < ActiveRecord::Migration[7.0] + def change + add_reference :organisations, :absorbing_organisation, null: true, foreign_key: { to_table: :organisations } + end +end diff --git a/db/schema.rb b/db/schema.rb index 75282b2bd..ee5c1c1ad 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do +ActiveRecord::Schema[7.0].define(version: 2023_07_25_081029) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -434,6 +434,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do t.string "old_org_id" t.string "old_visible_id" t.datetime "merge_date" + t.bigint "absorbing_organisation_id" + t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end @@ -715,6 +717,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do add_foreign_key "locations", "schemes" add_foreign_key "organisation_relationships", "organisations", column: "child_organisation_id" add_foreign_key "organisation_relationships", "organisations", column: "parent_organisation_id" + add_foreign_key "organisations", "organisations", column: "absorbing_organisation_id" add_foreign_key "sales_logs", "organisations", column: "owning_organisation_id", on_delete: :cascade add_foreign_key "schemes", "organisations", column: "owning_organisation_id", on_delete: :cascade add_foreign_key "users", "organisations", on_delete: :cascade diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb index 10b18ac1e..6ab012907 100644 --- a/spec/helpers/filters_helper_spec.rb +++ b/spec/helpers/filters_helper_spec.rb @@ -161,23 +161,26 @@ RSpec.describe FiltersHelper do end end - describe "#owning_organisations_filter_options" do + describe "#owning_organisation_filter_options" do let(:parent_organisation) { FactoryBot.create(:organisation, name: "Parent organisation") } let(:child_organisation) { FactoryBot.create(:organisation, name: "Child organisation") } + let!(:absorbed_organisation) { FactoryBot.create(:organisation, name: "Absorbed organisation", absorbing_organisation: child_organisation) } before do FactoryBot.create(:organisation_relationship, parent_organisation:, child_organisation:) FactoryBot.create(:organisation, name: "Other organisation", id: 99) + user.organisation.reload end context "with a support user" do let(:user) { FactoryBot.create(:user, :support, organisation: child_organisation) } it "returns a list of all organisations" do - expect(owning_organisations_filter_options(user)).to eq([ + expect(owning_organisation_filter_options(user)).to eq([ OpenStruct.new(id: "", name: "Select an option"), - OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), OpenStruct.new(id: child_organisation.id, name: "Child organisation"), + OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"), + OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), OpenStruct.new(id: 99, name: "Other organisation"), ]) end @@ -186,11 +189,51 @@ RSpec.describe FiltersHelper do context "with a data coordinator user" do let(:user) { FactoryBot.create(:user, :data_coordinator, organisation: child_organisation) } - it "returns a list of paret orgs and your own organisation" do - expect(owning_organisations_filter_options(user)).to eq([ + it "returns a list of parent orgs and your own organisation" do + expect(owning_organisation_filter_options(user.reload)).to eq([ OpenStruct.new(id: "", name: "Select an option"), OpenStruct.new(id: child_organisation.id, name: "Child organisation"), OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), + OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"), + ]) + end + end + end + + describe "#managing_organisation_filter_options" do + let(:parent_organisation) { FactoryBot.create(:organisation, name: "Parent organisation") } + let(:child_organisation) { FactoryBot.create(:organisation, name: "Child organisation") } + let!(:absorbed_organisation) { FactoryBot.create(:organisation, name: "Absorbed organisation", absorbing_organisation: parent_organisation) } + + before do + FactoryBot.create(:organisation_relationship, parent_organisation:, child_organisation:) + FactoryBot.create(:organisation, name: "Other organisation", id: 99) + user.organisation.reload + end + + context "with a support user" do + let(:user) { FactoryBot.create(:user, :support, organisation: parent_organisation) } + + it "returns a list of all organisations" do + expect(managing_organisation_filter_options(user)).to eq([ + OpenStruct.new(id: "", name: "Select an option"), + OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), + OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"), + OpenStruct.new(id: child_organisation.id, name: "Child organisation"), + OpenStruct.new(id: 99, name: "Other organisation"), + ]) + end + end + + context "with a data coordinator user" do + let(:user) { FactoryBot.create(:user, :data_coordinator, organisation: parent_organisation) } + + it "returns a list of child orgs and your own organisation" do + expect(managing_organisation_filter_options(user.reload)).to eq([ + OpenStruct.new(id: "", name: "Select an option"), + OpenStruct.new(id: parent_organisation.id, name: "Parent organisation"), + OpenStruct.new(id: child_organisation.id, name: "Child organisation"), + OpenStruct.new(id: absorbed_organisation.id, name: "Absorbed organisation"), ]) end end diff --git a/spec/models/form/common/pages/organisation_spec.rb b/spec/models/form/sales/pages/organisation_spec.rb similarity index 95% rename from spec/models/form/common/pages/organisation_spec.rb rename to spec/models/form/sales/pages/organisation_spec.rb index bc56adbd7..62156031f 100644 --- a/spec/models/form/common/pages/organisation_spec.rb +++ b/spec/models/form/sales/pages/organisation_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe Form::Common::Pages::Organisation, type: :model do +RSpec.describe Form::Sales::Pages::Organisation, type: :model do subject(:page) { described_class.new(page_id, page_definition, subsection) } let(:page_id) { nil } diff --git a/spec/models/form/common/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb similarity index 97% rename from spec/models/form/common/questions/owning_organisation_id_spec.rb rename to spec/models/form/sales/questions/owning_organisation_id_spec.rb index 8b57bebe1..f6dc02c27 100644 --- a/spec/models/form/common/questions/owning_organisation_id_spec.rb +++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe Form::Common::Questions::OwningOrganisationId, type: :model do +RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do subject(:question) { described_class.new(question_id, question_definition, page) } let(:question_id) { nil } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 034d37015..32ba7bb10 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -39,7 +39,7 @@ RSpec.describe User, type: :model do end describe "#lettings_logs" do - let!(:owned_lettings_log) do + let!(:managed_lettings_log) do create( :lettings_log, :completed, @@ -47,7 +47,7 @@ RSpec.describe User, type: :model do created_by: user, ) end - let!(:managed_lettings_log) do + let!(:owned_lettings_log) do create( :lettings_log, created_by: user, @@ -58,6 +58,27 @@ RSpec.describe User, type: :model do it "has lettings logs through their organisation" do expect(user.lettings_logs.to_a).to match_array([owned_lettings_log, managed_lettings_log]) end + + context "when the user's organisation has absorbed another" do + let!(:absorbed_org) { create(:organisation, absorbing_organisation_id: user.organisation.id) } + let!(:absorbed_org_managed_lettings_log) do + create( + :lettings_log, + :completed, + managing_organisation: absorbed_org, + ) + end + let!(:absorbed_org_owned_lettings_log) do + create( + :lettings_log, + owning_organisation: absorbed_org, + ) + end + + it "has lettings logs through both their organisation and absorbed organisation" do + expect(user.reload.lettings_logs.to_a).to match_array([owned_lettings_log, managed_lettings_log, absorbed_org_owned_lettings_log, absorbed_org_managed_lettings_log]) + end + end end it "has a role" do diff --git a/spec/models/validations/setup_validations_spec.rb b/spec/models/validations/setup_validations_spec.rb index 7e343f266..bcd7924c5 100644 --- a/spec/models/validations/setup_validations_spec.rb +++ b/spec/models/validations/setup_validations_spec.rb @@ -465,7 +465,7 @@ RSpec.describe Validations::SetupValidations do describe "#validate_organisation" do let(:user) { create(:user) } - let(:other_organisation) { create(:organisation) } + let(:other_organisation) { create(:organisation, name: "other org") } it "validates if neither managing nor owning organisation is the same as created_by user organisation" do record.created_by = user @@ -478,7 +478,7 @@ RSpec.describe Validations::SetupValidations do expect(record.errors["managing_organisation_id"]).to include(I18n.t("validations.setup.managing_organisation.invalid")) end - it "doesn not validate if either managing or owning organisation is the same as current user organisation" do + it "does not validate if either managing or owning organisation is the same as current user organisation" do record.created_by = user record.owning_organisation = user.organisation record.managing_organisation = other_organisation