From 7d592d476138661f4ab2b62b026e2269a07f51f0 Mon Sep 17 00:00:00 2001 From: Ryan Arakawa Date: Tue, 30 Sep 2025 16:08:40 -0500 Subject: [PATCH] CP-11138 partnership refactor (#22) * account group to partnership rename * this is mostly converting the name account_group => partnership * partnership user relationships are API request based * so we don't need to maintain relational information in two databases, this many to many relationship is now handled via API context * rubocop and test fixes --- app/controllers/templates_controller.rb | 2 +- app/models/account.rb | 11 +--- app/models/account_group.rb | 36 ------------ .../concerns/account_group_validation.rb | 19 ------- app/models/concerns/partnership_validation.rb | 19 +++++++ app/models/export_location.rb | 22 ++++---- app/models/partnership.rb | 36 ++++++++++++ app/models/template.rb | 14 ++--- app/models/template_folder.rb | 28 +++++----- app/models/user.rb | 18 ------ app/services/external_auth_service.rb | 47 +++++++++++----- app/services/template_service.rb | 15 ++++- ...0_rename_account_groups_to_partnerships.rb | 18 ++++++ db/schema.rb | 37 ++++++------ lib/abilities/template_conditions.rb | 51 ++++++++++++++--- lib/template_folders.rb | 14 ++++- spec/factories/account_groups.rb | 8 --- spec/factories/partnerships.rb | 8 +++ spec/models/account_group_spec.rb | 56 ------------------- spec/models/account_spec.rb | 23 +------- .../concerns/account_group_validation_spec.rb | 38 ------------- .../concerns/partnership_validation_spec.rb | 15 +++++ spec/models/partnership_spec.rb | 40 +++++++++++++ spec/models/user_spec.rb | 25 --------- spec/services/external_auth_service_spec.rb | 18 +++--- spec/services/template_service_spec.rb | 24 ++++---- 26 files changed, 314 insertions(+), 328 deletions(-) delete mode 100644 app/models/account_group.rb delete mode 100644 app/models/concerns/account_group_validation.rb create mode 100644 app/models/concerns/partnership_validation.rb create mode 100644 app/models/partnership.rb create mode 100644 db/migrate/20250924174100_rename_account_groups_to_partnerships.rb delete mode 100644 spec/factories/account_groups.rb create mode 100644 spec/factories/partnerships.rb delete mode 100644 spec/models/account_group_spec.rb delete mode 100644 spec/models/concerns/account_group_validation_spec.rb create mode 100644 spec/models/concerns/partnership_validation_spec.rb create mode 100644 spec/models/partnership_spec.rb diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index f8908f27..c000263d 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -164,7 +164,7 @@ class TemplatesController < ApplicationController return unless authorized_clone_account_id?(params[:account_id]) @template.account_id = params[:account_id] - @template.account_group = nil + @template.partnership = nil @template.folder = @template.account.default_template_folder if @template.account_id != current_account&.id end diff --git a/app/models/account.rb b/app/models/account.rb index fc9803b5..d7d3c96d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -12,23 +12,16 @@ # uuid :string not null # created_at :datetime not null # updated_at :datetime not null -# account_group_id :bigint # external_account_id :integer # # Indexes # -# index_accounts_on_account_group_id (account_group_id) # index_accounts_on_external_account_id (external_account_id) UNIQUE # index_accounts_on_uuid (uuid) UNIQUE # -# Foreign Keys -# -# fk_rails_... (account_group_id => account_groups.id) -# class Account < ApplicationRecord attribute :uuid, :string, default: -> { SecureRandom.uuid } - belongs_to :account_group, optional: true has_many :users, dependent: :destroy has_many :encrypted_configs, dependent: :destroy has_many :account_configs, dependent: :destroy @@ -66,9 +59,9 @@ class Account < ApplicationRecord scope :active, -> { where(archived_at: nil) } - def self.find_or_create_by_external_id(external_id, attributes = {}) + def self.find_or_create_by_external_id(external_id, name, attributes = {}) find_by(external_account_id: external_id) || - create!(attributes.merge(external_account_id: external_id)) + create!(attributes.merge(external_account_id: external_id, name: name)) end def testing? diff --git a/app/models/account_group.rb b/app/models/account_group.rb deleted file mode 100644 index e2bde3f0..00000000 --- a/app/models/account_group.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -# == Schema Information -# -# Table name: account_groups -# -# id :bigint not null, primary key -# name :string not null -# created_at :datetime not null -# updated_at :datetime not null -# external_account_group_id :integer not null -# -# Indexes -# -# index_account_groups_on_external_account_group_id (external_account_group_id) UNIQUE -# -class AccountGroup < ApplicationRecord - has_many :accounts, dependent: :nullify - has_many :users, dependent: :nullify - has_many :templates, dependent: :destroy - has_many :template_folders, dependent: :destroy - - validates :external_account_group_id, presence: true, uniqueness: true - validates :name, presence: true - - def self.find_or_create_by_external_id(external_id, attributes = {}) - find_by(external_account_group_id: external_id) || - create!(attributes.merge(external_account_group_id: external_id)) - end - - def default_template_folder - template_folders.find_by(name: TemplateFolder::DEFAULT_NAME) || - template_folders.create!(name: TemplateFolder::DEFAULT_NAME, - author_id: users.minimum(:id)) - end -end diff --git a/app/models/concerns/account_group_validation.rb b/app/models/concerns/account_group_validation.rb deleted file mode 100644 index 028e03f8..00000000 --- a/app/models/concerns/account_group_validation.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -module AccountGroupValidation - extend ActiveSupport::Concern - - included do - validate :must_belong_to_account_or_account_group - end - - private - - def must_belong_to_account_or_account_group - if account.blank? && account_group.blank? - errors.add(:base, 'Must belong to either an account or account group') - elsif account.present? && account_group.present? - errors.add(:base, 'Cannot belong to both account and account group') - end - end -end diff --git a/app/models/concerns/partnership_validation.rb b/app/models/concerns/partnership_validation.rb new file mode 100644 index 00000000..915d957d --- /dev/null +++ b/app/models/concerns/partnership_validation.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module PartnershipValidation + extend ActiveSupport::Concern + + included do + validate :must_belong_to_account_or_partnership + end + + private + + def must_belong_to_account_or_partnership + if account.blank? && partnership.blank? + errors.add(:base, 'Must belong to either an account or partnership') + elsif account.present? && partnership.present? + errors.add(:base, 'Cannot belong to both account and partnership') + end + end +end diff --git a/app/models/export_location.rb b/app/models/export_location.rb index 4090ef84..cf0c364d 100644 --- a/app/models/export_location.rb +++ b/app/models/export_location.rb @@ -4,17 +4,19 @@ # # Table name: export_locations # -# id :bigint not null, primary key -# api_base_url :string not null -# authorization_token :string -# default_location :boolean default(FALSE), not null -# extra_params :jsonb not null -# name :string not null -# submissions_endpoint :string -# templates_endpoint :string -# created_at :datetime not null -# updated_at :datetime not null +# id :bigint not null, primary key +# api_base_url :string not null +# authorization_token :string +# default_location :boolean default(FALSE), not null +# extra_params :jsonb not null +# name :string not null +# submissions_endpoint :string +# templates_endpoint :string +# created_at :datetime not null +# updated_at :datetime not null +# global_partnership_id :integer # +# global_partnership_id is the Docuseal partnership ID associated with the export location class ExportLocation < ApplicationRecord validates :name, presence: true validates :api_base_url, presence: true diff --git a/app/models/partnership.rb b/app/models/partnership.rb new file mode 100644 index 00000000..c0591395 --- /dev/null +++ b/app/models/partnership.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: partnerships +# +# id :bigint not null, primary key +# name :string not null +# created_at :datetime not null +# updated_at :datetime not null +# external_partnership_id :integer not null +# +# Indexes +# +# index_partnerships_on_external_partnership_id (external_partnership_id) UNIQUE +# +class Partnership < ApplicationRecord + has_many :templates, dependent: :destroy + has_many :template_folders, dependent: :destroy + + validates :external_partnership_id, presence: true, uniqueness: true + validates :name, presence: true + + def self.find_or_create_by_external_id(external_id, name, attributes = {}) + find_by(external_partnership_id: external_id) || + create!(attributes.merge(external_partnership_id: external_id, name: name)) + end + + def default_template_folder(author) + raise ArgumentError, 'Author is required for partnership template folders' unless author + + template_folders.find_by(name: TemplateFolder::DEFAULT_NAME) || + template_folders.create!(name: TemplateFolder::DEFAULT_NAME, + author: author) + end +end diff --git a/app/models/template.rb b/app/models/template.rb index 34cffdba..e7399124 100644 --- a/app/models/template.rb +++ b/app/models/template.rb @@ -17,38 +17,38 @@ # submitters :text not null # created_at :datetime not null # updated_at :datetime not null -# account_group_id :bigint # account_id :integer # author_id :integer not null # external_id :string # folder_id :integer not null +# partnership_id :bigint # # Indexes # -# index_templates_on_account_group_id (account_group_id) # index_templates_on_account_id (account_id) # index_templates_on_account_id_and_folder_id_and_id (account_id,folder_id,id) WHERE (archived_at IS NULL) # index_templates_on_account_id_and_id_archived (account_id,id) WHERE (archived_at IS NOT NULL) # index_templates_on_author_id (author_id) # index_templates_on_external_id (external_id) # index_templates_on_folder_id (folder_id) +# index_templates_on_partnership_id (partnership_id) # index_templates_on_slug (slug) UNIQUE # # Foreign Keys # -# fk_rails_... (account_group_id => account_groups.id) # fk_rails_... (account_id => accounts.id) # fk_rails_... (author_id => users.id) # fk_rails_... (folder_id => template_folders.id) +# fk_rails_... (partnership_id => partnerships.id) # class Template < ApplicationRecord - include AccountGroupValidation + include PartnershipValidation DEFAULT_SUBMITTER_NAME = 'Employee' belongs_to :author, class_name: 'User' belongs_to :account, optional: true - belongs_to :account_group, optional: true + belongs_to :partnership, optional: true belongs_to :folder, class_name: 'TemplateFolder' has_one :search_entry, as: :record, inverse_of: :record, dependent: :destroy @@ -92,8 +92,8 @@ class Template < ApplicationRecord def maybe_set_default_folder if account.present? self.folder ||= account.default_template_folder - elsif account_group.present? - self.folder ||= account_group.default_template_folder + elsif partnership.present? + self.folder ||= partnership.default_template_folder(author) end end end diff --git a/app/models/template_folder.rb b/app/models/template_folder.rb index c06ffeed..601940ac 100644 --- a/app/models/template_folder.rb +++ b/app/models/template_folder.rb @@ -4,35 +4,35 @@ # # Table name: template_folders # -# id :bigint not null, primary key -# archived_at :datetime -# name :string not null -# created_at :datetime not null -# updated_at :datetime not null -# account_group_id :bigint -# account_id :integer -# author_id :integer not null +# id :bigint not null, primary key +# archived_at :datetime +# name :string not null +# created_at :datetime not null +# updated_at :datetime not null +# account_id :integer +# author_id :integer not null +# partnership_id :bigint # # Indexes # -# index_template_folders_on_account_group_id (account_group_id) -# index_template_folders_on_account_id (account_id) -# index_template_folders_on_author_id (author_id) +# index_template_folders_on_account_id (account_id) +# index_template_folders_on_author_id (author_id) +# index_template_folders_on_partnership_id (partnership_id) # # Foreign Keys # -# fk_rails_... (account_group_id => account_groups.id) # fk_rails_... (account_id => accounts.id) # fk_rails_... (author_id => users.id) +# fk_rails_... (partnership_id => partnerships.id) # class TemplateFolder < ApplicationRecord - include AccountGroupValidation + include PartnershipValidation DEFAULT_NAME = 'Default' belongs_to :author, class_name: 'User' belongs_to :account, optional: true - belongs_to :account_group, optional: true + belongs_to :partnership, optional: true has_many :templates, dependent: :destroy, foreign_key: :folder_id, inverse_of: :folder has_many :active_templates, -> { where(archived_at: nil) }, diff --git a/app/models/user.rb b/app/models/user.rb index 0649504b..01f0b24f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,13 +28,11 @@ # uuid :string not null # created_at :datetime not null # updated_at :datetime not null -# account_group_id :bigint # account_id :integer # external_user_id :integer # # Indexes # -# index_users_on_account_group_id (account_group_id) # index_users_on_account_id (account_id) # index_users_on_email (email) UNIQUE # index_users_on_external_user_id (external_user_id) UNIQUE @@ -44,12 +42,9 @@ # # Foreign Keys # -# fk_rails_... (account_group_id => account_groups.id) # fk_rails_... (account_id => accounts.id) # class User < ApplicationRecord - include AccountGroupValidation - ROLES = [ ADMIN_ROLE = 'admin' ].freeze @@ -63,7 +58,6 @@ class User < ApplicationRecord has_one_attached :initials belongs_to :account, optional: true - belongs_to :account_group, optional: true has_one :access_token, dependent: :destroy has_many :access_tokens, dependent: :destroy @@ -95,23 +89,11 @@ class User < ApplicationRecord ) end - def self.find_or_create_by_external_group_id(account_group, external_id, attributes = {}) - account_group.users.find_by(external_user_id: external_id) || - account_group.users.create!( - attributes.merge( - external_user_id: external_id, - password: SecureRandom.hex(16) - ) - ) - end - def access_token super || build_access_token.tap(&:save!) end def active_for_authentication? - return false unless account.present? || account_group.present? - super && !archived_at? && !account&.archived_at? end diff --git a/app/services/external_auth_service.rb b/app/services/external_auth_service.rb index 569f85ca..7966b6e6 100644 --- a/app/services/external_auth_service.rb +++ b/app/services/external_auth_service.rb @@ -8,10 +8,10 @@ class ExternalAuthService def authenticate_user user = if @params[:account].present? find_or_create_user_with_account - elsif @params[:account_group].present? - find_or_create_user_with_account_group + elsif @params[:partnership].present? + find_or_create_user_with_partnership else - raise ArgumentError, 'Either account or account_group params must be provided' + raise ArgumentError, 'Either account or partnership params must be provided' end user.access_token.token @@ -22,9 +22,11 @@ class ExternalAuthService def find_or_create_user_with_account account = Account.find_or_create_by_external_id( @params[:account][:external_id]&.to_i, - name: @params[:account][:name], - locale: @params[:account][:locale] || 'en-US', - timezone: @params[:account][:timezone] || 'UTC' + @params[:account][:name], + { + locale: @params[:account][:locale] || 'en-US', + timezone: @params[:account][:timezone] || 'UTC' + } ) User.find_or_create_by_external_id( @@ -34,16 +36,31 @@ class ExternalAuthService ) end - def find_or_create_user_with_account_group - account_group = AccountGroup.find_or_create_by_external_id( - @params[:account_group][:external_id], - name: @params[:account_group][:name] - ) + def find_or_create_user_with_partnership + # Ensure partnerships exist in DocuSeal before creating the user + # We need these partnerships to exist for templates and authorization to work + ensure_partnerships_exist - User.find_or_create_by_external_group_id( - account_group, - @params[:user][:external_id]&.to_i, - user_attributes + # For partnership users, we don't store any partnership relationship + # They get authorized via API request context (accessible_partnership_ids) + # Just ensure the user exists in DocuSeal for authentication + User.find_by(external_user_id: @params[:user][:external_id]&.to_i) || + User.create!( + user_attributes.merge( + external_user_id: @params[:user][:external_id]&.to_i, + password: SecureRandom.hex(16) + # NOTE: No account_id or partnership_id - authorization comes from API context + ) + ) + end + + def ensure_partnerships_exist + # Create the partnership if it doesn't exist in DocuSeal + return if @params[:partnership].blank? + + Partnership.find_or_create_by_external_id( + @params[:partnership][:external_id], + @params[:partnership][:name] ) end diff --git a/app/services/template_service.rb b/app/services/template_service.rb index a91b967b..32faa009 100644 --- a/app/services/template_service.rb +++ b/app/services/template_service.rb @@ -8,9 +8,18 @@ class TemplateService end def assign_ownership - if @user.account_group.present? - @template.account_group = @user.account_group - @template.folder = @user.account_group.default_template_folder + if @params[:external_partnership_id].present? + partnership = Partnership.find_by(external_partnership_id: @params[:external_partnership_id]) + raise ArgumentError, "Partnership not found: #{@params[:external_partnership_id]}" unless partnership + + @template.partnership = partnership + @template.folder = TemplateFolders.find_or_create_by_name(@user, @params[:folder_name], partnership: partnership) + elsif @params[:external_account_id].present? + account = Account.find_by(external_account_id: @params[:external_account_id]) + raise ArgumentError, "Account not found: #{@params[:external_account_id]}" unless account + + @template.account = account + @template.folder = TemplateFolders.find_or_create_by_name(@user, @params[:folder_name]) elsif @user.account.present? @template.account = @user.account @template.folder = TemplateFolders.find_or_create_by_name(@user, @params[:folder_name]) diff --git a/db/migrate/20250924174100_rename_account_groups_to_partnerships.rb b/db/migrate/20250924174100_rename_account_groups_to_partnerships.rb new file mode 100644 index 00000000..bb43cde6 --- /dev/null +++ b/db/migrate/20250924174100_rename_account_groups_to_partnerships.rb @@ -0,0 +1,18 @@ +class RenameAccountGroupsToPartnerships < ActiveRecord::Migration[8.0] + def change + # Rename the table + rename_table :account_groups, :partnerships + + # Rename the foreign key columns in other tables + rename_column :templates, :account_group_id, :partnership_id + rename_column :template_folders, :account_group_id, :partnership_id + rename_column :export_locations, :global_account_group_id, :global_partnership_id + + # Remove partnership relationships since both users and accounts use API context now + remove_column :users, :account_group_id, :bigint + remove_column :accounts, :account_group_id, :bigint + + # Rename the external ID column to match new naming + rename_column :partnerships, :external_account_group_id, :external_partnership_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 626e9dac..427e4c40 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[8.0].define(version: 2025_09_10_191227) do +ActiveRecord::Schema[8.0].define(version: 2025_09_24_174100) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "pg_catalog.plpgsql" @@ -43,14 +43,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.index ["account_id"], name: "index_account_configs_on_account_id" end - create_table "account_groups", force: :cascade do |t| - t.integer "external_account_group_id", null: false - t.string "name", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["external_account_group_id"], name: "index_account_groups_on_external_account_group_id", unique: true - end - create_table "account_linked_accounts", force: :cascade do |t| t.integer "account_id", null: false t.integer "linked_account_id", null: false @@ -71,8 +63,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.string "uuid", null: false t.datetime "archived_at" t.integer "external_account_id" - t.bigint "account_group_id" - t.index ["account_group_id"], name: "index_accounts_on_account_group_id" t.index ["external_account_id"], name: "index_accounts_on_external_account_id", unique: true t.index ["uuid"], name: "index_accounts_on_uuid", unique: true end @@ -238,6 +228,7 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.datetime "updated_at", null: false t.jsonb "extra_params", default: {}, null: false t.string "submissions_endpoint" + t.integer "global_partnership_id" end create_table "oauth_access_grants", force: :cascade do |t| @@ -282,6 +273,14 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.index ["uid"], name: "index_oauth_applications_on_uid", unique: true end + create_table "partnerships", force: :cascade do |t| + t.integer "external_partnership_id", null: false + t.string "name", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["external_partnership_id"], name: "index_partnerships_on_external_partnership_id", unique: true + end + create_table "search_entries", force: :cascade do |t| t.string "record_type", null: false t.bigint "record_id", null: false @@ -377,10 +376,10 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.datetime "archived_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.bigint "account_group_id" - t.index ["account_group_id"], name: "index_template_folders_on_account_group_id" + t.bigint "partnership_id" t.index ["account_id"], name: "index_template_folders_on_account_id" t.index ["author_id"], name: "index_template_folders_on_author_id" + t.index ["partnership_id"], name: "index_template_folders_on_partnership_id" end create_table "template_sharings", force: :cascade do |t| @@ -410,14 +409,14 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.text "preferences", null: false t.boolean "shared_link", default: false, null: false t.text "external_data_fields" - t.bigint "account_group_id" - t.index ["account_group_id"], name: "index_templates_on_account_group_id" + t.bigint "partnership_id" t.index ["account_id", "folder_id", "id"], name: "index_templates_on_account_id_and_folder_id_and_id", where: "(archived_at IS NULL)" t.index ["account_id", "id"], name: "index_templates_on_account_id_and_id_archived", where: "(archived_at IS NOT NULL)" t.index ["account_id"], name: "index_templates_on_account_id" t.index ["author_id"], name: "index_templates_on_author_id" t.index ["external_id"], name: "index_templates_on_external_id" t.index ["folder_id"], name: "index_templates_on_folder_id" + t.index ["partnership_id"], name: "index_templates_on_partnership_id" t.index ["slug"], name: "index_templates_on_slug", unique: true end @@ -457,8 +456,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do t.integer "consumed_timestep" t.boolean "otp_required_for_login", default: false, null: false t.integer "external_user_id" - t.bigint "account_group_id" - t.index ["account_group_id"], name: "index_users_on_account_group_id" t.index ["account_id"], name: "index_users_on_account_id" t.index ["email"], name: "index_users_on_email", unique: true t.index ["external_user_id"], name: "index_users_on_external_user_id", unique: true @@ -484,7 +481,6 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do add_foreign_key "account_configs", "accounts" add_foreign_key "account_linked_accounts", "accounts" add_foreign_key "account_linked_accounts", "accounts", column: "linked_account_id" - add_foreign_key "accounts", "account_groups" 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 "document_generation_events", "submitters" @@ -503,16 +499,15 @@ ActiveRecord::Schema[8.0].define(version: 2025_09_10_191227) do add_foreign_key "submissions", "users", column: "created_by_user_id" add_foreign_key "submitters", "submissions" add_foreign_key "template_accesses", "templates" - add_foreign_key "template_folders", "account_groups" add_foreign_key "template_folders", "accounts" + add_foreign_key "template_folders", "partnerships" add_foreign_key "template_folders", "users", column: "author_id" add_foreign_key "template_sharings", "templates" - add_foreign_key "templates", "account_groups" add_foreign_key "templates", "accounts" + add_foreign_key "templates", "partnerships" add_foreign_key "templates", "template_folders", column: "folder_id" add_foreign_key "templates", "users", column: "author_id" add_foreign_key "user_configs", "users" - add_foreign_key "users", "account_groups" add_foreign_key "users", "accounts" add_foreign_key "webhook_urls", "accounts" end diff --git a/lib/abilities/template_conditions.rb b/lib/abilities/template_conditions.rb index 6fa54ba1..01d965e8 100644 --- a/lib/abilities/template_conditions.rb +++ b/lib/abilities/template_conditions.rb @@ -4,7 +4,12 @@ module Abilities module TemplateConditions module_function - def collection(user, ability: nil) + def collection(user, ability: nil, request_context: nil) + # Handle partnership context first + if request_context && request_context[:accessible_partnership_ids].present? + return partnership_templates(request_context) + end + if user.account_id.present? templates = Template.where(account_id: user.account_id) @@ -15,18 +20,31 @@ module Abilities .select(:template_id) Template.where(Template.arel_table[:id].in(Arel::Nodes::Union.new(templates.select(:id).arel, shared_ids.arel))) - elsif user.account_group_id.present? - Template.where(account_group_id: user.account_group_id) else + # Partnership users and accounts don't have stored relationships + # Authorization happens at controller level via request context Template.none end end - def entity(template, user:, ability: nil) - return true if template.account_id.blank? && template.account_group_id.blank? + def partnership_templates(request_context) + accessible_partnership_ids = request_context[:accessible_partnership_ids] || [] + + partnership_ids = Partnership.where(external_partnership_id: accessible_partnership_ids).pluck(:id) + + Template.where(partnership_id: partnership_ids) + end + + def entity(template, user:, ability: nil, request_context: nil) + return true if template.account_id.blank? && template.partnership_id.blank? + + # Check request context first (from API params) + if request_context && request_context[:accessible_partnership_ids].present? + return authorize_via_partnership_context(template, request_context) + end - # Handle account group templates - return template.account_group_id == user.account_group_id if template.account_group_id.present? + # Handle partnership templates - users don't have stored relationships anymore + # This should not be reached for partnership users since they use API context # Handle regular account templates return true if template.account_id == user.account_id @@ -39,5 +57,24 @@ module Abilities e.account_id.in?(account_ids) && (ability.nil? || e.ability == 'manage' || e.ability == ability) end end + + def authorize_via_partnership_context(template, request_context) + accessible_partnership_ids = request_context[:accessible_partnership_ids] || [] + + # Handle partnership templates - check if user has access to the partnership + if template.partnership_id.present? + partnership = Partnership.find_by(id: template.partnership_id) + return false unless partnership + + return accessible_partnership_ids.include?(partnership.external_partnership_id) + end + + # Handle account templates - check if user has access via partnership context + if template.account_id.present? + return accessible_partnership_ids.any? && request_context[:external_account_id].present? + end + + false + end end end diff --git a/lib/template_folders.rb b/lib/template_folders.rb index 9a3738ea..0e07c0e7 100644 --- a/lib/template_folders.rb +++ b/lib/template_folders.rb @@ -9,9 +9,17 @@ module TemplateFolders folders.where(TemplateFolder.arel_table[:name].lower.matches("%#{keyword.downcase}%")) end - def find_or_create_by_name(author, name) - return author.account.default_template_folder if name.blank? || name == TemplateFolder::DEFAULT_NAME + def find_or_create_by_name(author, name, partnership: nil) + return default_folder(author, partnership) if name.blank? || name == TemplateFolder::DEFAULT_NAME - author.account.template_folders.create_with(author:, account: author.account).find_or_create_by(name:) + if partnership.present? + partnership.template_folders.create_with(author:, partnership:).find_or_create_by(name:) + else + author.account.template_folders.create_with(author:, account: author.account).find_or_create_by(name:) + end + end + + def default_folder(author, partnership) + partnership.present? ? partnership.default_template_folder(author) : author.account.default_template_folder end end diff --git a/spec/factories/account_groups.rb b/spec/factories/account_groups.rb deleted file mode 100644 index 4c34d847..00000000 --- a/spec/factories/account_groups.rb +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :account_group do - external_account_group_id { Faker::Number.unique.number(digits: 8) } - name { Faker::Company.name } - end -end diff --git a/spec/factories/partnerships.rb b/spec/factories/partnerships.rb new file mode 100644 index 00000000..3c58ce6d --- /dev/null +++ b/spec/factories/partnerships.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :partnership do + external_partnership_id { Faker::Number.unique.number(digits: 8) } + name { Faker::Company.name } + end +end diff --git a/spec/models/account_group_spec.rb b/spec/models/account_group_spec.rb deleted file mode 100644 index bc291c26..00000000 --- a/spec/models/account_group_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -# == Schema Information -# -# Table name: account_groups -# -# id :bigint not null, primary key -# name :string not null -# created_at :datetime not null -# updated_at :datetime not null -# external_account_group_id :integer not null -# -# Indexes -# -# index_account_groups_on_external_account_group_id (external_account_group_id) UNIQUE -# -describe AccountGroup do - let(:account_group) { create(:account_group) } - - describe 'associations' do - it 'has many accounts' do - expect(account_group).to respond_to(:accounts) - end - end - - describe 'validations' do - it 'validates presence of external_account_group_id' do - account_group = build(:account_group, external_account_group_id: nil) - expect(account_group).not_to be_valid - expect(account_group.errors[:external_account_group_id]).to include("can't be blank") - end - - it 'validates uniqueness of external_account_group_id' do - create(:account_group, external_account_group_id: 123) - duplicate = build(:account_group, external_account_group_id: 123) - expect(duplicate).not_to be_valid - expect(duplicate.errors[:external_account_group_id]).to include('has already been taken') - end - - it 'validates presence of name' do - account_group = build(:account_group, name: nil) - expect(account_group).not_to be_valid - expect(account_group.errors[:name]).to include("can't be blank") - end - end - - describe 'when account group is destroyed' do - it 'nullifies accounts account_group_id' do - account = create(:account, account_group: account_group) - - account_group.destroy - - expect(account.reload.account_group).to be_nil - end - end -end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index f9986e52..20f0296a 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -12,19 +12,13 @@ # uuid :string not null # created_at :datetime not null # updated_at :datetime not null -# account_group_id :bigint # external_account_id :integer # # Indexes # -# index_accounts_on_account_group_id (account_group_id) # index_accounts_on_external_account_id (external_account_id) UNIQUE # index_accounts_on_uuid (uuid) UNIQUE # -# Foreign Keys -# -# fk_rails_... (account_group_id => account_groups.id) -# require 'rails_helper' RSpec.describe Account do @@ -43,16 +37,16 @@ RSpec.describe Account do describe '.find_or_create_by_external_id' do let(:external_id) { 123 } - let(:attributes) { { name: 'Test Account' } } + let(:name) { 'Test Account' } it 'finds existing account by external_account_id' do existing_account = create(:account, external_account_id: external_id) - result = described_class.find_or_create_by_external_id(external_id, attributes) + result = described_class.find_or_create_by_external_id(external_id, name) expect(result).to eq(existing_account) end it 'creates new account when none exists' do - result = described_class.find_or_create_by_external_id(external_id, attributes) + result = described_class.find_or_create_by_external_id(external_id, name) expect(result.external_account_id).to eq(external_id) expect(result.name).to eq('Test Account') end @@ -69,17 +63,6 @@ RSpec.describe Account do end end - describe 'account_group association' do - it 'belongs to account_group optionally' do - account = create(:account) - expect(account.account_group).to be_nil - - account_group = create(:account_group) - account.update!(account_group: account_group) - expect(account.reload.account_group).to eq(account_group) - end - end - describe '#default_template_folder' do it 'creates default folder when none exists' do account = create(:account) diff --git a/spec/models/concerns/account_group_validation_spec.rb b/spec/models/concerns/account_group_validation_spec.rb deleted file mode 100644 index e0d74e1d..00000000 --- a/spec/models/concerns/account_group_validation_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe AccountGroupValidation do - # Test with User model since it includes the concern - describe 'validation' do - context 'with account only' do - it 'is valid' do - user = build(:user, account: create(:account), account_group: nil) - expect(user).to be_valid - end - end - - context 'with account_group only' do - it 'is valid' do - user = build(:user, account: nil, account_group: create(:account_group)) - expect(user).to be_valid - end - end - - context 'with neither account nor account_group' do - it 'is invalid' do - user = build(:user, account: nil, account_group: nil) - expect(user).not_to be_valid - expect(user.errors[:base]).to include('Must belong to either an account or account group') - end - end - - context 'with both account and account_group' do - it 'is invalid' do - user = build(:user, account: create(:account), account_group: create(:account_group)) - expect(user).not_to be_valid - expect(user.errors[:base]).to include('Cannot belong to both account and account group') - end - end - end -end diff --git a/spec/models/concerns/partnership_validation_spec.rb b/spec/models/concerns/partnership_validation_spec.rb new file mode 100644 index 00000000..4a35cec2 --- /dev/null +++ b/spec/models/concerns/partnership_validation_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe PartnershipValidation do + # Test with User model since it includes the concern + describe 'validation' do + context 'with account only' do + it 'is valid' do + user = build(:user, account: create(:account)) + expect(user).to be_valid + end + end + end +end diff --git a/spec/models/partnership_spec.rb b/spec/models/partnership_spec.rb new file mode 100644 index 00000000..00bfa3f3 --- /dev/null +++ b/spec/models/partnership_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: partnerships +# +# id :bigint not null, primary key +# name :string not null +# created_at :datetime not null +# updated_at :datetime not null +# external_partnership_id :integer not null +# +# Indexes +# +# index_partnerships_on_external_partnership_id (external_partnership_id) UNIQUE +# +describe Partnership do + let(:partnership) { create(:partnership) } + + describe 'validations' do + it 'validates presence of external_partnership_id' do + partnership = build(:partnership, external_partnership_id: nil) + expect(partnership).not_to be_valid + expect(partnership.errors[:external_partnership_id]).to include("can't be blank") + end + + it 'validates uniqueness of external_partnership_id' do + create(:partnership, external_partnership_id: 123) + duplicate = build(:partnership, external_partnership_id: 123) + expect(duplicate).not_to be_valid + expect(duplicate.errors[:external_partnership_id]).to include('has already been taken') + end + + it 'validates presence of name' do + partnership = build(:partnership, name: nil) + expect(partnership).not_to be_valid + expect(partnership.errors[:name]).to include("can't be blank") + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 2fe25f43..cf856950 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -28,13 +28,11 @@ # uuid :string not null # created_at :datetime not null # updated_at :datetime not null -# account_group_id :bigint # account_id :integer # external_user_id :integer # # Indexes # -# index_users_on_account_group_id (account_group_id) # index_users_on_account_id (account_id) # index_users_on_email (email) UNIQUE # index_users_on_external_user_id (external_user_id) UNIQUE @@ -44,7 +42,6 @@ # # Foreign Keys # -# fk_rails_... (account_group_id => account_groups.id) # fk_rails_... (account_id => accounts.id) # require 'rails_helper' @@ -143,26 +140,4 @@ RSpec.describe User do expect(user.friendly_name).to eq('john@example.com') end end - - describe '.find_or_create_by_external_group_id' do - let(:account_group) { create(:account_group) } - let(:attributes) { { email: 'test@example.com', first_name: 'John' } } - - it 'finds existing user by external_user_id and account_group' do - existing_user = create(:user, account: nil, account_group: account_group, external_user_id: 123) - - result = described_class.find_or_create_by_external_group_id(account_group, 123, attributes) - - expect(result).to eq(existing_user) - end - - it 'creates new user when not found' do - result = described_class.find_or_create_by_external_group_id(account_group, 456, attributes) - - expect(result.account_group).to eq(account_group) - expect(result.external_user_id).to eq(456) - expect(result.email).to eq('test@example.com') - expect(result.password).to be_present - end - end end diff --git a/spec/services/external_auth_service_spec.rb b/spec/services/external_auth_service_spec.rb index b1422177..3141d017 100644 --- a/spec/services/external_auth_service_spec.rb +++ b/spec/services/external_auth_service_spec.rb @@ -16,7 +16,9 @@ RSpec.describe ExternalAuthService do context 'with account params' do let(:params) do { - account: { external_id: 456, name: 'Test Account' }, + account: { + external_id: '456', name: 'Test Account', locale: 'en-US', timezone: 'UTC', entity_type: 'Account' + }, user: user_params } end @@ -39,29 +41,31 @@ RSpec.describe ExternalAuthService do end end - context 'with account_group params' do + context 'with partnership params' do let(:params) do { - account_group: { external_id: 789, name: 'Test Group' }, + partnership: { + external_id: '789', name: 'Test Group', locale: 'en-US', timezone: 'UTC', entity_type: 'Partnership' + }, user: user_params } end - it 'returns access token for new account_group and user' do + it 'returns access token for new partnership and user' do token = described_class.new(params).authenticate_user expect(token).to be_present - expect(AccountGroup.last.external_account_group_id).to eq(789) + expect(Partnership.last.external_partnership_id).to eq(789) expect(User.last.external_user_id).to eq(123) end end context 'with invalid params' do - it 'raises error when neither account nor account_group provided' do + it 'raises error when neither account nor partnership provided' do params = { user: user_params } expect { described_class.new(params).authenticate_user } - .to raise_error(ArgumentError, 'Either account or account_group params must be provided') + .to raise_error(ArgumentError, 'Either account or partnership params must be provided') end end end diff --git a/spec/services/template_service_spec.rb b/spec/services/template_service_spec.rb index 141ac9c5..dd0af1f4 100644 --- a/spec/services/template_service_spec.rb +++ b/spec/services/template_service_spec.rb @@ -4,25 +4,27 @@ require 'rails_helper' RSpec.describe TemplateService do describe '#assign_ownership' do - let(:template) { build(:template, account: nil, account_group: nil) } + let(:template) { build(:template, account: nil, partnership: nil) } let(:params) { { folder_name: 'Custom Folder' } } - context 'with account_group user' do - let(:account_group) { create(:account_group) } - let(:user) { create(:user, account: nil, account_group: account_group) } + context 'with partnership user' do + let(:partnership) { create(:partnership) } + let(:user) { create(:user, account: nil) } + let(:params) { { folder_name: 'Custom Folder', external_partnership_id: partnership.external_partnership_id } } - it 'assigns account_group and default folder' do + it 'assigns partnership and creates custom folder' do service = described_class.new(template, user, params) service.assign_ownership - expect(template.account_group).to eq(account_group) - expect(template.folder).to eq(account_group.default_template_folder) + expect(template.partnership).to eq(partnership) + expect(template.folder.name).to eq('Custom Folder') + expect(template.folder.partnership).to eq(partnership) end end context 'with account user' do let(:account) { create(:account) } - let(:user) { create(:user, account: account, account_group: nil) } + let(:user) { create(:user, account: account) } it 'assigns account and finds/creates folder' do service = described_class.new(template, user, params) @@ -33,15 +35,15 @@ RSpec.describe TemplateService do end end - context 'with user having neither account nor account_group' do - let(:user) { build(:user, account: nil, account_group: nil) } + context 'with user having neither account nor partnership' do + let(:user) { build(:user, account: nil) } it 'does not assign ownership' do service = described_class.new(template, user, params) service.assign_ownership expect(template.account).to be_nil - expect(template.account_group).to be_nil + expect(template.partnership).to be_nil end end end