fix(server): allow app-defined permission flags to be referenced by a role in the same sync (#21742)

## Context When an application defined custom permission flags and a role referencing them in the same sync, installation failed, first at validation (Permission flag not found) and then at execution (Migration action 'create' for 'rolePermissionFlag' failed). Root cause: both the migration builder order and the runner execution order processed rolePermissionFlag before permissionFlag, so the role's flag assignments were validated/inserted before the flags they reference existed. ## Changes - Builder order: run the permissionFlag builder before rolePermissionFlag so newly created flags are visible in the optimistic maps when assignments are validated. - Execution order: order the permission-flag actions so definitions are created before assignments, and assignments deleted before definitions, keeping the FK satisfied in both directions. - In-use check: move the "flag still assigned to a role" guard out of the per-entity deletion validator (order-dependent, false-positived when a flag and its assignments were deleted together) into a new order-independent validatePermissionFlagNotInUseCrossEntity (aligned with existing validateObjectMetadataCrossEntity, validateViewFieldLabelIdentifierCrossEntity, ...), run after all builders against the migration's final state. This fixes both the create path (define flag + reference it in one sync) and the teardown path (delete flag + its assignments in one sync).  <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21742?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
2026-06-17 17:44:01 +02:00
parent d4e0b78d20
commit 607d9ee6e5
7 changed files with 180 additions and 83 deletions
@@ -0,0 +1,99 @@
+import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant';
+import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
+import { type FlatRolePermissionFlag } from 'src/engine/metadata-modules/flat-role-permission-flag/types/flat-role-permission-flag.type';
+import { validatePermissionFlagNotInUseCrossEntity } from 'src/engine/metadata-modules/flat-permission-flag/validators/utils/validate-permission-flag-not-in-use-cross-entity.util';
+import { PermissionFlagExceptionCode } from 'src/engine/metadata-modules/permission-flag/permission-flag.exception';
+
+const FLAG_UNIVERSAL_IDENTIFIER = '00000000-0000-0000-0000-000000000001';
+
+const buildRolePermissionFlag = (
+  overrides: Partial<FlatRolePermissionFlag> = {},
+): FlatRolePermissionFlag =>
+  ({
+    id: '00000000-0000-0000-0000-000000000101',
+    universalIdentifier: '00000000-0000-0000-0000-000000000101',
+    permissionFlagId: FLAG_UNIVERSAL_IDENTIFIER,
+    permissionFlagUniversalIdentifier: FLAG_UNIVERSAL_IDENTIFIER,
+    roleUniversalIdentifier: '00000000-0000-0000-0000-000000000201',
+    workspaceId: 'workspace-id',
+    applicationId: '00000000-0000-0000-0000-000000000aaa',
+    applicationUniversalIdentifier: '00000000-0000-0000-0000-000000000aaa',
+    createdAt: new Date().toISOString(),
+    updatedAt: new Date().toISOString(),
+    ...overrides,
+  }) as unknown as FlatRolePermissionFlag;
+
+const buildRolePermissionFlagMaps = (
+  rolePermissionFlags: FlatRolePermissionFlag[] = [],
+): FlatEntityMaps<FlatRolePermissionFlag> => {
+  const maps =
+    createEmptyFlatEntityMaps() as unknown as FlatEntityMaps<FlatRolePermissionFlag>;
+
+  for (const rolePermissionFlag of rolePermissionFlags) {
+    maps.byUniversalIdentifier[rolePermissionFlag.universalIdentifier] =
+      rolePermissionFlag;
+  }
+
+  return maps;
+};
+
+const buildArgs = (args: {
+  rolePermissionFlags: FlatRolePermissionFlag[];
+  deletedFlagUniversalIdentifiers: string[];
+}) =>
+  ({
+    optimisticUniversalFlatMaps: {
+      flatRolePermissionFlagMaps: buildRolePermissionFlagMaps(
+        args.rolePermissionFlags,
+      ),
+    },
+    deletedPermissionFlagActions: args.deletedFlagUniversalIdentifiers.map(
+      (universalIdentifier) => ({
+        type: 'delete',
+        metadataName: 'permissionFlag',
+        universalIdentifier,
+        flatEntity: { universalIdentifier, key: 'MANAGE_INVOICES' },
+      }),
+    ),
+  }) as unknown as Parameters<
+    typeof validatePermissionFlagNotInUseCrossEntity
+  >[0];
+
+describe('validatePermissionFlagNotInUseCrossEntity', () => {
+  it('returns no error when no permission flag is being deleted', () => {
+    const result = validatePermissionFlagNotInUseCrossEntity(
+      buildArgs({
+        rolePermissionFlags: [buildRolePermissionFlag()],
+        deletedFlagUniversalIdentifiers: [],
+      }),
+    );
+
+    expect(result.permissionFlag).toHaveLength(0);
+  });
+
+  it('rejects deleting a flag still referenced by a role in the target state', () => {
+    const result = validatePermissionFlagNotInUseCrossEntity(
+      buildArgs({
+        rolePermissionFlags: [buildRolePermissionFlag()],
+        deletedFlagUniversalIdentifiers: [FLAG_UNIVERSAL_IDENTIFIER],
+      }),
+    );
+
+    expect(
+      result.permissionFlag.flatMap((failure) =>
+        failure.errors.map((error) => error.code),
+      ),
+    ).toEqual([PermissionFlagExceptionCode.PERMISSION_FLAG_IN_USE]);
+  });
+
+  it('allows deleting a flag whose assignments are deleted in the same migration', () => {
+    const result = validatePermissionFlagNotInUseCrossEntity(
+      buildArgs({
+        rolePermissionFlags: [],
+        deletedFlagUniversalIdentifiers: [FLAG_UNIVERSAL_IDENTIFIER],
+      }),
+    );
+
+    expect(result.permissionFlag).toHaveLength(0);
+  });
+});
@@ -0,0 +1,66 @@
+import { msg, t } from '@lingui/core/macro';
+import { isDefined } from 'twenty-shared/utils';
+
+import { PermissionFlagExceptionCode } from 'src/engine/metadata-modules/permission-flag/permission-flag.exception';
+import { type OrchestratorFailureReport } from 'src/engine/workspace-manager/workspace-migration/types/workspace-migration-orchestrator.type';
+import { type AllUniversalFlatEntityMaps } from 'src/engine/workspace-manager/workspace-migration/universal-flat-entity/types/all-universal-flat-entity-maps.type';
+import { type UniversalDeletePermissionFlagAction } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/permission-flag/types/workspace-migration-permission-flag-action.type';
+import { getEmptyFlatEntityValidationError } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/builders/utils/get-flat-entity-validation-error.util';
+
+export const validatePermissionFlagNotInUseCrossEntity = ({
+  optimisticUniversalFlatMaps,
+  deletedPermissionFlagActions,
+}: {
+  optimisticUniversalFlatMaps: Pick<
+    AllUniversalFlatEntityMaps,
+    'flatRolePermissionFlagMaps'
+  >;
+  deletedPermissionFlagActions: UniversalDeletePermissionFlagAction[];
+}): Pick<OrchestratorFailureReport, 'permissionFlag'> => {
+  const validationErrors: Pick<OrchestratorFailureReport, 'permissionFlag'> = {
+    permissionFlag: [],
+  };
+
+  if (deletedPermissionFlagActions.length === 0) {
+    return validationErrors;
+  }
+
+  const survivingRolePermissionFlags = Object.values(
+    optimisticUniversalFlatMaps.flatRolePermissionFlagMaps
+      .byUniversalIdentifier,
+  ).filter(isDefined);
+
+  for (const deleteAction of deletedPermissionFlagActions) {
+    const isStillReferenced = survivingRolePermissionFlags.some(
+      (rolePermissionFlag) =>
+        rolePermissionFlag.permissionFlagUniversalIdentifier ===
+        deleteAction.universalIdentifier,
+    );
+
+    if (!isStillReferenced) {
+      continue;
+    }
+
+    const flagKey =
+      deleteAction.flatEntity?.key ?? deleteAction.universalIdentifier;
+
+    const failedValidation = getEmptyFlatEntityValidationError({
+      flatEntityMinimalInformation: {
+        universalIdentifier: deleteAction.universalIdentifier,
+        key: deleteAction.flatEntity?.key,
+      },
+      metadataName: 'permissionFlag',
+      type: 'delete',
+    });
+
+    failedValidation.errors.push({
+      code: PermissionFlagExceptionCode.PERMISSION_FLAG_IN_USE,
+      message: t`Permission flag definition with key ${flagKey} is still assigned to a role`,
+      userFriendlyMessage: msg`Remove this permission from all roles before deleting it`,
+    });
+
+    validationErrors.permissionFlag.push(failedValidation);
+  }
+
+  return validationErrors;
+};
@@ -228,14 +228,14 @@ export class WorkspaceMigrationBuildOrchestratorService {
        ALL_METADATA_NAME.fieldPermission,
        workspaceMigrationFieldPermissionActionsBuilderService,
      ),
-      createEntityActionsBuilderTask(
-        ALL_METADATA_NAME.rolePermissionFlag,
-        workspaceMigrationRolePermissionFlagActionsBuilderService,
-      ),
      createEntityActionsBuilderTask(
        ALL_METADATA_NAME.permissionFlag,
        workspaceMigrationPermissionFlagActionsBuilderService,
      ),
+      createEntityActionsBuilderTask(
+        ALL_METADATA_NAME.rolePermissionFlag,
+        workspaceMigrationRolePermissionFlagActionsBuilderService,
+      ),
      createEntityActionsBuilderTask(
        ALL_METADATA_NAME.roleTarget,
        workspaceMigrationRoleTargetActionsBuilderService,
@@ -71,16 +71,13 @@ export const computeOrderedMigrationActions = (
    ...aggregatedOrchestratorActionsReport.fieldPermission.update,
    ///

-    // Permission flags
+    // Permission flag definitions and their role assignments.
    ...aggregatedOrchestratorActionsReport.rolePermissionFlag.delete,
-    ...aggregatedOrchestratorActionsReport.rolePermissionFlag.create,
-    ...aggregatedOrchestratorActionsReport.rolePermissionFlag.update,
-    ///
-
-    // Permission flag definitions
    ...aggregatedOrchestratorActionsReport.permissionFlag.delete,
    ...aggregatedOrchestratorActionsReport.permissionFlag.create,
+    ...aggregatedOrchestratorActionsReport.rolePermissionFlag.create,
    ...aggregatedOrchestratorActionsReport.permissionFlag.update,
+    ...aggregatedOrchestratorActionsReport.rolePermissionFlag.update,
    ///

    // Agents
@@ -1,4 +1,5 @@
 import { validateObjectMetadataCrossEntity } from 'src/engine/metadata-modules/flat-object-metadata/validators/utils/validate-object-metadata-cross-entity.util';
+import { validatePermissionFlagNotInUseCrossEntity } from 'src/engine/metadata-modules/flat-permission-flag/validators/utils/validate-permission-flag-not-in-use-cross-entity.util';
 import { validateViewFieldLabelIdentifierCrossEntity } from 'src/engine/metadata-modules/flat-view-field/validators/utils/validate-view-field-label-identifier-cross-entity.util';
 import {
  type OrchestratorActionsReport,
@@ -32,8 +33,15 @@ export const crossEntityTransversalValidation = ({
    preDeletionFlatViewFieldMaps,
  });

+  const { permissionFlag } = validatePermissionFlagNotInUseCrossEntity({
+    optimisticUniversalFlatMaps,
+    deletedPermissionFlagActions:
+      orchestratorActionsReport.permissionFlag.delete,
+  });
+
  crossEntityFailureReport.objectMetadata.push(...objectMetadata);
  crossEntityFailureReport.viewField.push(...viewField);
+  crossEntityFailureReport.permissionFlag.push(...permissionFlag);

  validateUniversalIdentifierCrossEntityUniquenessThroughReportMutation({
    optimisticUniversalFlatMaps,
@@ -1,6 +1,5 @@
 import { Test, type TestingModule } from '@nestjs/testing';

-import { PermissionFlagType } from 'twenty-shared/constants';
 import { ALL_METADATA_NAME } from 'twenty-shared/metadata';

 import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant';
@@ -38,23 +37,6 @@ const buildEmptyRolePermissionFlagMaps =
  (): FlatEntityMaps<FlatRolePermissionFlag> =>
    createEmptyFlatEntityMaps() as unknown as FlatEntityMaps<FlatRolePermissionFlag>;

-const buildFlatRolePermissionFlag = (
-  overrides: Partial<FlatRolePermissionFlag> = {},
-): FlatRolePermissionFlag =>
-  ({
-    id: '00000000-0000-0000-0000-000000000101',
-    universalIdentifier: '00000000-0000-0000-0000-000000000101',
-    permissionFlagId: '00000000-0000-0000-0000-000000000001',
-    permissionFlagUniversalIdentifier: '00000000-0000-0000-0000-000000000001',
-    roleUniversalIdentifier: '00000000-0000-0000-0000-000000000201',
-    workspaceId: 'workspace-id',
-    applicationId: '00000000-0000-0000-0000-000000000aaa',
-    applicationUniversalIdentifier: '00000000-0000-0000-0000-000000000aaa',
-    createdAt: new Date().toISOString(),
-    updatedAt: new Date().toISOString(),
-    ...overrides,
-  }) as unknown as FlatRolePermissionFlag;
-
 const buildArgs = (
  flatEntityToValidate: FlatPermissionFlag,
  optimisticMaps: FlatEntityMaps<FlatPermissionFlag> = buildEmptyMaps(),
@@ -335,43 +317,6 @@ describe('FlatPermissionFlagValidatorService', () => {
        PermissionFlagExceptionCode.PERMISSION_FLAG_IS_STANDARD,
      ]);
    });
-
-    it('rejects deleting a definition while roles still grant its key', () => {
-      const existing = buildFlatDefinition({
-        key: PermissionFlagType.WORKSPACE,
-      });
-      const optimisticMaps = buildEmptyMaps();
-      optimisticMaps.byUniversalIdentifier[existing.universalIdentifier] =
-        existing;
-
-      const rolePermissionFlagMaps = buildEmptyRolePermissionFlagMaps();
-      const rolePermissionFlag = buildFlatRolePermissionFlag({
-        permissionFlagId: existing.id,
-        permissionFlagUniversalIdentifier: existing.universalIdentifier,
-      });
-      rolePermissionFlagMaps.byUniversalIdentifier[
-        rolePermissionFlag.universalIdentifier
-      ] = rolePermissionFlag;
-
-      const result = service.validateFlatPermissionFlagDeletion({
-        flatEntityToValidate: existing,
-        optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
-          flatPermissionFlagMaps: optimisticMaps,
-          flatRolePermissionFlagMaps: rolePermissionFlagMaps,
-        },
-        buildOptions: {
-          isSystemBuild: false,
-          applicationUniversalIdentifier:
-            '00000000-0000-0000-0000-000000000aaa',
-        },
-      } as unknown as Parameters<
-        FlatPermissionFlagValidatorService['validateFlatPermissionFlagDeletion']
-      >[0]);
-
-      expect(result.errors.map((error) => error.code)).toEqual([
-        PermissionFlagExceptionCode.PERMISSION_FLAG_IN_USE,
-      ]);
-    });
  });

  // ALL_METADATA_NAME is referenced as a sanity check the metadata name exists
@@ -166,7 +166,6 @@ export class FlatPermissionFlagValidatorService {
    flatEntityToValidate: { universalIdentifier },
    optimisticFlatEntityMapsAndRelatedFlatEntityMaps: {
      flatPermissionFlagMaps: optimisticFlatPermissionFlagMaps,
-      flatRolePermissionFlagMaps: optimisticFlatRolePermissionFlagMaps,
    },
    buildOptions,
  }: UniversalFlatEntityValidationArgs<
@@ -209,23 +208,6 @@ export class FlatPermissionFlagValidatorService {
      });
    }

-    const isPermissionFlagInUse = Object.values(
-      optimisticFlatRolePermissionFlagMaps.byUniversalIdentifier,
-    ).some(
-      (rolePermissionFlag) =>
-        isDefined(rolePermissionFlag) &&
-        rolePermissionFlag.permissionFlagUniversalIdentifier ===
-          existing.universalIdentifier,
-    );
-
-    if (isPermissionFlagInUse) {
-      validationResult.errors.push({
-        code: PermissionFlagExceptionCode.PERMISSION_FLAG_IN_USE,
-        message: t`Permission flag definition with key ${existing.key} is still assigned to a role`,
-        userFriendlyMessage: msg`Remove this permission from all roles before deleting it`,
-      });
-    }
-
    return validationResult;
  }
 }