Add many-to-many relationship between users and organizations (#1971)

### What changed in this PR and why

This introduces the many-to-many relationship that is a requirement for
some of our clients to belong to more than one organization.

Instead of a simply HABTM, it uses `has_many, through:`. This gives us
more flexibility to save information on the relationship itself. For
example, how do we handle active/inactive state. One idea would be to
add a `deactivated_at` in the `users_organizations` table.


### TODOs

- [ ] Add automated tests that test for a user that belongs to multiple
organizations
- [x] Decide on the deactivated/reactivated logic
- [x] When an admin is creating a new user, try to find the user by
their email first and do something - ie either simply add the user to
the organization, or throw a validation error and direct the admin to
edit the existing user. EDIT: The email is already validated for
uniqueness by the clearance gem. An error is displayed if someone tries
to create a user with the same email.

app/controllers/admin/users_controller.rb

@@ -4,7 +4,6 @@ module Admin
   class UsersController < Admin::ApplicationController
     def create
       user = User.new(resource_params)
-      user.organization = Organization.last unless user.admin?
 
       if user.save
         redirect_to admin_users_path(user), flash: { success: 'User was successfully created.' }

app/controllers/application_controller.rb

@@ -2,8 +2,7 @@
 
 class ApplicationController < ActionController::Base
   include Clearance::Controller
-  before_action :require_login, :require_otp_setup
-  before_action :set_organization
+  before_action :require_login, :require_otp_setup, :set_organization, :user_permitted?
 
   def require_otp_setup
     redirect_to otp_setup_path if signed_in? && !current_user.otp_enabled?
@@ -14,7 +13,7 @@ def sign_in(user)
 
     super(user) do |status|
       if status.success?
-        redirect_to organizations_path
+        redirect_to redirect_path
       else
         flash.now.alert = status.failure_message
         render template: 'sessions/new', status: :unauthorized
@@ -34,9 +33,21 @@ def delete_otp_session_variables
 
   private
 
+  def user_permitted?
+    raise ActionController::RoutingError, 'Not Found' unless current_user.admin? || @organization.in?(current_user.organizations)
+  end
+
   def set_organization
     @organization = Organization.find(params[:organization_id]) if params[:organization_id].present?
   rescue ActiveRecord::RecordNotFound
     raise ActionController::RoutingError, 'Not Found'
   end
+
+  def redirect_path
+    if current_user.admin? || current_user.organizations.count > 1
+      organizations_path
+    else
+      organization_dashboard_path(current_user.organizations.first)
+    end
+  end
 end

app/controllers/onboarding_controller.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class OnboardingController < ApplicationController
-  skip_before_action :require_login
+  skip_before_action :require_login, :user_permitted?
   before_action :ensure_onboarding_allowed
   before_action :verify_jwt, except: :success
   before_action :resume_telegram_onboarding, only: %i[index show]

app/controllers/organizations_controller.rb

@@ -1,13 +1,15 @@
 # frozen_string_literal: true
 
 class OrganizationsController < ApplicationController
-  skip_before_action :set_organization
+  skip_before_action :user_permitted?, :set_organization
   layout 'minimal'
 
   def index
-    # TODO: change this when having a organization habtm user relation
-    redirect_to organization_dashboard_path(current_user.organization) if current_user.organization.present?
-
     @organizations = Organization.all
   end
+
+  def set_organization
+    organization = Organization.find(params[:organization_id])
+    redirect_to organization_dashboard_path(organization)
+  end
 end

app/controllers/otp_auth_controller.rb

@@ -1,8 +1,7 @@
 # frozen_string_literal: true
 
 class OtpAuthController < ApplicationController
-  skip_before_action :require_login
-  skip_before_action :require_otp_setup
+  skip_before_action :require_login, :require_otp_setup, :user_permitted?, :set_organization
   before_action :redirect_if_signed_in, :redirect_unless_user_set, :reset_when_inactive
 
   layout 'minimal'
@@ -36,6 +35,6 @@ def reset_when_inactive
   end
 
   def redirect_if_signed_in
-    redirect_to organizations_path if signed_in?
+    redirect_to redirect_path if signed_in?
   end
 end

app/controllers/otp_setup_controller.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class OtpSetupController < ApplicationController
-  skip_before_action :require_otp_setup
+  skip_before_action :require_otp_setup, :user_permitted?, :set_organization
   before_action :redirect_if_set_up
 
   layout 'minimal'
@@ -14,7 +14,7 @@ def create
       current_user.save!
       session[:otp_verified_for_user] = current_user.id
 
-      redirect_back_or organizations_path
+      redirect_back_or redirect_path
     else
       flash.now[:error] = I18n.t('sessions.errors.otp_incorrect')
       render :show, status: :unauthorized
@@ -24,7 +24,7 @@ def create
   private
 
   def redirect_if_set_up
-    redirect_to organizations_path if current_user.otp_enabled?
+    redirect_to redirect_path if current_user.otp_enabled?
   end
 
   def otp_params

app/controllers/passwords_controller.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 class PasswordsController < Clearance::PasswordsController
-  skip_before_action :require_login
+  skip_before_action :require_login, :user_permitted?, :set_organization
 end

app/controllers/profile_controller.rb

@@ -7,7 +7,7 @@ def index; end
 
   def create_user
     password = SecureRandom.alphanumeric(20)
-    user = User.new(user_params.merge(password: password, organization: @organization))
+    user = User.new(user_params.merge(password: password, organizations: [@organization]))
     if user.save
       redirect_to organization_profile_path(@organization), flash: { success: I18n.t('profile.user.created_successfully') }
     else

app/controllers/sessions_controller.rb

@@ -1,8 +1,7 @@
 # frozen_string_literal: true
 
 class SessionsController < Clearance::SessionsController
-  skip_before_action :require_login
-  skip_before_action :require_otp_setup
+  skip_before_action :require_login, :require_otp_setup, :user_permitted?, :set_organization
 
   def create
     user = authenticate(params)

app/controllers/threema/webhook_controller.rb

@@ -3,7 +3,7 @@
 require 'openssl'
 
 class Threema::WebhookController < ApplicationController
-  skip_before_action :require_login, :verify_authenticity_token
+  skip_before_action :require_login, :verify_authenticity_token, :user_permitted?, :set_organization
 
   attr_reader :adapter
 

app/controllers/whats_app/three_sixty_dialog_webhook_controller.rb

@@ -4,8 +4,7 @@ module WhatsApp
   class ThreeSixtyDialogWebhookController < ApplicationController
     include WhatsAppHandleCallbacks
 
-    skip_before_action :require_login, :verify_authenticity_token
-    before_action :organization
+    skip_before_action :require_login, :verify_authenticity_token, :user_permitted?
 
     # rubocop:disable Metrics/AbcSize
     def message
@@ -57,10 +56,6 @@ def create_api_key
 
     private
 
-    def organization
-      @organization ||= Organization.find(params['organization_id'])
-    end
-
     def message_params
       params.permit({ three_sixty_dialog_webhook:
                         [contacts: [:wa_id, { profile: [:name] }],

app/controllers/whats_app/webhook_controller.rb

@@ -6,7 +6,7 @@ module WhatsApp
   class WebhookController < ApplicationController
     include WhatsAppHandleCallbacks
 
-    skip_before_action :require_login, :verify_authenticity_token
+    skip_before_action :require_login, :verify_authenticity_token, :user_permitted?
     before_action :set_organization, only: :status
     before_action :set_contributor, only: :status
 

app/dashboards/user_dashboard.rb

@@ -13,7 +13,8 @@ class UserDashboard < Administrate::BaseDashboard
     created_at: Field::DateTime,
     updated_at: Field::DateTime,
     deactivated_at: Field::DateTime,
-    active: Field::Boolean
+    active: Field::Boolean,
+    organizations: Field::HasMany
   }.freeze
 
   COLLECTION_ATTRIBUTES = %i[
@@ -45,6 +46,7 @@ class UserDashboard < Administrate::BaseDashboard
     admin
     otp_enabled
     active
+    organizations
   ].freeze
 
   COLLECTION_FILTERS = {}.freeze

app/models/organization.rb

@@ -9,7 +9,8 @@ class Organization < ApplicationRecord
 
   belongs_to :business_plan
   belongs_to :contact_person, class_name: 'User', optional: true
-  has_many :users, class_name: 'User', dependent: :destroy
+  has_many :users_organizations, dependent: :destroy
+  has_many :users, through: :users_organizations
   has_many :contributors, dependent: :destroy
   has_many :requests, dependent: :destroy
   has_many :notifications_as_mentioned, class_name: 'ActivityNotification', dependent: :destroy

app/models/user.rb

@@ -9,7 +9,8 @@ class User < ApplicationRecord
            dependent: :destroy
   has_many :notifications_as_mentioned, class_name: 'ActivityNotification', dependent: :destroy
   has_many :messages, as: :sender, dependent: :destroy
-  belongs_to :organization, optional: true
+  has_many :users_organizations, dependent: :destroy
+  has_many :organizations, through: :users_organizations
 
   has_one_time_password
   validates :password, length: { in: 8..128 }, unless: :skip_password_validation?
@@ -48,10 +49,14 @@ def active=(value)
   private
 
   def notify_admin
-    return unless organization && User.admin(false).count > organization.business_plan.number_of_users
+    return unless organizations.any? { |organization| organization.users.admin(false).count > organization.business_plan.number_of_users }
 
     User.admin.find_each do |admin|
-      PostmarkAdapter::Outbound.send_user_count_exceeds_plan_limit_message!(admin, organization)
+      organizations.each do |organization|
+        next unless organization.users.admin(false).count > organization.business_plan.number_of_users
+
+        PostmarkAdapter::Outbound.send_user_count_exceeds_plan_limit_message!(admin, organization)
+      end
     end
   end
 

app/models/users_organization.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+class UsersOrganization < ApplicationRecord
+  belongs_to :user
+  belongs_to :organization
+end

db/data/20240812102718_migrate_users_to_users_organizations.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class MigrateUsersToUsersOrganizations < ActiveRecord::Migration[6.1]
+  def up
+    User.admin(false).find_each do |user|
+      UsersOrganization.create!(user_id: user.id, organization_id: user.organization_id)
+    end
+  end
+
+  def down
+    UsersOrganization.destroy_all
+  end
+end

db/data_schema.rb

@@ -1,3 +1,3 @@
 # frozen_string_literal: true
 
-DataMigrate::Data.define(version: 20_240_730_085_839)
+DataMigrate::Data.define(version: 20_240_812_102_718)

db/migrate/20240812102032_create_users_organizations.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+class CreateUsersOrganizations < ActiveRecord::Migration[6.1]
+  def change
+    create_table :users_organizations do |t|
+      t.references :user, null: false, foreign_key: true
+      t.references :organization, null: false, foreign_key: true
+
+      t.timestamps
+    end
+  end
+end

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_07_26_065204) do
+ActiveRecord::Schema.define(version: 2024_08_12_102032) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -336,6 +336,15 @@
     t.index ["remember_token"], name: "index_users_on_remember_token"
   end
 
+  create_table "users_organizations", force: :cascade do |t|
+    t.bigint "user_id", null: false
+    t.bigint "organization_id", null: false
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+    t.index ["organization_id"], name: "index_users_organizations_on_organization_id"
+    t.index ["user_id"], name: "index_users_organizations_on_user_id"
+  end
+
   add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id"
   add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id"
   add_foreign_key "activity_notifications", "contributors"
@@ -356,4 +365,6 @@
   add_foreign_key "requests", "users"
   add_foreign_key "taggings", "tags"
   add_foreign_key "users", "organizations"
+  add_foreign_key "users_organizations", "organizations"
+  add_foreign_key "users_organizations", "users"
 end

db/seeds/multi_tenancy.rb

@@ -15,7 +15,7 @@
   )
 end
 users = 10.times.collect do
-  FactoryBot.create(:user, organization: organizations.sample)
+  FactoryBot.create(:user, organizations: [organizations.sample])
 end
 
 # images = 10.times.map { URI(Faker::Avatar.image(size: '50x50', format: 'png', set: 'set5')) }

spec/factories/organizations.rb

@@ -22,7 +22,7 @@
 
     after(:create) do |org, evaluator|
       if evaluator.users_count.positive?
-        org.users << create_list(:user, evaluator.users_count, organization: org)
+        org.users << create_list(:user, evaluator.users_count, organizations: [org])
         org.save!
       end
     end

spec/factories/users_organizations.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :users_organization do
+    user { nil }
+    organization { nil }
+  end
+end

spec/jobs/mark_inactive_contributor_inactive_job_spec.rb

@@ -34,9 +34,7 @@
         context 'that does belong to the organization' do
           before do
             contributor.update(organization_id: organization.id)
-            non_admin_user.update(organization_id: organization.id)
-            organization.users << non_admin_user
-            organization.save!
+            non_admin_user.update(organizations: [organization])
           end
 
           it { is_expected.to change { contributor.reload.deactivated_at }.from(nil).to(kind_of(ActiveSupport::TimeWithZone)) }

spec/jobs/resubscribe_contributor_job_spec.rb

@@ -4,7 +4,7 @@
 
 RSpec.describe ResubscribeContributorJob do
   describe '#perform_later(contributor_id, adapter)' do
-    let(:user) { create(:user, organization: organization) }
+    let(:user) { create(:user, organizations: [organization]) }
     let(:organization) { create(:organization) }
 
     subject { -> { described_class.new.perform(organization.id, contributor.id, adapter) } }