From 8982bfac2b8e3eefefb789f4a0166b2186b0368e Mon Sep 17 00:00:00 2001
From: Constantin Graf <constantin@solidtime.io>
Date: Mon, 17 Jun 2024 12:50:39 +0200
Subject: [PATCH] Fixed bug in user delete feature

---
 app/Service/DeletionService.php            | 11 ++++++-----
 tests/Unit/Service/DeletionServiceTest.php |  4 +++-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/app/Service/DeletionService.php b/app/Service/DeletionService.php
index d06bd4e7..4492f70b 100644
--- 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
index cf207326..cab34f1c 100644
--- 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);