Fix user deletion flows by ijreilly · Pull Request #15614 · twentyhq/twenty

ijreilly · 2025-11-04T17:44:34Z

Before

any user with workpace_members permission was able to remove a user from their workspace. This triggered the deletion of workspaceMember + of userWorkspace, but did not delete the user (even if they had no workspace left) nor the roleTarget (acts as junction between role and userWorkspace) which was left with a userWorkspaceId pointing to nothing. This is because roleTarget points to userWorkspaceId but the foreign key constraint was not implemented
any user could delete their own account. This triggered the deletion of all their workspaceMembers, but not of their userWorkspace nor their user nor the roleTarget --> we have orphaned userWorkspace, not technically but product wise - a userWorkspace without a workspaceMember does not make sense

So the problems are

we have some roleTargets pointing to non-existing userWorkspaceId (which caused Deleting a role gives a message Workspace not found #14608 )
we have userWorkspaces that should not exist and that have no workspaceMember counterpart
it is not possible for a user to leave a workspace by themselves, they can only leave all workspaces at once, except if they are being removed from the workspace by another user

Now

if a user has multiple workspaces, they are given the possibility to leave one workspace while remaining in the others (we show two buttons: Leave workspace and Delete account buttons). if a user has just one workspace, they only see Delete account
when a user leaves a workspace, we delete their workspaceMember, userWorkspace and roleTarget. If they don't belong to any other workspace we also soft-delete their user
soft-deleted users get hard deleted after 30 days thanks to a cron
we have two commands to clean the orphans roleTarget and userWorkspace (TODO: query db to see how many must be run)

Next

once the commands have been run, we can implement and introduce the foreign key constraint on roleTarget

Fixes #14608

…tion

packages/twenty-server/src/engine/core-modules/user/services/user.service.ts

greptile-apps

Greptile Overview

Greptile Summary

This PR fixes user deletion flows by properly cleaning up workspaceMember, userWorkspace, and roleTarget records. It adds the ability for users to leave individual workspaces while remaining in others, and implements soft-deletion with a 30-day retention period.

Key Changes:

Added deleteUserWorkspaceAndPotentiallyDeleteUser to allow users to leave workspaces individually
Introduced soft-delete for users with hard-delete after 30 days via cron job
Created cleanup commands for orphaned roleTarget and userWorkspace records
Updated frontend to show "Leave workspace" button when user has multiple workspaces

Critical Issues Found:

Missing await on async call in user.service.ts:144 will cause incomplete cleanup
Frontend mutation missing required workspaceMemberIdToDelete parameter - feature will fail at runtime
Cron job has infinite loop bug - hasMore flag never updated
Role targets cleanup command only processes one workspace due to early return

Confidence Score: 1/5

This PR has critical bugs that will cause runtime failures and data inconsistency
Score reflects multiple critical logic errors: missing await causing race conditions, missing mutation parameter causing frontend failures, infinite loop in cron job, and broken cleanup command. These issues will prevent the feature from working correctly and could lead to orphaned records.
Critical attention needed: user.service.ts (missing await), DeleteAccount.tsx (missing mutation parameter), user-hard-delete.cron.job.ts (infinite loop), and 1-11-clean-orphaned-role-targets.command.ts (only processes one workspace)

Important Files Changed

File Analysis

Filename	Score	Overview
packages/twenty-server/src/engine/core-modules/user/services/user.service.ts	1/5	Added user deletion logic with workspace cleanup, but missing await on async call (line 144) which could cause incomplete cleanup
packages/twenty-front/src/modules/settings/profile/components/DeleteAccount.tsx	0/5	UI for leave workspace feature, but mutation call missing required `workspaceMemberIdToDelete` parameter
packages/twenty-server/src/engine/core-modules/user/crons/user-hard-delete.cron.job.ts	2/5	Cron job for hard-deleting soft-deleted users after 30 days, but `hasMore` flag never updated causing infinite loop
packages/twenty-server/src/database/commands/upgrade-version-command/1-11/1-11-clean-orphaned-role-targets.command.ts	2/5	Cleanup command for orphaned role targets, but `hasRunOnce` flag prevents processing multiple workspaces

Sequence Diagram

sequenceDiagram
    participant User
    participant Frontend
    participant UserResolver
    participant UserService
    participant UserWorkspaceService
    participant WorkspaceMemberRepo
    participant UserWorkspaceRepo
    participant RoleTargetsRepo
    participant WorkspaceService

    alt User has multiple workspaces - Leave Workspace
        User->>Frontend: Click "Leave workspace"
        Frontend->>UserResolver: deleteUserFromWorkspace(workspaceMemberIdToDelete)
        UserResolver->>UserResolver: Validate permissions
        UserResolver->>UserService: deleteUserWorkspaceAndPotentiallyDeleteUser()
        UserService->>UserService: Load user with userWorkspaces
        UserService->>UserService: Find userWorkspace for workspaceId
        UserService->>UserService: removeUserFromWorkspaceAndPotentiallyDeleteWorkspace()
        UserService->>WorkspaceMemberRepo: delete(userId)
        UserService->>UserWorkspaceService: deleteUserWorkspace(userWorkspaceId)
        UserWorkspaceService->>RoleTargetsRepo: delete(userWorkspaceId)
        UserWorkspaceService->>UserWorkspaceRepo: delete(id)
        alt User has only 1 workspace
            UserService->>UserService: softDelete(userId)
        end
    else User deletes account entirely
        User->>Frontend: Click "Delete account"
        Frontend->>UserResolver: deleteUser()
        UserResolver->>UserService: deleteUser(userId)
        UserService->>UserService: Load user with userWorkspaces
        loop For each userWorkspace
            UserService->>UserService: removeUserFromWorkspaceAndPotentiallyDeleteWorkspace()
            UserService->>WorkspaceMemberRepo: delete(userId)
            UserService->>UserWorkspaceService: deleteUserWorkspace(userWorkspaceId)
            UserWorkspaceService->>RoleTargetsRepo: delete(userWorkspaceId)
            UserWorkspaceService->>UserWorkspaceRepo: delete(id)
            alt Last member in workspace
                UserService->>WorkspaceService: deleteWorkspace(workspaceId)
            end
        end
        UserService->>UserService: softDelete(userId)
    end

    Note over UserService: After 30 days
    UserHardDeleteCronJob->>UserWorkspaceRepo: Find soft-deleted users older than 30 days
    UserHardDeleteCronJob->>UserWorkspaceRepo: Hard delete users

_{27 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-11-04T17:50:21Z

packages/twenty-server/src/engine/core-modules/user/services/user.service.ts


-          if (workspaceMembers.length === 1) {
-            await this.workspaceService.deleteWorkspace(workspaceId);
+    this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(userWorkspace);


logic: missing await - removeUserFromWorkspaceAndPotentiallyDeleteWorkspace is async but not awaited

Suggested change

this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(userWorkspace);

await this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(userWorkspace);

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-server/src/engine/core-modules/user/services/user.service.ts Line: 144:144 Comment: **logic:** missing `await` - `removeUserFromWorkspaceAndPotentiallyDeleteWorkspace` is async but not awaited ```suggestion await this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(userWorkspace); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-11-04T17:50:22Z

packages/twenty-server/src/engine/core-modules/user/crons/user-hard-delete.cron.job.ts

+    let totalDeleted = 0;
+
+    let hasMore = true;
+


logic: hasMore variable is set but never used - the while loop will run indefinitely

Suggested change

while (hasMore) {

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-server/src/engine/core-modules/user/crons/user-hard-delete.cron.job.ts Line: 36:36 Comment: **logic:** `hasMore` variable is set but never used - the while loop will run indefinitely ```suggestion while (hasMore) { ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-11-04T17:50:22Z

packages/twenty-server/src/engine/core-modules/user/crons/user-hard-delete.cron.job.ts

+      if (usersToDelete.length === 0) break;
+
+      await this.userRepository.delete({
+        id: In(usersToDelete.map((user) => user.id)),
+      });
+      totalDeleted += usersToDelete.length;


logic: hasMore should be updated after determining batch size

Suggested change

if (usersToDelete.length === 0) break;

await this.userRepository.delete({

id: In(usersToDelete.map((user) => user.id)),

});

totalDeleted += usersToDelete.length;

if (usersToDelete.length === 0) {

hasMore = false;

break;

}

await this.userRepository.delete({

id: In(usersToDelete.map((user) => user.id)),

});

totalDeleted += usersToDelete.length;

hasMore = usersToDelete.length === BATCH_SIZE;

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-server/src/engine/core-modules/user/crons/user-hard-delete.cron.job.ts Line: 47:52 Comment: **logic:** `hasMore` should be updated after determining batch size ```suggestion if (usersToDelete.length === 0) { hasMore = false; break; } await this.userRepository.delete({ id: In(usersToDelete.map((user) => user.id)), }); totalDeleted += usersToDelete.length; hasMore = usersToDelete.length === BATCH_SIZE; ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-11-04T17:50:23Z

...c/database/commands/upgrade-version-command/1-11/1-11-clean-orphaned-role-targets.command.ts

+    if (this.hasRunOnce) {
+      this.logger.log('Skipping kanban field metadata id foreign key creation');
+
+      return;
+    }


logic: early return prevents command from processing multiple workspaces - only first workspace will be processed

Suggested change

if (this.hasRunOnce) {

this.logger.log('Skipping kanban field metadata id foreign key creation');

return;

}

const isDryRun = options.dryRun || false;

if (isDryRun && this.hasRunOnce) {

this.logger.log('Dry run already executed, skipping');

return;

}

if (!isDryRun && this.hasRunOnce) {

this.logger.log('Command already executed, skipping');

return;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-server/src/database/commands/upgrade-version-command/1-11/1-11-clean-orphaned-role-targets.command.ts Line: 35:39 Comment: **logic:** early return prevents command from processing multiple workspaces - only first workspace will be processed ```suggestion const isDryRun = options.dryRun || false; if (isDryRun && this.hasRunOnce) { this.logger.log('Dry run already executed, skipping'); return; } if (!isDryRun && this.hasRunOnce) { this.logger.log('Command already executed, skipping'); return; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-11-04T17:50:24Z

packages/twenty-front/src/modules/settings/profile/components/DeleteAccount.tsx

+  const leaveWorkspace = async () => {
+    await deleteUserFromWorkspace();
+    await signOut();
+  };


logic: missing required argument workspaceMemberIdToDelete for mutation - backend expects this parameter but frontend doesn't provide it

Check backend mutation signature:

deleteUserFromWorkspace( @Args('workspaceMemberIdToDelete') workspaceMemberIdToDelete: string, ... )

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/twenty-front/src/modules/settings/profile/components/DeleteAccount.tsx Line: 45:48 Comment: **logic:** missing required argument `workspaceMemberIdToDelete` for mutation - backend expects this parameter but frontend doesn't provide it Check backend mutation signature: ```typescript deleteUserFromWorkspace( @Args('workspaceMemberIdToDelete') workspaceMemberIdToDelete: string, ... ) ``` How can I resolve this? If you propose a fix, please make it concise.

github-actions · 2025-11-04T17:56:36Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:37301

This environment will automatically shut down when the PR is closed or after 5 hours.

ijreilly · 2025-11-04T18:22:34Z

...ages/twenty-server/src/engine/twenty-orm/utils/format-column-name-for-relation-field.util.ts

  fieldName: string,
  fieldMetadataSettings: FieldMetadataRelationSettings,
 ): string => {
+  if (fieldMetadataSettings.relationType === RelationType.ONE_TO_MANY) {


forgotten from another pr

Weiko

Left some comments otherwise LGTM

...c/database/commands/upgrade-version-command/1-11/1-11-clean-orphaned-role-targets.command.ts

packages/twenty-server/src/engine/core-modules/user/services/user.service.ts

Weiko · 2025-11-05T13:26:33Z

packages/twenty-front/src/modules/object-record/hooks/useFindManyRecords.ts

  const hasReadPermission = objectPermissions.canReadObjectRecords;

-  const { data, loading, error, fetchMore } =
+  const { data, loading, error, fetchMore, refetch } =


I'm not sure about exposing the refetch for useFindMany for this new use case only 🤔 Could we use optimistic update / apollo cache mutation instead?

why not? by fear it would be use in the wrong context?

@lucasbordeau wdyt?

There's dedicated tooling for fetchMore and doing a refetch like that has not been QAed, so I would not expose this method.

ijreilly · 2025-11-05T14:29:06Z

@Bonapara @StephanieJoly4 @FelixMalfait
Do you agree with soft-deleted users being hard-deleted after 30 days (unlike today - they are never hard deleted), or should we keep them forever?

@etiennejouan @charlesBochet fyi

…tion

Weiko

LGTM

ijreilly added 8 commits October 31, 2025 15:42

unrelated but forgot to commit before

de2a2e7

replace cascade deletion

6d664a6

commands to clean

e96cca2

fixed commands

5be6594

Merge branch 'main' of github.com:twentyhq/twenty into fix--role-dele…

3380417

…tion

some updates

7c1c9ca

more updates

540d763

cron to clean deleteed users

fc4cb60

sentry bot reviewed Nov 4, 2025

View reviewed changes

packages/twenty-server/src/engine/core-modules/user/services/user.service.ts Outdated Show resolved Hide resolved

ijreilly mentioned this pull request Nov 4, 2025

Deleting a role gives a message Workspace not found #14608

Closed

greptile-apps bot reviewed Nov 4, 2025

View reviewed changes

ijreilly commented Nov 4, 2025

View reviewed changes

ijreilly added 2 commits November 5, 2025 10:15

fixes

e660481

fixes

e0ba131

Weiko self-requested a review November 5, 2025 10:12

Weiko reviewed Nov 5, 2025

View reviewed changes

ijreilly added 3 commits November 5, 2025 16:17

fix comment

79fb589

revert user hard deletion

6796d2f

Merge branch 'main' of github.com:twentyhq/twenty into fix--role-dele…

1428f18

…tion

prastoin assigned ijreilly Nov 5, 2025

Weiko approved these changes Nov 5, 2025

View reviewed changes

fix

b44844b

ijreilly enabled auto-merge (squash) November 5, 2025 17:54

ijreilly added 5 commits November 6, 2025 09:21

generate graphql

3f5bc25

fix

8943894

wip

66a4d5a

improve

c81a1b5

forbid access to deleteOneWorkspaceMember

279ada3

charlesBochet added the -PR: awaiting author label Nov 6, 2025

ijreilly added 2 commits November 6, 2025 16:46

fix test

5984f1e

tests

5575577

ijreilly merged commit 4ce93ae into main Nov 6, 2025
65 checks passed

ijreilly deleted the fix--role-deletion branch November 6, 2025 18:29

	this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(userWorkspace);
	await this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(userWorkspace);

-    if (this.hasRunOnce) {
-      this.logger.log('Skipping kanban field metadata id foreign key creation');
-      return;
-    }
+    const isDryRun = options.dryRun || false;
+    if (isDryRun && this.hasRunOnce) {
+      this.logger.log('Dry run already executed, skipping');
+      return;
+    }
+    if (!isDryRun && this.hasRunOnce) {
+      this.logger.log('Command already executed, skipping');
+      return;
+    }

Conversation

ijreilly commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 1/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ijreilly Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

Weiko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Weiko Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

ijreilly Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

ijreilly Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

lucasbordeau Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ijreilly commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Weiko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ijreilly commented Nov 4, 2025 •

edited

Loading

github-actions bot commented Nov 4, 2025 •

edited

Loading

lucasbordeau Nov 5, 2025 •

edited

Loading

ijreilly commented Nov 5, 2025 •

edited

Loading