From 0f7ebd302660d8c7584e15db5ccd859d9d5ce1d5 Mon Sep 17 00:00:00 2001 From: martmull Date: Wed, 20 Nov 2024 16:41:42 +0100 Subject: [PATCH] Fix wrong getAll keyValue method (#8603) Fixes https://github.com/twentyhq/twenty/issues/8520#issuecomment-2485913500 - Fix the getAll key-value pairs method, as it returns incorrect results. The keyVar values were being affected by keyValues from other workspaces. - Fix missing Sync Email step for existing users joining existing workspace Before, this method returned keyValues that verifies: - workspaceId = user.defaultWorkspaceId - userId = user.id This method now returns: - userId = null && workspaceId = user.defaultWorkspaceId - userId = user.id && workspaceId = null - userId = user.id && workspaceId = user.defaultWorkspaceId ## Result https://github.com/user-attachments/assets/b7563db6-a6a8-4e09-a0c6-c372d7e2b712 --- .../auth/services/sign-in-up.service.ts | 27 ++++------- .../user-vars/services/user-vars.service.ts | 47 +++++++++++++------ .../engine/core-modules/user/user.resolver.ts | 21 +++++---- 3 files changed, 51 insertions(+), 44 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts index c5cb98f0c6..e4cc32d50d 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts @@ -209,12 +209,10 @@ export class SignInUpService { ) : await this.userWorkspaceService.addUserToWorkspace(user, workspace); - if (isNewUser) { - await this.activateOnboardingForNewUser(user, workspace, { - firstName, - lastName, - }); - } + await this.activateOnboardingForUser(user, workspace, { + firstName, + lastName, + }); return Object.assign(user, updatedUser); } @@ -266,7 +264,7 @@ export class SignInUpService { return workspace; } - private async activateOnboardingForNewUser( + private async activateOnboardingForUser( user: User, workspace: Workspace, { firstName, lastName }: { firstName: string; lastName: string }, @@ -331,20 +329,11 @@ export class SignInUpService { await this.userWorkspaceService.create(user.id, workspace.id); - await this.onboardingService.setOnboardingConnectAccountPending({ - userId: user.id, - workspaceId: workspace.id, - value: true, + await this.activateOnboardingForUser(user, workspace, { + firstName, + lastName, }); - if (firstName === '' && lastName === '') { - await this.onboardingService.setOnboardingCreateProfilePending({ - userId: user.id, - workspaceId: workspace.id, - value: true, - }); - } - await this.onboardingService.setOnboardingInviteTeamPending({ workspaceId: workspace.id, value: true, diff --git a/packages/twenty-server/src/engine/core-modules/user/user-vars/services/user-vars.service.ts b/packages/twenty-server/src/engine/core-modules/user/user-vars/services/user-vars.service.ts index e4e345b3d9..2d60cf030b 100644 --- a/packages/twenty-server/src/engine/core-modules/user/user-vars/services/user-vars.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user/user-vars/services/user-vars.service.ts @@ -82,25 +82,42 @@ export class UserVarsService< userId?: string; workspaceId?: string; }): Promise, any>> { - const userVarsWorkspaceLevel = await this.keyValuePairService.get({ - type: KeyValuePairType.USER_VAR, - userId: null, - workspaceId, - }); - - let userVarsUserLevel: any[] = []; + let result: any[] = []; if (userId) { - userVarsUserLevel = await this.keyValuePairService.get({ - type: KeyValuePairType.USER_VAR, - userId, - }); + result = [ + ...result, + ...(await this.keyValuePairService.get({ + type: KeyValuePairType.USER_VAR, + userId, + workspaceId: null, + })), + ]; } - return mergeUserVars>([ - ...userVarsWorkspaceLevel, - ...userVarsUserLevel, - ]); + if (workspaceId) { + result = [ + ...result, + ...(await this.keyValuePairService.get({ + type: KeyValuePairType.USER_VAR, + userId: null, + workspaceId, + })), + ]; + } + + if (workspaceId && userId) { + result = [ + ...result, + ...(await this.keyValuePairService.get({ + type: KeyValuePairType.USER_VAR, + userId, + workspaceId, + })), + ]; + } + + return mergeUserVars>(result); } set({ diff --git a/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts b/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts index 80c0922a79..58dc0830fd 100644 --- a/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts @@ -25,7 +25,10 @@ import { EnvironmentService } from 'src/engine/core-modules/environment/environm import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { OnboardingStatus } from 'src/engine/core-modules/onboarding/enums/onboarding-status.enum'; -import { OnboardingService } from 'src/engine/core-modules/onboarding/onboarding.service'; +import { + OnboardingService, + OnboardingStepKeys, +} from 'src/engine/core-modules/onboarding/onboarding.service'; import { WorkspaceMember } from 'src/engine/core-modules/user/dtos/workspace-member.dto'; import { UserService } from 'src/engine/core-modules/user/services/user.service'; import { UserVarsService } from 'src/engine/core-modules/user/user-vars/services/user-vars.service'; @@ -36,6 +39,7 @@ import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorat import { DemoEnvGuard } from 'src/engine/guards/demo.env.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; +import { AccountsToReconnectKeys } from 'src/modules/connected-account/types/accounts-to-reconnect-key-value.type'; const getHMACKey = (email?: string, key?: string | null) => { if (!email || !key) return null; @@ -75,20 +79,17 @@ export class UserResolver { } @ResolveField(() => GraphQLJSONObject) - async userVars( - @Parent() user: User, - @AuthWorkspace() workspace: Workspace, - ): Promise> { + async userVars(@Parent() user: User): Promise> { const userVars = await this.userVarService.getAll({ userId: user.id, - workspaceId: workspace?.id ?? user.defaultWorkspaceId, + workspaceId: user.defaultWorkspaceId, }); const userVarAllowList = [ - 'SYNC_EMAIL_ONBOARDING_STEP', - 'ACCOUNTS_TO_RECONNECT_INSUFFICIENT_PERMISSIONS', - 'ACCOUNTS_TO_RECONNECT_EMAIL_ALIASES', - ]; + OnboardingStepKeys.ONBOARDING_CONNECT_ACCOUNT_PENDING, + AccountsToReconnectKeys.ACCOUNTS_TO_RECONNECT_INSUFFICIENT_PERMISSIONS, + AccountsToReconnectKeys.ACCOUNTS_TO_RECONNECT_EMAIL_ALIASES, + ] as string[]; const filteredMap = new Map( [...userVars].filter(([key]) => userVarAllowList.includes(key)),