Fixed bug in user delete feature

solidtime-io · Jun 17, 2024 · 8982bfa · 8982bfa
1 parent 9ac1d19
commit 8982bfa
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 6 deletions.
diff --git a/app/Service/DeletionService.php b/app/Service/DeletionService.php
@@ -29,7 +29,7 @@ public function __construct(UserService $userService)
         $this->userService = $userService;
     }
 
-    public function deleteOrganization(Organization $organization, bool $inTransaction = true): void
+    public function deleteOrganization(Organization $organization, bool $inTransaction = true, ?User $ignoreUser = null): void
     {
         if ($inTransaction) {
             DB::transaction(function () use ($organization) {
@@ -85,8 +85,11 @@ public function deleteOrganization(Organization $organization, bool $inTransacti
             ->get();
         $organization->users()->sync([]);
 
-        // Make sure all users have at least one organization
+        // Make sure all users have at least one organization and delete placeholders
         foreach ($users as $user) {
+            if ($ignoreUser !== null && $user->is($ignoreUser)) {
+                continue;
+            }
             if ($user->is_placeholder) {
                 $user->delete();
             } else {
@@ -140,9 +143,7 @@ public function deleteUser(User $user, bool $inTransaction = true): void
         /** @var Member $member */
         foreach ($members as $member) {
             if ($member->role === Role::Owner->value) {
-                // Note: The member needs to be deleted first, otherwise the organization delete function will recreate a new personal organization for the user
-                $member->delete();
-                $this->deleteOrganization($member->organization, false);
+                $this->deleteOrganization($member->organization, false, $user);
             } else {
                 $this->userService->makeMemberToPlaceholder($member);
             }

diff --git a/tests/Unit/Service/DeletionServiceTest.php b/tests/Unit/Service/DeletionServiceTest.php
@@ -224,7 +224,7 @@ public function test_delete_user_rolls_back_on_error_if_transaction_is_active():
         $memberOwner = Member::factory()->forUser($user)->forOrganization($organization)->role(Role::Owner)->create();
         $otherOrganization = Organization::factory()->create();
 
-        $brokenTimeEntry = TimeEntry::factory()->forOrganization($otherOrganization)->forMember($memberOwner)->create();
+        $brokenTimeEntry = TimeEntry::factory()->forMember($memberOwner)->forOrganization($otherOrganization)->create();
 
         // Act
         try {
@@ -310,6 +310,8 @@ public function test_delete_user_deletes_owned_organizations_that_have_only_one_
         $organizationNotOwned = Organization::factory()->create();
         $memberOwned = Member::factory()->forUser($user)->forOrganization($organizationOwned)->role(Role::Owner)->create();
         $memberNotOwned = Member::factory()->forUser($user)->forOrganization($organizationNotOwned)->role(Role::Employee)->create();
+        TimeEntry::factory()->forOrganization($organizationOwned)->forMember($memberOwned)->createMany(2);
+        TimeEntry::factory()->forOrganization($organizationNotOwned)->forMember($memberNotOwned)->createMany(2);
 
         // Act
         $this->deletionService->deleteUser($user);