diff --git a/packages/twenty-front/src/modules/settings/roles/role-settings/components/SettingsRoleSettingsDeleteRoleConfirmationModalSubtitle.tsx b/packages/twenty-front/src/modules/settings/roles/role-settings/components/SettingsRoleSettingsDeleteRoleConfirmationModalSubtitle.tsx index ffa1b37fc0..2b07bd5fa4 100644 --- a/packages/twenty-front/src/modules/settings/roles/role-settings/components/SettingsRoleSettingsDeleteRoleConfirmationModalSubtitle.tsx +++ b/packages/twenty-front/src/modules/settings/roles/role-settings/components/SettingsRoleSettingsDeleteRoleConfirmationModalSubtitle.tsx @@ -1,6 +1,6 @@ import { settingsDraftRoleFamilyState } from '@/settings/roles/states/settingsDraftRoleFamilyState'; import { useAtomFamilyStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomFamilyStateValue'; -import { t } from '@lingui/core/macro'; +import { plural, t } from '@lingui/core/macro'; type SettingsRoleSettingsDeleteRoleConfirmationModalSubtitleProps = { roleId: string; @@ -14,8 +14,51 @@ export const SettingsRoleSettingsDeleteRoleConfirmationModalSubtitle = ({ roleId, ); const roleName = settingsDraftRole.label; + const memberCount = settingsDraftRole.workspaceMembers.length; + const apiKeyCount = settingsDraftRole.apiKeys.length; + const agentCount = settingsDraftRole.agents.length; + + const segments: string[] = []; + + if (memberCount > 0) { + segments.push( + plural(memberCount, { + one: `${memberCount} member`, + other: `${memberCount} members`, + }), + ); + } + + if (apiKeyCount > 0) { + segments.push( + plural(apiKeyCount, { + one: `${apiKeyCount} API key`, + other: `${apiKeyCount} API keys`, + }), + ); + } + + if (agentCount > 0) { + segments.push( + plural(agentCount, { + one: `${agentCount} agent`, + other: `${agentCount} agents`, + }), + ); + } + + if (segments.length === 0) { + return ( + <>{t`Confirm deletion of ${roleName} role? This cannot be undone.`} + ); + } + + const reassignSubject = + segments.length === 1 + ? segments[0] + : `${segments.slice(0, -1).join(', ')} ${t`and`} ${segments.at(-1)}`; return ( - <>{t`Confirm deletion of ${roleName} role? This cannot be undone. All members will be reassigned to the default role.`} + <>{t`Confirm deletion of ${roleName} role? This cannot be undone. ${reassignSubject} will be reassigned to the default role.`} ); }; diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts index ec1a3c3693..6f4fd477a1 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts @@ -48,6 +48,8 @@ export enum PermissionsExceptionCode { COMPOSITE_TYPE_NOT_FOUND = 'COMPOSITE_TYPE_NOT_FOUND', ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET = 'ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET', ROLE_CANNOT_BE_ASSIGNED_TO_USERS = 'ROLE_CANNOT_BE_ASSIGNED_TO_USERS', + ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS = 'ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS', + ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS = 'ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS', APPLICATION_ROLE_NOT_FOUND = 'APPLICATION_ROLE_NOT_FOUND', ROLE_BELONGS_TO_ANOTHER_APPLICATION = 'ROLE_BELONGS_TO_ANOTHER_APPLICATION', } @@ -142,6 +144,10 @@ const getPermissionsExceptionUserFriendlyMessage = ( return msg`Role must have at least one target.`; case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_USERS: return msg`This role cannot be assigned to users.`; + case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS: + return msg`This role cannot be assigned to API keys.`; + case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS: + return msg`This role cannot be assigned to agents.`; case PermissionsExceptionCode.APPLICATION_ROLE_NOT_FOUND: return msg`No role assigned to the application.`; case PermissionsExceptionCode.ROLE_BELONGS_TO_ANOTHER_APPLICATION: diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts index e03148fbe7..a53069f69a 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts @@ -44,6 +44,8 @@ export const permissionGraphqlApiExceptionHandler = ( case PermissionsExceptionCode.EMPTY_FIELD_PERMISSION_NOT_ALLOWED: case PermissionsExceptionCode.ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET: case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_USERS: + case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS: + case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS: throw new UserInputError(error); case PermissionsExceptionCode.ROLE_NOT_FOUND: case PermissionsExceptionCode.OBJECT_METADATA_NOT_FOUND: diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-rest-api-exception-code-to-http-status.util.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-rest-api-exception-code-to-http-status.util.ts index 8f21a3a0a5..b58728a73d 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-rest-api-exception-code-to-http-status.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-rest-api-exception-code-to-http-status.util.ts @@ -26,6 +26,8 @@ export const permissionRestApiExceptionCodeToHttpStatus = ( case PermissionsExceptionCode.EMPTY_FIELD_PERMISSION_NOT_ALLOWED: case PermissionsExceptionCode.ROLE_MUST_HAVE_AT_LEAST_ONE_TARGET: case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_USERS: + case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS: + case PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS: return 400; case PermissionsExceptionCode.ROLE_NOT_FOUND: case PermissionsExceptionCode.OBJECT_METADATA_NOT_FOUND: diff --git a/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts b/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts index 53f045ab21..e86a2481d0 100644 --- a/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts @@ -5,9 +5,19 @@ import { msg } from '@lingui/core/macro'; import { isDefined } from 'twenty-shared/utils'; import { Repository } from 'typeorm'; +import { + ApiKeyException, + ApiKeyExceptionCode, +} from 'src/engine/core-modules/api-key/exceptions/api-key.exception'; +import { ApiKeyRoleService } from 'src/engine/core-modules/api-key/services/api-key-role.service'; import { ApplicationService } from 'src/engine/core-modules/application/application.service'; import { type FlatApplication } from 'src/engine/core-modules/application/types/flat-application.type'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; +import { AiAgentRoleService } from 'src/engine/metadata-modules/ai/ai-agent-role/ai-agent-role.service'; +import { + AiException, + AiExceptionCode, +} from 'src/engine/metadata-modules/ai/ai.exception'; import { WorkspaceManyOrAllFlatEntityMapsCacheService } from 'src/engine/metadata-modules/flat-entity/services/workspace-many-or-all-flat-entity-maps-cache.service'; import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util'; import { findFlatEntityByUniversalIdentifier } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-universal-identifier.util'; @@ -40,6 +50,8 @@ export class RoleService { private readonly roleRepository: Repository, private readonly userRoleService: UserRoleService, private readonly applicationService: ApplicationService, + private readonly apiKeyRoleService: ApiKeyRoleService, + private readonly aiAgentRoleService: AiAgentRoleService, ) {} public async getWorkspaceRoles(workspaceId: string): Promise { @@ -316,8 +328,9 @@ export class RoleService { ); } - await this.assignDefaultRoleToMembersWithRoleToDelete({ + await this.rebindTargetsOfRoleToDeleteToDefaultRole({ roleId, + roleLabel: flatRoleToDelete.label, workspaceId, defaultRoleId, }); @@ -407,12 +420,14 @@ export class RoleService { } // TODO: Move to migration side effect / To address for rollback of role deletion - private async assignDefaultRoleToMembersWithRoleToDelete({ + private async rebindTargetsOfRoleToDeleteToDefaultRole({ roleId, + roleLabel, workspaceId, defaultRoleId, }: { roleId: string; + roleLabel: string; workspaceId: string; defaultRoleId: string; }): Promise { @@ -427,5 +442,82 @@ export class RoleService { roleId: defaultRoleId, workspaceId, }); + + const apiKeysToRebind = + await this.apiKeyRoleService.getApiKeysAssignedToRole( + roleId, + workspaceId, + ); + + for (const apiKey of apiKeysToRebind) { + try { + await this.apiKeyRoleService.assignRoleToApiKey({ + apiKeyId: apiKey.id, + roleId: defaultRoleId, + workspaceId, + }); + } catch (error) { + if ( + error instanceof ApiKeyException && + error.code === ApiKeyExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS + ) { + throw this.toRoleDeleteRebindException({ + roleLabel, + targetKind: 'apiKey', + }); + } + throw error; + } + } + + const agentsToRebind = + await this.aiAgentRoleService.getAgentsAssignedToRole( + roleId, + workspaceId, + ); + + for (const agent of agentsToRebind) { + try { + await this.aiAgentRoleService.assignRoleToAgent({ + agentId: agent.id, + roleId: defaultRoleId, + workspaceId, + }); + } catch (error) { + if ( + error instanceof AiException && + error.code === AiExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS + ) { + throw this.toRoleDeleteRebindException({ + roleLabel, + targetKind: 'agent', + }); + } + throw error; + } + } + } + + private toRoleDeleteRebindException({ + roleLabel, + targetKind, + }: { + roleLabel: string; + targetKind: 'apiKey' | 'agent'; + }): Error { + const targetLabel = targetKind === 'apiKey' ? 'API key' : 'agent'; + + return new PermissionsException( + `Cannot delete role "${roleLabel}": the workspace default role cannot be assigned to ${targetLabel}s.`, + targetKind === 'apiKey' + ? PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_API_KEYS + : PermissionsExceptionCode.ROLE_CANNOT_BE_ASSIGNED_TO_AGENTS, + { + userFriendlyMessage: + targetKind === 'apiKey' + ? msg`Cannot delete this role: it is still assigned to one or more API keys, and the workspace default role cannot be assigned to API keys. Please reassign these API keys to another role first.` + : msg`Cannot delete this role: it is still assigned to one or more agents, and the workspace default role cannot be assigned to agents. Please reassign these agents to another role first.`, + }, + ); } } diff --git a/packages/twenty-server/test/integration/metadata/suites/agent/successful-agent-creation.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/agent/successful-agent-creation.integration-spec.ts index 2487634bd2..16e4e234d2 100644 --- a/packages/twenty-server/test/integration/metadata/suites/agent/successful-agent-creation.integration-spec.ts +++ b/packages/twenty-server/test/integration/metadata/suites/agent/successful-agent-creation.integration-spec.ts @@ -208,7 +208,15 @@ describe('Agent creation should succeed', () => { isCustom: true, }); - // Clean up the role + // Delete the agent first so its role_target is removed; otherwise + // deleting the role would refuse to orphan the agent (the workspace + // default role isn't agent-assignable). + await deleteOneAgent({ + expectToFail: false, + input: { id: createdAgentId }, + }); + createdAgentId = ''; + await deleteOneRole({ expectToFail: false, input: { idToDelete: createdRoleId },