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