Migrate field permission to syncable entity#18751
Merged
charlesBochet merged 158 commits intomainfrom Mar 27, 2026
Merged
Conversation
- Introduced `permissionFlag` entity and its configuration across various constants. - Added editable properties for `permissionFlag` and defined its flat entity types. - Updated related metadata structures to include `permissionFlag` in serialization and validation. - Created new types for handling permission flag actions in workspace migration. This update enhances the metadata handling for permission flags, allowing for better integration and management within the system.
…missionFlag table - Introduced a new migration that adds `universalIdentifier` and `applicationId` columns to the `permissionFlag` table. - Created a unique index on the combination of `workspaceId` and `universalIdentifier`. - Established a foreign key constraint for `applicationId` referencing the `application` table. This update enhances the permission flag structure, allowing for better identification and association with applications.
- Introduced `WorkspaceFlatPermissionFlagMapCacheService` for managing permission flag mappings in the workspace cache. - Added utility functions for converting between permission flag entities and flat permission flags. - Updated role and application metadata to include permission flag identifiers. - Enhanced workspace migration utilities to support permission flag actions. This update improves the management and integration of permission flags within the system, facilitating better access control and permissions handling.
- Introduced `WorkspaceMigrationPermissionFlagActionsBuilderService` for handling permission flag actions during workspace migration. - Added `FlatPermissionFlagValidatorService` to validate creation, update, and deletion of permission flags. - Updated `workspace-migration-builder` module to include new services and validators for permission flags. - Enhanced existing migration utilities to support permission flag integration. This update improves the management of permission flags, ensuring robust validation and action handling within the workspace migration process.
- Introduced `CreatePermissionFlagActionHandlerService`, `DeletePermissionFlagActionHandlerService`, and `UpdatePermissionFlagActionHandlerService` to manage permission flag actions during workspace migration. - Implemented methods for transpiling actions and executing database operations for creating, deleting, and updating permission flags. - Enhanced the workspace migration utilities to support the new permission flag action handlers. This update improves the handling of permission flags within the workspace migration process, ensuring robust action management and integration.
- Updated `PermissionFlagModule` to include new dependencies: `ApplicationModule`, `WorkspaceMigrationModule`, and `WorkspaceManyOrAllFlatEntityMapsCacheModule`. - Enhanced `PermissionFlagService` to integrate with `WorkspaceMigrationValidateBuildAndRunService` and `WorkspaceManyOrAllFlatEntityMapsCacheService`, improving permission flag handling during workspace migrations. - Introduced utility function `fromFlatPermissionFlagToPermissionFlagDto` for converting flat permission flags to DTOs. This update streamlines permission flag management and enhances integration with workspace migration processes.
- Introduced two new test files: `failing-permission-flag-upsert.integration-spec.ts` and `successful-permission-flag-upsert.integration-spec.ts`. - The failing tests cover scenarios such as invalid UUIDs, non-existent roles, and invalid permission flag keys. - The successful tests validate the upsert functionality with various permission flag inputs, including empty and multiple flags. - Added utility functions for constructing GraphQL queries related to permission flag upserts. This update enhances the test coverage for permission flag operations, ensuring robust validation of both success and failure cases during upsert operations.
- Cleaned up the `fromPermissionFlagEntityToFlatPermissionFlag` utility by adjusting the formatting of the `universalIdentifier` property assignment for better readability. - Added import for `WorkspaceMigrationViewSortActionsBuilderService` in the `workspace-migration-build-orchestrator.service.ts` to enhance the workspace migration capabilities. This update improves code clarity and integrates additional functionality for workspace migration processes.
- Updated the `useMetadataErrorHandler` hook to include a new `permissionFlag` property in the metadata structure. - This addition enhances the metadata handling capabilities related to permission flags, improving integration with permission management processes. This update aligns with ongoing improvements to permission flag functionalities within the system.
- Created a new snapshot file for integration tests related to the permission flag upsert functionality. - The snapshots cover various failure scenarios, including invalid enum values, non-editable roles, non-existent role IDs, and invalid UUIDs. This addition enhances the test coverage for permission flag operations, ensuring accurate validation of error handling during upsert operations.
- Updated the AllMetadataName type in schema.ts to include 'permissionFlag'. - Added 'permissionFlag' to the GraphQL enum definition in schema.graphql. This change enhances the metadata structure by incorporating permission flags, improving integration with permission management functionalities.
…ssage formatting - Adjusted the formatting of the error message in the Jest snapshot for the failing permission flag upsert integration test. - This change ensures consistency in the error message output, improving clarity in test results. This update aligns with ongoing efforts to enhance the accuracy and readability of test outputs.
…ingification - Updated the Jest snapshot for `ALL_UNIVERSAL_FLAT_ENTITY_PROPERTIES_TO_COMPARE_AND_STRINGIFY` to include the new `permissionFlag` property. - Defined `propertiesToCompare` for `permissionFlag`, enhancing the test coverage for permission flag functionalities. This change ensures that the snapshot accurately reflects the current state of the permission flag properties, improving the reliability of tests related to permission management.
- Included 'permissionFlag' in the AllMetadataName enum to enhance metadata structure. - Removed unused InstallApplication mutation and related types to streamline the codebase. This update improves integration with permission management functionalities and cleans up the GraphQL schema by eliminating unnecessary code.
- Added `permissionFlag` to the snapshots for `getMetadataRelatedMetadataNames` and `sortMetadataNamesChildrenFirst` utilities, ensuring comprehensive test coverage for permission-related functionalities. - Updated the snapshot for `ALL_UNIVERSAL_FLAT_ENTITY_FOREIGN_KEY_AGGREGATOR_PROPERTIES` to reflect the inclusion of `permissionFlag` and its associated properties. This change enhances the accuracy of tests related to permission management, aligning with recent updates to the metadata structure.
…orService - Implemented a check for duplicate permission flags for the same role within the FlatPermissionFlagValidatorService. - Added error handling to ensure that attempts to set a duplicate permission flag result in appropriate validation errors. This update enhances the validation logic for permission flags, improving the integrity of permission management within the workspace migration process.
- Removed unused `isDefined` utility and simplified the role ID assignment in the successful permission flag upsert integration test. - This change enhances code readability and maintains the integrity of the test logic. This update aligns with ongoing efforts to improve test clarity and maintainability.
- Introduced objectPermission handling in the application manifest converters and role utilities. - Updated metadata constants to include objectPermission properties and relations. - Added FlatObjectPermission types and editable properties for better integration. - Enhanced workspace migration utilities to accommodate objectPermission in universal flat entities. - Refactored ObjectPermissionEntity to extend SyncableEntity for improved functionality.
…ectPermission table - Added columns `universalIdentifier` and `applicationId` to the `objectPermission` table. - Created a unique index on `workspaceId` and `universalIdentifier`. - Established a foreign key constraint on `applicationId` referencing the `application` table. - Implemented rollback functionality to remove the added columns and constraints.
- Introduced WorkspaceFlatObjectPermissionMapCacheService for caching object permissions. - Added utility functions for converting between object permission entities and flat object permissions. - Created CreateObjectPermissionInput DTO for handling object permission creation. - Implemented functions to resolve entity relations and generate universal flat object permissions. - Updated workspace-many-or-all-flat-entity-maps-cache.module.ts to include new imports and services.
- Updated mock data and utility functions to include objectPermissionIds and objectPermissionUniversalIdentifiers. - Enhanced metadata handling in workspace migration utilities to support object permissions. - Modified tests to reflect changes in objectPermission integration across multiple components. - Added objectPermission case handling in migration action utilities for create, update, and delete actions.
- Introduced WorkspaceMigrationObjectPermissionActionsBuilderService for managing object permissions during migration. - Updated WorkspaceMigrationBuildOrchestratorService to integrate object permission validation and actions. - Enhanced workspace-migration-builder module to include object permission services and validators. - Added FlatObjectPermissionValidatorService for validating object permission creation, update, and deletion. - Implemented necessary changes in related modules to support object permission functionality.
…ace migration actions - Updated the actions property in the workspace migration runner to explicitly cast to AllUniversalWorkspaceMigrationAction[] for improved type safety. - Reorganized import statements for better clarity and structure.
- Implemented CreateObjectPermissionActionHandlerService, DeleteObjectPermissionActionHandlerService, and UpdateObjectPermissionActionHandlerService to manage object permissions during workspace migration. - Updated workspace-schema-migration-runner-action-handlers.module.ts to include new object permission services. - Enhanced the migration runner to support create, update, and delete actions for object permissions.
- Introduced `fromFlatObjectPermissionToObjectPermissionDto` utility function to transform `FlatObjectPermission` into `ObjectPermissionDTO`. - This utility enhances the handling of object permissions by providing a structured DTO representation.
…on services - Updated ObjectPermissionService to include ApplicationService and WorkspaceMigrationValidateBuildAndRunService for enhanced functionality. - Modified the upsertObjectPermissions method to improve permission validation and error handling. - Adjusted module imports in object-permission.module.ts to include ApplicationModule and WorkspaceMigrationModule. - Enhanced unit tests to reflect changes in service dependencies and validation logic.
- Created tests for both failing and successful scenarios of object permission upsert. - Implemented various test cases to validate error handling for invalid role IDs, non-editable roles, and system objects. - Added utility functions for upserting object permissions and generating GraphQL queries. - Included snapshot tests to ensure consistent error responses for failing cases.
…ean up related code.
- Removed the `createUpsertFieldPermissionsOperation` function from `upsert-field-permissions-operation-factory.util.ts`. - Updated `upsert-field-permissions.util.ts` to use `upsertFieldPermissionsQueryFactory` for constructing GraphQL operations. - Adjusted the input structure for the `upsertFieldPermissions` function to accept a single `UpsertFieldPermissionsInput` object instead of multiple parameters. This change streamlines the upsert field permissions logic and improves code organization.
- Removed unnecessary intermediate variable for resolved identifiers. - Directly destructured identifiers from the result of `resolveEntityRelationUniversalIdentifiers`. - Simplified the code for better readability and maintainability.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
abdulrahmancodes
force-pushed
the
migrate-field-permission-to-syncable-entity
branch
from
9f13a35 to
6f2c2f6
Compare
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
abdulrahmancodes
force-pushed
the
migrate-field-permission-to-syncable-entity
branch
from
e0dcd18 to
6f2c2f6
Compare
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
…ion-to-syncable-entity Made-with: Cursor # Conflicts: # packages/twenty-server/src/database/commands/upgrade-version-command/1-20/1-20-upgrade-version-command.module.ts # packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts
Comment on lines
+86
to
+96
| applicationIdToUniversalIdentifierMap, | ||
| roleIdToUniversalIdentifierMap, | ||
| objectMetadataIdToUniversalIdentifierMap, | ||
| fieldMetadataIdToUniversalIdentifierMap, | ||
| }); | ||
|
|
||
| addFlatEntityToFlatEntityMapsThroughMutationOrThrow({ | ||
| flatEntity: flatFieldPermission, | ||
| flatEntityMapsToMutate: flatFieldPermissionMaps, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: Cache computation for field permissions will fail for records with a null applicationId, causing requests to crash between deployment and the completion of a manual backfill migration.
Severity: CRITICAL
Suggested Fix
Make the cache computation logic in fromFieldPermissionEntityToFlatFieldPermission robust to null applicationId values. It should filter out or ignore field permissions that have not yet been migrated, preventing the entire cache recomputation process from failing due to a single unmigrated record.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-server/src/engine/metadata-modules/flat-field-permission/services/workspace-flat-field-permission-map-cache.service.ts#L82-L96
Potential issue: The `WorkspaceFlatFieldPermissionMapCacheService` computes a cache
on-demand when `upsertFieldPermissions` is called. This service uses
`fromFieldPermissionEntityToFlatFieldPermission`, which assumes the `applicationId` on a
field permission is never null. However, a database migration adds `applicationId` as a
nullable column, and a separate, manual command is required to backfill this value for
existing records. In the time between deployment and running the manual command, any
user action that triggers this cache computation will fail for records with a `null`
`applicationId`, causing an internal server error.
...r/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts
Show resolved
Hide resolved
…to-expansion The test picks the first field of the first non-system object, which is now a relation field after the main merge. addRelatedFieldPermissionsToDesired auto-adds the target field, producing 2 ROLE_NOT_EDITABLE errors instead of 1. Made-with: Cursor
The test picked the first field of a non-system object, which could be a RELATION field. addRelatedFieldPermissionsToDesired auto-adds the target field for relations, making the error count non-deterministic (1 vs 2). Now explicitly filters to a non-RELATION field for predictable results. Made-with: Cursor
Comment on lines
326
to
331
| if ( | ||
| ('canUpdateFieldValue' in fieldPermission && | ||
| fieldPermission.canUpdateFieldValue !== null && | ||
| (fieldPermission.canUpdateFieldValue !== null && | ||
| fieldPermission.canUpdateFieldValue !== false) || | ||
| ('canReadFieldValue' in fieldPermission && | ||
| fieldPermission.canReadFieldValue !== null && | ||
| (fieldPermission.canReadFieldValue !== null && | ||
| fieldPermission.canReadFieldValue !== false) | ||
| ) { |
There was a problem hiding this comment.
Bug: The validation logic in validateFieldPermission incorrectly rejects requests with partial input because it doesn't check if optional fields like canUpdateFieldValue are present before evaluating them.
Severity: CRITICAL
Suggested Fix
Restore the logic that checks for the presence of optional keys in the fieldPermission object before validating their values. For example, use the 'in' operator, such as 'canUpdateFieldValue' in fieldPermission, to guard the validation checks. This ensures that omitted fields are not incorrectly evaluated.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts#L326-L331
Potential issue: The `validateFieldPermission` function was refactored, removing checks
that verified if optional fields were present in the input object. When a GraphQL
mutation omits an optional field like `canReadFieldValue` or `canUpdateFieldValue`, its
value is `undefined`. The current validation logic, for example
`(fieldPermission.canUpdateFieldValue !== null && fieldPermission.canUpdateFieldValue
!== false)`, evaluates to `true` for `undefined`. This incorrectly triggers an
`ONLY_FIELD_RESTRICTION_ALLOWED` error for valid requests that only set one of the two
permissions (e.g., setting only `canReadFieldValue: false`). This blocks valid API
requests for updating field permissions.
|
Hey @abdulrahmancodes! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
Contributor
|
Thanks @abdulrahmancodes for your contribution! |
charlesBochet
added a commit
that referenced
this pull request
Mar 27, 2026
…19044) ## Summary ### Cache invalidation fix - After migrating object/field permissions to syncable entities (#18609, #18751, #18567), changes to `flatObjectPermissionMaps`, `flatFieldPermissionMaps`, or `flatPermissionFlagMaps` no longer triggered `rolesPermissions` cache invalidation - This caused stale permission data to be served, leading to flaky `permissions-on-relations` integration tests and potentially incorrect permission enforcement in production after object permission upserts - Adds the three permission-related flat map keys to the condition that triggers `rolesPermissions` cache recomputation in `WorkspaceMigrationRunnerService.getLegacyCacheInvalidationPromises` - Clears memoizer after recomputation to prevent concurrent `getOrRecompute` calls from caching stale data ### Docker Hub rate limit fix - CI service containers (postgres, redis, clickhouse) and `docker run`/`docker build` steps were pulling from Docker Hub **unauthenticated**, hitting the 100-pull-per-6-hour rate limit on shared GitHub-hosted runner IPs - Adds `credentials` blocks to all service container definitions and `docker/login-action` steps before `docker run`/`docker compose` commands - Uses `vars.DOCKERHUB_USERNAME` + `secrets.DOCKERHUB_PASSWORD` (matching the existing twenty-infra convention) - Affected workflows: ci-server, ci-merge-queue, ci-breaking-changes, ci-zapier, ci-sdk, ci-create-app-e2e, ci-website, ci-test-docker-compose, preview-env-keepalive, spawn-twenty-docker-image action
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.