From aa7df870d0cf640e56c05398aa9dd8483e1d8de6 Mon Sep 17 00:00:00 2001 From: Ben Shimmin Date: Fri, 6 Dec 2024 10:14:20 +0000 Subject: [PATCH] Allow users to have additional organisations This creates a `has_and_belongs_to_many` join table relationship between organisations and users, allowing a user to belong to `additional_organisations` and paves the way to allowing a user to set their `organisation` to be a `Current.user_organisation` and thus view data for that organisation instead of their own "real" organisation. This should change nothing about the existing functionality of the site and will be entirely invisible to end users. (The next phase will include front-end changes which *will* allow selected users to switch their current organisation.) Helper methods on the `User` model to navigate this relationship include `primary_organisation` (to retrieve the user's actual `organisation`, which is necessarily overwritten), `all_organisations` (the union of `primary_organisation` and `additional_organisations`) and `additional_organisations?` (to determine whether a user has a non-zero number of `additional_organisations`). --- CHANGELOG.md | 9 ++- README.md | 3 + app/models/current.rb | 3 + app/models/user.rb | 20 ++++++ .../active_support_deprecation_fix.rb | 11 ---- ...9_create_join_table_organisations_users.rb | 8 +++ db/schema.rb | 9 ++- ...rs-may-belong-to-multiple-organisations.md | 39 +++++++++++ spec/models/user_spec.rb | 64 +++++++++++++++++++ 9 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 app/models/current.rb delete mode 100644 config/initializers/active_support_deprecation_fix.rb create mode 100644 db/migrate/20241204220209_create_join_table_organisations_users.rb create mode 100644 doc/architecture/decisions/0038-users-may-belong-to-multiple-organisations.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 61d805da2..823a006a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ [Full changelog][unreleased] +- Migration (and logic) to allow users to belong to multiple organisations + +## Release 154 - 2024-12-05 + +[Full changelog][154] + - Remove Spring from development - Switch to the Zeitwerk autoloader - Use Rails 6.1 default configuration @@ -1723,7 +1729,8 @@ - Planned start and end dates are mandatory - Actual start and end dates must not be in the future -[unreleased]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-153...HEAD +[unreleased]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-154...HEAD +[154]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-153...release-154 [153]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-152...release-153 [152]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-151...release-152 [151]: https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/compare/release-150...release-151 diff --git a/README.md b/README.md index f2b665a1a..cbfe9722a 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,9 @@ each environment. If this is your first time running the application, see the [getting started documentation](/doc/getting-started.md) for instructions. +You will likely need access to this repository, contact Integrated Corporate +Services (ICS) via DSIT. + You will likely need access to the environments, see the [hosting documentation](/doc/hosting.md) on how to obtain this. diff --git a/app/models/current.rb b/app/models/current.rb new file mode 100644 index 000000000..2623ad992 --- /dev/null +++ b/app/models/current.rb @@ -0,0 +1,3 @@ +class Current < ActiveSupport::CurrentAttributes + attribute :user_organisation +end diff --git a/app/models/user.rb b/app/models/user.rb index 90bac71db..0e73e2c58 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,7 @@ class User < ApplicationRecord otp_secret_encryption_key: ENV["SECRET_KEY_BASE"] belongs_to :organisation + has_and_belongs_to_many :additional_organisations, class_name: "Organisation", join_table: "organisations_users" has_many :historical_events validates_presence_of :name, :email validates :email, with: :email_cannot_be_changed_after_create, on: :update @@ -18,6 +19,25 @@ class User < ApplicationRecord delegate :service_owner?, :partner_organisation?, to: :organisation + def organisation + if Current.user_organisation + return Organisation.find(Current.user_organisation) + end + super + end + + def primary_organisation + Organisation.find(organisation_id) + end + + def all_organisations + Organisation.where(id: [organisation_id, additional_organisations.map(&:id)].flatten) + end + + def additional_organisations? + additional_organisations.any? + end + def active_for_authentication? active end diff --git a/config/initializers/active_support_deprecation_fix.rb b/config/initializers/active_support_deprecation_fix.rb deleted file mode 100644 index 621f12bc5..000000000 --- a/config/initializers/active_support_deprecation_fix.rb +++ /dev/null @@ -1,11 +0,0 @@ -module ActiveSupport - class Deprecated - class DeprecatedConstantProxy < Module - # Remove "SourceAnnotationExtractor is deprecated" warnings that were - # interfering with pry's tab complete - # - # REMOVE AFTER Rails 6.0.4 RELEASE (https://github.com/rails/rails/pull/37468) - delegate :hash, :instance_methods, :name, :respond_to?, to: :target - end - end -end diff --git a/db/migrate/20241204220209_create_join_table_organisations_users.rb b/db/migrate/20241204220209_create_join_table_organisations_users.rb new file mode 100644 index 000000000..be314c350 --- /dev/null +++ b/db/migrate/20241204220209_create_join_table_organisations_users.rb @@ -0,0 +1,8 @@ +class CreateJoinTableOrganisationsUsers < ActiveRecord::Migration[6.1] + def change + create_join_table :organisations, :users, column_options: {type: :uuid} do |t| + t.index [:organisation_id, :user_id] + t.index [:user_id, :organisation_id] + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 3355aa58a..596c0cb34 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.define(version: 2024_11_14_114012) do +ActiveRecord::Schema.define(version: 2024_12_04_220209) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" @@ -286,6 +286,13 @@ t.index ["iati_reference"], name: "index_organisations_on_iati_reference", unique: true end + create_table "organisations_users", id: false, force: :cascade do |t| + t.uuid "organisation_id", null: false + t.uuid "user_id", null: false + t.index ["organisation_id", "user_id"], name: "index_organisations_users_on_organisation_id_and_user_id" + t.index ["user_id", "organisation_id"], name: "index_organisations_users_on_user_id_and_organisation_id" + end + create_table "outgoing_transfers", id: :uuid, default: -> { "public.gen_random_uuid()" }, force: :cascade do |t| t.uuid "source_id", null: false t.uuid "destination_id", null: false diff --git a/doc/architecture/decisions/0038-users-may-belong-to-multiple-organisations.md b/doc/architecture/decisions/0038-users-may-belong-to-multiple-organisations.md new file mode 100644 index 000000000..23e80e490 --- /dev/null +++ b/doc/architecture/decisions/0038-users-may-belong-to-multiple-organisations.md @@ -0,0 +1,39 @@ +# 38. Users may belong to multiple organisations + +Date: 2024-12-05 + +## Status + +Accepted + +## Context + +Some RODA users wish to have the ability to view reports, activities, etc from +other organisations. Currently these users have to have their organisation +changed for them so that they can do this, which requires manual intervention +by an administrator. + +## Decision + +Much of the logic in RODA is predicated on the idea that one user belongs to +one organisation; changing the relationship to one where a user has many +organisations would require a very large engineering effort. + +We believe the easiest way to allow a RODA user belonging to one organisation +to be able to view data belonging to other organisations is to maintain its +existing relationship where it belongs to one organisation, but to add an +additional relationship where a user has and belongs to many organisations, +which we are calling "additional organisations". This way, all the existing +logic and tests still work, but we can augment the functionality to allow a +user to switch their current organisation to be a different one from an +"additional organisations" list (the contents of which would be set by an +administrator for a given user). + +## Consequences + +It will be easier for users who need to see data from other organisations to be +able to do this without requiring manual intervention. + +The relationship between a user and its organisation is critical functionality +and so we must tread carefully. Our proposed solution does not require any +changes to the existing policy logic. \ No newline at end of file diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 598442d9d..00333f501 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -31,6 +31,70 @@ # This also validates that the relationship is present it { is_expected.to belong_to(:organisation) } it { is_expected.to have_many(:historical_events) } + it { is_expected.to have_and_belong_to_many(:additional_organisations) } + + it "has a primary organisation" do + user = create(:administrator) + + expect(user.primary_organisation).to be_valid + expect(user.primary_organisation.id).to eq(user.organisation_id) + end + + it "has additional organisations" do + org1 = create(:partner_organisation) + org2 = create(:partner_organisation) + org3 = create(:partner_organisation) + + user = create(:administrator) + user.additional_organisations << [org1, org2, org3] + + expect(user.additional_organisations.pluck(:id)).to include(org1.id) + expect(user.additional_organisations.count).to eq(3) + end + + it "shows all organisations including the primary organisation" do + org = create(:partner_organisation) + + user = create(:administrator) + user.additional_organisations << org + + expect(user.all_organisations.size).to eq(2) + expect(user.all_organisations.pluck(:id)).to include(user.primary_organisation.id) + expect(user.all_organisations.pluck(:id)).to include(org.id) + end + + it "determines whether there are additional organisations" do + org = create(:partner_organisation) + + user = create(:administrator) + user.additional_organisations << org + + expect(user.additional_organisations?).to eq(true) + + user.additional_organisations = [] + expect(user.additional_organisations?).to eq(false) + end + + context "when the current organisation has been set" do + let(:current_organisation) do + create(:partner_organisation) + end + + before do + Current.user_organisation = current_organisation.id + end + + it "returns the current organisation instead of the primary organisation" do + user = create(:administrator) + + expect(user.organisation).to eq(current_organisation) + expect(user.primary_organisation).not_to eq(current_organisation) + end + + after do + Current.user_organisation = nil + end + end end describe "delegations" do