From b870ed238fa1d75af822d7cf812081ab85f659d5 Mon Sep 17 00:00:00 2001 From: Paul Rastoin <45004772+prastoin@users.noreply.github.com> Date: Mon, 6 Oct 2025 15:51:48 +0200 Subject: [PATCH] Fix `twenty-server` integration test on main (#14905) Following https://github.com/twentyhq/twenty/pull/14869 Dynamically clearing the cache, as one entry was forgotten It seems like a permission test deletes a role that's expected to be existing in following tests, as now cache is getting invalidated tests are failing Seems to be related to https://discord.com/channels/1130383047699738754/1423768505911869460 ## Singleton local cache key collision When set for the first time caches local keys looks like `workspaceId:undefined", singleton is shared between several cache instances If the given key is undefined it will read on other entity cache entry. --- .vscode/launch.json | 2 +- .../interfaces/base-resolver-service.ts | 3 +- ...aphql-query-delete-one-resolver.service.ts | 4 +- .../storage/workspace-query-hook.storage.ts | 4 +- .../hooks/use-graphql-error-handler.hook.ts | 5 - ...space-permissions-cache-storage.service.ts | 32 ++-- .../workspace-permissions-cache.service.ts | 3 + ...-data-from-cache-with-recompute.service.ts | 12 +- .../workspace-cache-storage.service.ts | 118 ++++++-------- packages/twenty-server/src/instrument.ts | 4 +- ...pace-member-delete-one.post-query.hook.ts} | 30 ++-- .../workspace-member-query-hook.module.ts | 4 +- .../workspace-members.integration-spec.ts | 153 +++++++++--------- 13 files changed, 183 insertions(+), 191 deletions(-) rename packages/twenty-server/src/modules/workspace-member/query-hooks/{workspace-member-delete-one.pre-query.hook.ts => workspace-member-delete-one.post-query.hook.ts} (80%) diff --git a/.vscode/launch.json b/.vscode/launch.json index eb67626726..bd3a306621 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -67,7 +67,7 @@ "--config", "./jest-integration.config.ts", "${relativeFile}", - "--testTimeout=0" + "--silent=false" ], "cwd": "${workspaceFolder}/packages/twenty-server", "console": "integratedTerminal", diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts index 51332c2a63..c051d61999 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts @@ -28,6 +28,7 @@ import { workspaceQueryRunnerGraphqlApiExceptionHandler } from 'src/engine/api/g import { WorkspaceQueryHookService } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/workspace-query-hook.service'; import { ApiKeyRoleService } from 'src/engine/core-modules/api-key/api-key-role.service'; import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; +import { WorkspaceNotFoundDefaultError } from 'src/engine/core-modules/workspace/workspace.exception'; import { type PermissionFlagType } from 'src/engine/metadata-modules/permissions/constants/permission-flag-type.constants'; import { PermissionsException, @@ -39,7 +40,6 @@ import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role import { type WorkspaceDataSource } from 'src/engine/twenty-orm/datasource/workspace.datasource'; import { type WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; -import { WorkspaceNotFoundDefaultError } from 'src/engine/core-modules/workspace/workspace.exception'; export type GraphqlQueryResolverExecutionArgs = { args: Input; @@ -185,7 +185,6 @@ export abstract class GraphqlQueryBaseResolverService< roleId, shouldBypassPermissionChecks, }; - const results = await this.resolve( graphqlQueryResolverExecutionArgs, featureFlagsMap, diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-delete-one-resolver.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-delete-one-resolver.service.ts index 062ac22055..d38b767506 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-delete-one-resolver.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-delete-one-resolver.service.ts @@ -77,12 +77,14 @@ export class GraphqlQueryDeleteOneResolverService extends GraphqlQueryBaseResolv const typeORMObjectRecordsParser = new ObjectRecordsToGraphqlConnectionHelper(objectMetadataMaps); - return typeORMObjectRecordsParser.processRecord({ + const result = typeORMObjectRecordsParser.processRecord({ objectRecord: deletedRecord, objectName: objectMetadataItemWithFieldMaps.nameSingular, take: 1, totalCount: 1, }); + + return result; } async validate( diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-hook/storage/workspace-query-hook.storage.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-hook/storage/workspace-query-hook.storage.ts index 5943190415..9a48d43228 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-hook/storage/workspace-query-hook.storage.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-hook/storage/workspace-query-hook.storage.ts @@ -98,6 +98,8 @@ export class WorkspaceQueryHookStorage { wildcardInstances = wildcardPosthooksInstance; } - return [...wildcardInstances, ...(this.postHookInstances.get(key) ?? [])]; + const specificInstances = this.postHookInstances.get(key) ?? []; + + return [...wildcardInstances, ...specificInstances]; } } diff --git a/packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts b/packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts index e8164090cd..bd43061cbc 100644 --- a/packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts +++ b/packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts @@ -95,11 +95,6 @@ export const useGraphQLErrorHandlerHook = < 'Anonymous Operation'; const workspaceInfo = extractWorkspaceInfo(args.contextValue.req); - // eslint-disable-next-line no-console - console.log( - `[GQL Execute] Processing GQL query ${opName} on workspace ${workspaceInfo?.id}`, - ); - return { onExecuteDone(payload) { const handleResult: OnExecuteDoneHookResultOnNextHook = ({ diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts index a0e1e0326e..b583f63442 100644 --- a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache-storage.service.ts @@ -7,7 +7,7 @@ import { InjectCacheStorage } from 'src/engine/core-modules/cache-storage/decora import { CacheStorageService } from 'src/engine/core-modules/cache-storage/services/cache-storage.service'; import { CacheStorageNamespace } from 'src/engine/core-modules/cache-storage/types/cache-storage-namespace.enum'; import { type UserWorkspaceRoleMap } from 'src/engine/metadata-modules/workspace-permissions-cache/types/user-workspace-role-map.type'; -import { WorkspaceCacheKeys } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; +import { WORKSPACE_CACHE_KEYS } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; const TTL_INFINITE = 0; @@ -24,7 +24,7 @@ export class WorkspacePermissionsCacheStorageService { const rolesPermissionsVersion = v4(); await this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataPermissionsRolesPermissionsVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsRolesPermissionsVersion}:${workspaceId}`, rolesPermissionsVersion, TTL_INFINITE, ); @@ -40,7 +40,7 @@ export class WorkspacePermissionsCacheStorageService { }> { const [, newRolesPermissionsVersion] = await Promise.all([ this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataPermissionsRolesPermissions}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsRolesPermissions}:${workspaceId}`, permissions, TTL_INFINITE, ), @@ -54,13 +54,13 @@ export class WorkspacePermissionsCacheStorageService { workspaceId: string, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.MetadataPermissionsRolesPermissions}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsRolesPermissions}:${workspaceId}`, ); } getRolesPermissionsVersion(workspaceId: string): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.MetadataPermissionsRolesPermissionsVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsRolesPermissionsVersion}:${workspaceId}`, ); } @@ -70,7 +70,7 @@ export class WorkspacePermissionsCacheStorageService { ): Promise { await Promise.all([ this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, userWorkspaceRoleMap, TTL_INFINITE, ), @@ -82,7 +82,7 @@ export class WorkspacePermissionsCacheStorageService { const userWorkspaceRoleMapVersion = v4(); await this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsUserWorkspaceRoleMapVersion}:${workspaceId}`, userWorkspaceRoleMapVersion, TTL_INFINITE, ); @@ -94,7 +94,7 @@ export class WorkspacePermissionsCacheStorageService { workspaceId: string, ): Promise | undefined> { return this.cacheStorageService.get>( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, ); } @@ -102,13 +102,13 @@ export class WorkspacePermissionsCacheStorageService { workspaceId: string, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsUserWorkspaceRoleMapVersion}:${workspaceId}`, ); } removeUserWorkspaceRoleMap(workspaceId: string) { return this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, ); } @@ -118,7 +118,7 @@ export class WorkspacePermissionsCacheStorageService { ): Promise { await Promise.all([ this.cacheStorageService.set>( - `${WorkspaceCacheKeys.MetadataPermissionsApiKeyRoleMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsApiKeyRoleMap}:${workspaceId}`, apiKeyRoleMap, TTL_INFINITE, ), @@ -130,7 +130,7 @@ export class WorkspacePermissionsCacheStorageService { workspaceId: string, ): Promise | undefined> { return this.cacheStorageService.get>( - `${WorkspaceCacheKeys.MetadataPermissionsApiKeyRoleMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsApiKeyRoleMap}:${workspaceId}`, ); } @@ -138,17 +138,17 @@ export class WorkspacePermissionsCacheStorageService { workspaceId: string, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.MetadataPermissionsApiKeyRoleMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsApiKeyRoleMapVersion}:${workspaceId}`, ); } async removeApiKeyRoleMap(workspaceId: string): Promise { await Promise.all([ this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsApiKeyRoleMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsApiKeyRoleMap}:${workspaceId}`, ), this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsApiKeyRoleMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsApiKeyRoleMapVersion}:${workspaceId}`, ), ]); } @@ -157,7 +157,7 @@ export class WorkspacePermissionsCacheStorageService { const apiKeyRoleMapVersion = v4(); await this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataPermissionsApiKeyRoleMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.MetadataPermissionsApiKeyRoleMapVersion}:${workspaceId}`, apiKeyRoleMapVersion, TTL_INFINITE, ); diff --git a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts index 64585ac2e3..692964c23c 100644 --- a/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service.ts @@ -109,6 +109,9 @@ export class WorkspacePermissionsCacheService { workspaceId, freshUserWorkspaceRoleMap, ); + await this.workspacePermissionsCacheStorageService.setUserWorkspaceRoleMapVersion( + workspaceId, + ); } catch { // Flush stale userWorkspaceRoleMap await this.workspacePermissionsCacheStorageService.removeUserWorkspaceRoleMap( diff --git a/packages/twenty-server/src/engine/workspace-cache-storage/services/get-data-from-cache-with-recompute.service.ts b/packages/twenty-server/src/engine/workspace-cache-storage/services/get-data-from-cache-with-recompute.service.ts index bd39f80bae..4a06d820d7 100644 --- a/packages/twenty-server/src/engine/workspace-cache-storage/services/get-data-from-cache-with-recompute.service.ts +++ b/packages/twenty-server/src/engine/workspace-cache-storage/services/get-data-from-cache-with-recompute.service.ts @@ -40,11 +40,13 @@ export class GetDataFromCacheWithRecomputeService { cachedVersion = await getCacheVersion(workspaceId); - const cacheKey = `${workspaceId}-${cachedVersion}`; - const cachedValue = this.cache.get(cacheKey); + if (isDefined(cachedVersion)) { + const cacheKey = `${workspaceId}-${cachedVersion}`; + const cachedValue = this.cache.get(cacheKey); - if (cachedValue) { - return cachedValue; + if (cachedValue) { + return cachedValue; + } } cachedData = await getCacheData(workspaceId); @@ -77,6 +79,8 @@ export class GetDataFromCacheWithRecomputeService { } } + const cacheKey = `${workspaceId}-${cachedVersion}`; + this.cache.set(cacheKey, { version: cachedVersion, data: cachedData, diff --git a/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts b/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts index e7f8391555..7c0d3fd0fa 100644 --- a/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts +++ b/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts @@ -20,23 +20,29 @@ import { WorkspaceMetadataVersionExceptionCode, } from 'src/engine/metadata-modules/workspace-metadata-version/exceptions/workspace-metadata-version.exception'; -export enum WorkspaceCacheKeys { - GraphQLTypeDefs = 'graphql:type-defs', - GraphQLUsedScalarNames = 'graphql:used-scalar-names', - GraphQLOperations = 'graphql:operations', - ORMEntitySchemas = 'orm:entity-schemas', - GraphQLFeatureFlag = 'graphql:feature-flag', - MetadataObjectMetadataMaps = 'metadata:object-metadata-maps', - MetadataVersion = 'metadata:workspace-metadata-version', - FeatureFlagMap = 'feature-flag:feature-flag-map', - FeatureFlagMapVersion = 'feature-flag:feature-flag-map-version', - MetadataPermissionsRolesPermissions = 'metadata:permissions:roles-permissions', - MetadataPermissionsRolesPermissionsVersion = 'metadata:permissions:roles-permissions-version', - MetadataPermissionsUserWorkspaceRoleMap = 'metadata:permissions:user-workspace-role-map', - MetadataPermissionsUserWorkspaceRoleMapVersion = 'metadata:permissions:user-workspace-role-map-version', - MetadataPermissionsApiKeyRoleMap = 'metadata:permissions:api-key-role-map', - MetadataPermissionsApiKeyRoleMapVersion = 'metadata:permissions:api-key-role-map-version', -} +export const METADATA_VERSIONED_WORKSPACE_CACHE_KEY = { + GraphQLTypeDefs: 'graphql:type-defs', + MetadataVersion: 'metadata:workspace-metadata-version', + MetadataObjectMetadataMaps: 'metadata:object-metadata-maps', + GraphQLUsedScalarNames: 'graphql:used-scalar-names', + ORMEntitySchemas: 'orm:entity-schemas', +} as const; +export const WORKSPACE_CACHE_KEYS = { + GraphQLOperations: 'graphql:operations', + GraphQLFeatureFlag: 'graphql:feature-flag', + FeatureFlagMap: 'feature-flag:feature-flag-map', + FeatureFlagMapVersion: 'feature-flag:feature-flag-map-version', + MetadataPermissionsRolesPermissions: 'metadata:permissions:roles-permissions', + MetadataPermissionsRolesPermissionsVersion: + 'metadata:permissions:roles-permissions-version', + MetadataPermissionsUserWorkspaceRoleMap: + 'metadata:permissions:user-workspace-role-map', + MetadataPermissionsUserWorkspaceRoleMapVersion: + 'metadata:permissions:user-workspace-role-map-version', + MetadataPermissionsApiKeyRoleMap: 'metadata:permissions:api-key-role-map', + MetadataPermissionsApiKeyRoleMapVersion: + 'metadata:permissions:api-key-role-map-version', +} as const; const TTL_ONE_WEEK = 1000 * 60 * 60 * 24 * 7; @@ -55,7 +61,7 @@ export class WorkspaceCacheStorageService { ) { // eslint-disable-next-line @typescript-eslint/no-explicit-any return this.cacheStorageService.set[]>( - `${WorkspaceCacheKeys.ORMEntitySchemas}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.ORMEntitySchemas}:${workspaceId}:${metadataVersion}`, entitySchemas, TTL_ONE_WEEK, ); @@ -68,7 +74,7 @@ export class WorkspaceCacheStorageService { ): Promise[] | undefined> { // eslint-disable-next-line @typescript-eslint/no-explicit-any return this.cacheStorageService.get[]>( - `${WorkspaceCacheKeys.ORMEntitySchemas}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.ORMEntitySchemas}:${workspaceId}:${metadataVersion}`, ); } @@ -77,7 +83,7 @@ export class WorkspaceCacheStorageService { metadataVersion: number, ): Promise { return this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataVersion}:${workspaceId}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.MetadataVersion}:${workspaceId}`, metadataVersion, TTL_ONE_WEEK, ); @@ -85,7 +91,7 @@ export class WorkspaceCacheStorageService { getMetadataVersion(workspaceId: string): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.MetadataVersion}:${workspaceId}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.MetadataVersion}:${workspaceId}`, ); } @@ -95,7 +101,7 @@ export class WorkspaceCacheStorageService { objectMetadataMaps: ObjectMetadataMaps, ) { return this.cacheStorageService.set( - `${WorkspaceCacheKeys.MetadataObjectMetadataMaps}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.MetadataObjectMetadataMaps}:${workspaceId}:${metadataVersion}`, objectMetadataMaps, TTL_ONE_WEEK, ); @@ -106,7 +112,7 @@ export class WorkspaceCacheStorageService { metadataVersion: number, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.MetadataObjectMetadataMaps}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.MetadataObjectMetadataMaps}:${workspaceId}:${metadataVersion}`, ); } @@ -141,7 +147,7 @@ export class WorkspaceCacheStorageService { typeDefs: string, ): Promise { return this.cacheStorageService.set( - `${WorkspaceCacheKeys.GraphQLTypeDefs}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.GraphQLTypeDefs}:${workspaceId}:${metadataVersion}`, typeDefs, TTL_ONE_WEEK, ); @@ -152,7 +158,7 @@ export class WorkspaceCacheStorageService { metadataVersion: number, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.GraphQLTypeDefs}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.GraphQLTypeDefs}:${workspaceId}:${metadataVersion}`, ); } @@ -162,7 +168,7 @@ export class WorkspaceCacheStorageService { usedScalarNames: string[], ): Promise { return this.cacheStorageService.set( - `${WorkspaceCacheKeys.GraphQLUsedScalarNames}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.GraphQLUsedScalarNames}:${workspaceId}:${metadataVersion}`, usedScalarNames, TTL_ONE_WEEK, ); @@ -173,7 +179,7 @@ export class WorkspaceCacheStorageService { metadataVersion: number, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.GraphQLUsedScalarNames}:${workspaceId}:${metadataVersion}`, + `${METADATA_VERSIONED_WORKSPACE_CACHE_KEY.GraphQLUsedScalarNames}:${workspaceId}:${metadataVersion}`, ); } @@ -181,7 +187,7 @@ export class WorkspaceCacheStorageService { workspaceId: string, ): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.FeatureFlagMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.FeatureFlagMapVersion}:${workspaceId}`, ); } @@ -189,7 +195,7 @@ export class WorkspaceCacheStorageService { const featureFlagMapVersion = crypto.randomUUID(); await this.cacheStorageService.set( - `${WorkspaceCacheKeys.FeatureFlagMapVersion}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.FeatureFlagMapVersion}:${workspaceId}`, featureFlagMapVersion, TTL_ONE_WEEK, ); @@ -205,7 +211,7 @@ export class WorkspaceCacheStorageService { }> { const [, newFeatureFlagMapVersion] = await Promise.all([ this.cacheStorageService.set( - `${WorkspaceCacheKeys.FeatureFlagMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.FeatureFlagMap}:${workspaceId}`, featureFlagMap, TTL_ONE_WEEK, ), @@ -217,7 +223,7 @@ export class WorkspaceCacheStorageService { getFeatureFlagsMap(workspaceId: string): Promise { return this.cacheStorageService.get( - `${WorkspaceCacheKeys.FeatureFlagMap}:${workspaceId}`, + `${WORKSPACE_CACHE_KEYS.FeatureFlagMap}:${workspaceId}`, ); } @@ -229,7 +235,7 @@ export class WorkspaceCacheStorageService { workspaceId: string; }): Promise { await this.cacheStorageService.flushByPattern( - `${WorkspaceCacheKeys.GraphQLOperations}:${operationName}:${workspaceId}:*`, + `${WORKSPACE_CACHE_KEYS.GraphQLOperations}:${operationName}:${workspaceId}:*`, ); } @@ -241,48 +247,24 @@ export class WorkspaceCacheStorageService { ? `${metadataVersion}` : '*'; - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataObjectMetadataMaps}:${workspaceId}:${metadataVersionSuffix}`, - ); - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataVersion}:${workspaceId}:${metadataVersionSuffix}`, - ); - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.GraphQLTypeDefs}:${workspaceId}:${metadataVersionSuffix}`, - ); - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.GraphQLUsedScalarNames}:${workspaceId}:${metadataVersionSuffix}`, - ); - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.ORMEntitySchemas}:${workspaceId}:${metadataVersionSuffix}`, + await Promise.all( + Object.values(METADATA_VERSIONED_WORKSPACE_CACHE_KEY).map( + async (key) => + await this.cacheStorageService.del( + `${key}:${workspaceId}:${metadataVersionSuffix}`, + ), + ), ); } async flush(workspaceId: string, metadataVersion?: number): Promise { await this.flushVersionedMetadata(workspaceId, metadataVersion); - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsRolesPermissions}:${workspaceId}`, - ); - - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsRolesPermissionsVersion}:${workspaceId}`, - ); - - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMap}:${workspaceId}`, - ); - - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.MetadataPermissionsUserWorkspaceRoleMapVersion}:${workspaceId}`, - ); - - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.FeatureFlagMap}:${workspaceId}`, - ); - - await this.cacheStorageService.del( - `${WorkspaceCacheKeys.FeatureFlagMapVersion}:${workspaceId}`, + await Promise.all( + Object.values(WORKSPACE_CACHE_KEYS).map( + async (key) => + await this.cacheStorageService.del(`${key}:${workspaceId}`), + ), ); } } diff --git a/packages/twenty-server/src/instrument.ts b/packages/twenty-server/src/instrument.ts index fc68041b88..766ff13386 100644 --- a/packages/twenty-server/src/instrument.ts +++ b/packages/twenty-server/src/instrument.ts @@ -15,7 +15,7 @@ import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interface import { ExceptionHandlerDriver } from 'src/engine/core-modules/exception-handler/interfaces'; import { MeterDriver } from 'src/engine/core-modules/metrics/types/meter-driver.type'; -import { WorkspaceCacheKeys } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; +import { WORKSPACE_CACHE_KEYS } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; import { parseArrayEnvVar } from 'src/utils/parse-array-env-var'; const meterDrivers = parseArrayEnvVar( @@ -32,7 +32,7 @@ if (process.env.EXCEPTION_HANDLER_DRIVER === ExceptionHandlerDriver.SENTRY) { integrations: [ // TODO: Redis integration doesn't seem to work - investigate why Sentry.redisIntegration({ - cachePrefixes: Object.values(WorkspaceCacheKeys).map( + cachePrefixes: Object.values(WORKSPACE_CACHE_KEYS).map( (key) => `engine:${key}:`, ), }), diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.post-query.hook.ts similarity index 80% rename from packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts rename to packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.post-query.hook.ts index 26d64520eb..e4ee1768c8 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.post-query.hook.ts @@ -3,12 +3,13 @@ import { InjectRepository } from '@nestjs/typeorm'; import { assertIsDefinedOrThrow, isDefined } from 'twenty-shared/utils'; import { Repository } from 'typeorm'; -import { type WorkspacePreQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; -import { type DeleteOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; +import { type WorkspacePostQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { WorkspaceQueryHookType } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/types/workspace-query-hook.type'; import { type AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; +import { WorkspaceNotFoundDefaultError } from 'src/engine/core-modules/workspace/workspace.exception'; import { PermissionsException, PermissionsExceptionCode, @@ -17,11 +18,13 @@ import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; import { type AttachmentWorkspaceEntity } from 'src/modules/attachment/standard-objects/attachment.workspace-entity'; import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; import { type WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; -import { WorkspaceNotFoundDefaultError } from 'src/engine/core-modules/workspace/workspace.exception'; -@WorkspaceQueryHook(`workspaceMember.deleteOne`) -export class WorkspaceMemberDeleteOnePreQueryHook - implements WorkspacePreQueryHookInstance +@WorkspaceQueryHook({ + key: `workspaceMember.deleteOne`, + type: WorkspaceQueryHookType.POST_HOOK, +}) +export class WorkspaceMemberDeleteOnePostQueryHook + implements WorkspacePostQueryHookInstance { constructor( private readonly twentyORMManager: TwentyORMManager, @@ -33,9 +36,14 @@ export class WorkspaceMemberDeleteOnePreQueryHook async execute( authContext: AuthContext, _objectName: string, - payload: DeleteOneResolverArgs, - ): Promise { - const targettedWorkspaceMemberId = payload.id; + payload: WorkspaceMemberWorkspaceEntity[], + ): Promise { + if (!payload || payload.length === 0) { + return; + } + + const deletedWorkspaceMember = payload[0]; + const targettedWorkspaceMemberId = deletedWorkspaceMember.id; const workspace = authContext.workspace; @@ -71,10 +79,10 @@ export class WorkspaceMemberDeleteOnePreQueryHook where: { id: targettedWorkspaceMemberId, }, + withDeleted: true, }); if (!isDefined(workspaceMember)) { - // TODO: once this is migrated to userWorkspace service we should throw UserWorkspaceException throw new PermissionsException( 'Workspace member not found', PermissionsExceptionCode.WORKSPACE_MEMBER_NOT_FOUND, @@ -96,7 +104,5 @@ export class WorkspaceMemberDeleteOnePreQueryHook } await this.userWorkspaceRepository.delete(userWorkspace.id); - - return payload; } } diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts index 900a0f17b8..40a350e4a5 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts @@ -7,7 +7,7 @@ import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permi import { WorkspaceMemberCreateManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook'; import { WorkspaceMemberCreateOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook'; import { WorkspaceMemberDeleteManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-delete-many.pre-query.hook'; -import { WorkspaceMemberDeleteOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook'; +import { WorkspaceMemberDeleteOnePostQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-delete-one.post-query.hook'; import { WorkspaceMemberDestroyManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-destroy-many.pre-query.hook'; import { WorkspaceMemberDestroyOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-destroy-one.pre-query.hook'; import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; @@ -21,7 +21,7 @@ import { WorkspaceMemberUpdateOnePreQueryHook } from 'src/modules/workspace-memb WorkspaceMemberPreQueryHookService, WorkspaceMemberCreateOnePreQueryHook, WorkspaceMemberCreateManyPreQueryHook, - WorkspaceMemberDeleteOnePreQueryHook, + WorkspaceMemberDeleteOnePostQueryHook, WorkspaceMemberDeleteManyPreQueryHook, WorkspaceMemberDestroyOnePreQueryHook, WorkspaceMemberDestroyManyPreQueryHook, diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts index d859513542..2235c0c46b 100644 --- a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts @@ -15,94 +15,93 @@ const WORKSPACE_MEMBER_GQL_FIELDS = ` `; describe('workspace members permissions', () => { - describe('updateOne', () => { - it('should allow update when user is updating themself (member role)', async () => { - const graphqlOperation = updateOneOperationFactory({ - objectMetadataSingularName: 'workspaceMember', - gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, - recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, - data: { - name: { - firstName: 'Jony', - }, + it('should allow update when user is updating themself (member role)', async () => { + const graphqlOperation = updateOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + data: { + name: { + firstName: 'Jony', }, - }); - - const response = - await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); - - expect(response.body.data).toStrictEqual({ - updateWorkspaceMember: { - id: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, - name: { - firstName: 'Jony', - }, - }, - }); - expect(response.body.errors).toBeUndefined(); + }, }); - it('should throw when user does not have permission (member role)', async () => { - const graphqlOperation = updateOneOperationFactory({ - objectMetadataSingularName: 'workspaceMember', - gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, - recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.TIM, - data: { - name: { - firstName: 'Not Tim', - }, + + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); + + expect(response.body.errors).not.toBeDefined(); + expect(response.body.data).toStrictEqual({ + updateWorkspaceMember: { + id: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + name: { + firstName: 'Jony', }, - }); - - const response = - await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); - - expect(response.body.data).toStrictEqual({ updateWorkspaceMember: null }); - expect(response.body.errors).toBeDefined(); - expect(response.body.errors[0].message).toBe( - PermissionsExceptionMessage.PERMISSION_DENIED, - ); - expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }, }); + expect(response.body.errors).toBeUndefined(); + }); + it('should throw when user does not have permission (member role)', async () => { + const graphqlOperation = updateOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.TIM, + data: { + name: { + firstName: 'Not Tim', + }, + }, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); + + expect(response.body.data).toStrictEqual({ updateWorkspaceMember: null }); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); }); - describe('deleteOne', () => { - it('should throw when user does not have permission (member role)', async () => { - const graphqlOperation = deleteOneOperationFactory({ - objectMetadataSingularName: 'workspaceMember', - gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, - recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.TIM, - }); - - const response = - await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); - - expect(response.body.data).toStrictEqual({ deleteWorkspaceMember: null }); - expect(response.body.errors).toBeDefined(); - expect(response.body.errors[0].message).toBe( - PermissionsExceptionMessage.PERMISSION_DENIED, - ); - expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + it('should throw when user does not have permission (member role)', async () => { + const graphqlOperation = deleteOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.TIM, }); - it('should allow delete when user is deleting themself (member role)', async () => { - const deleteOperation = deleteOneOperationFactory({ - objectMetadataSingularName: 'workspaceMember', - gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, - recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, - }); + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); - const deleteResponse = - await makeGraphqlAPIRequestWithAcmeMemberRole(deleteOperation); + expect(response.body.data).toStrictEqual({ deleteWorkspaceMember: null }); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }); - expect(deleteResponse.body.data).toStrictEqual({ - deleteWorkspaceMember: { - id: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, - name: { - firstName: 'Jony', - }, + // This test is not idempotent + it('should allow delete when user is deleting themself (member role)', async () => { + const deleteOperation = deleteOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + }); + + const deleteResponse = + await makeGraphqlAPIRequestWithAcmeMemberRole(deleteOperation); + + expect(deleteResponse.body.errors).not.toBeDefined(); + expect(deleteResponse.body.data).toStrictEqual({ + deleteWorkspaceMember: { + id: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + name: { + firstName: 'Jony', }, - }); - expect(deleteResponse.body.errors).toBeUndefined(); + }, }); + expect(deleteResponse.body.errors).toBeUndefined(); }); });