Fixed bug in user delete feature

This commit is contained in:
Constantin Graf
2024-06-17 12:50:39 +02:00
parent 9ac1d19722
commit 8982bfac2b
2 changed files with 9 additions and 6 deletions

View File

@@ -29,7 +29,7 @@ class DeletionService
$this->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) { if ($inTransaction) {
DB::transaction(function () use ($organization) { DB::transaction(function () use ($organization) {
@@ -85,8 +85,11 @@ class DeletionService
->get(); ->get();
$organization->users()->sync([]); $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) { foreach ($users as $user) {
if ($ignoreUser !== null && $user->is($ignoreUser)) {
continue;
}
if ($user->is_placeholder) { if ($user->is_placeholder) {
$user->delete(); $user->delete();
} else { } else {
@@ -140,9 +143,7 @@ class DeletionService
/** @var Member $member */ /** @var Member $member */
foreach ($members as $member) { foreach ($members as $member) {
if ($member->role === Role::Owner->value) { 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 $this->deleteOrganization($member->organization, false, $user);
$member->delete();
$this->deleteOrganization($member->organization, false);
} else { } else {
$this->userService->makeMemberToPlaceholder($member); $this->userService->makeMemberToPlaceholder($member);
} }

View File

@@ -224,7 +224,7 @@ class DeletionServiceTest extends TestCaseWithDatabase
$memberOwner = Member::factory()->forUser($user)->forOrganization($organization)->role(Role::Owner)->create(); $memberOwner = Member::factory()->forUser($user)->forOrganization($organization)->role(Role::Owner)->create();
$otherOrganization = Organization::factory()->create(); $otherOrganization = Organization::factory()->create();
$brokenTimeEntry = TimeEntry::factory()->forOrganization($otherOrganization)->forMember($memberOwner)->create(); $brokenTimeEntry = TimeEntry::factory()->forMember($memberOwner)->forOrganization($otherOrganization)->create();
// Act // Act
try { try {
@@ -310,6 +310,8 @@ class DeletionServiceTest extends TestCaseWithDatabase
$organizationNotOwned = Organization::factory()->create(); $organizationNotOwned = Organization::factory()->create();
$memberOwned = Member::factory()->forUser($user)->forOrganization($organizationOwned)->role(Role::Owner)->create(); $memberOwned = Member::factory()->forUser($user)->forOrganization($organizationOwned)->role(Role::Owner)->create();
$memberNotOwned = Member::factory()->forUser($user)->forOrganization($organizationNotOwned)->role(Role::Employee)->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 // Act
$this->deletionService->deleteUser($user); $this->deletionService->deleteUser($user);