diff --git a/app/Http/Controllers/Api/V1/MemberController.php b/app/Http/Controllers/Api/V1/MemberController.php index 9a8e8876..484a02f5 100644 --- a/app/Http/Controllers/Api/V1/MemberController.php +++ b/app/Http/Controllers/Api/V1/MemberController.php @@ -16,6 +16,7 @@ use App\Exceptions\Api\OrganizationNeedsAtLeastOneOwner; use App\Exceptions\Api\ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException; use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException; use App\Exceptions\Api\UserNotPlaceholderApiException; +use App\Http\Requests\V1\Member\MemberDestroyRequest; use App\Http\Requests\V1\Member\MemberIndexRequest; use App\Http\Requests\V1\Member\MemberMergeIntoRequest; use App\Http\Requests\V1\Member\MemberUpdateRequest; @@ -100,11 +101,13 @@ class MemberController extends Controller * * @operationId removeMember */ - public function destroy(Organization $organization, Member $member, MemberService $memberService): JsonResponse + public function destroy(MemberDestroyRequest $request, Organization $organization, Member $member, MemberService $memberService): JsonResponse { $this->checkPermission($organization, 'members:delete', $member); - $memberService->removeMember($member, $organization); + $deleteRelated = $request->getDeleteRelated(); + + $memberService->removeMember($member, $organization, $deleteRelated); return response() ->json(null, 204); diff --git a/app/Http/Requests/V1/Member/MemberDestroyRequest.php b/app/Http/Requests/V1/Member/MemberDestroyRequest.php new file mode 100644 index 00000000..18231496 --- /dev/null +++ b/app/Http/Requests/V1/Member/MemberDestroyRequest.php @@ -0,0 +1,35 @@ +> + */ + public function rules(): array + { + return [ + 'delete_related' => [ + 'string', + 'in:true,false', + ], + ]; + } + + public function getDeleteRelated(): bool + { + return $this->input('delete_related', 'false') === 'true'; + } +} diff --git a/app/Service/MemberService.php b/app/Service/MemberService.php index d88660f7..3a45a12a 100644 --- a/app/Service/MemberService.php +++ b/app/Service/MemberService.php @@ -61,18 +61,24 @@ class MemberService * @throws CanNotRemoveOwnerFromOrganization * @throws EntityStillInUseApiException */ - public function removeMember(Member $member, Organization $organization): void + public function removeMember(Member $member, Organization $organization, bool $withRelations = false): void { - if (TimeEntry::query()->where('user_id', $member->user_id)->whereBelongsTo($organization, 'organization')->exists()) { - throw new EntityStillInUseApiException('member', 'time_entry'); - } - if (ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $member->user_id)->exists()) { - throw new EntityStillInUseApiException('member', 'project_member'); - } if ($member->role === Role::Owner->value) { throw new CanNotRemoveOwnerFromOrganization; } + if ($withRelations) { + TimeEntry::query()->where('user_id', $member->user_id)->whereBelongsTo($organization, 'organization')->delete(); + ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $member->user_id)->delete(); + } else { + if (TimeEntry::query()->where('user_id', $member->user_id)->whereBelongsTo($organization, 'organization')->exists()) { + throw new EntityStillInUseApiException('member', 'time_entry'); + } + if (ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $member->user_id)->exists()) { + throw new EntityStillInUseApiException('member', 'project_member'); + } + } + $member->delete(); MemberRemoved::dispatch($member, $organization); } diff --git a/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php b/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php index 011702af..dd157d6c 100644 --- a/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php @@ -653,6 +653,85 @@ class MemberEndpointTest extends ApiEndpointTestAbstract Event::assertNotDispatched(MemberRemoved::class); } + public function test_destroy_endpoint_succeeds_if_member_is_still_in_use_by_a_project_member_and_delete_related_is_active(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'members:delete', + ]); + $otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create(); + $project = Project::factory()->forOrganization($data->organization)->create(); + $projectMember = ProjectMember::factory()->forProject($project)->forMember($data->member)->create(); + $otherProjectMember = ProjectMember::factory()->forProject($project)->forMember($otherMember)->create(); + Passport::actingAs($data->user); + Event::fake([ + MemberRemoved::class, + ]); + + // Act + $response = $this->deleteJson(route('api.v1.members.destroy', [ + 'organization' => $data->organization->getKey(), + 'member' => $data->member->getKey(), + 'delete_related' => 'true', + ])); + + // Assert + $response->assertStatus(204); + $this->assertDatabaseMissing(Member::class, [ + 'id' => $data->member->getKey(), + ]); + $this->assertDatabaseHas(ProjectMember::class, [ + 'id' => $otherProjectMember->getKey(), + 'member_id' => $otherMember->getKey(), + 'user_id' => $otherMember->user_id, + ]); + $this->assertDatabaseMissing(ProjectMember::class, [ + 'id' => $projectMember->getKey(), + ]); + Event::assertDispatched(function (MemberRemoved $event) use ($data): bool { + return $event->organization->is($data->organization) && + $event->member->is($data->member); + }, 1); + } + + public function test_destroy_endpoint_succeeds_if_member_is_still_in_use_by_a_time_entry_and_delete_related_is_active(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'members:delete', + ]); + $otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create(); + $timeEntry = TimeEntry::factory()->forMember($data->member)->forOrganization($data->organization)->create(); + $otherTimeEntry = TimeEntry::factory()->forMember($otherMember)->forOrganization($data->organization)->create(); + Passport::actingAs($data->user); + Event::fake([ + MemberRemoved::class, + ]); + + // Act + $response = $this->deleteJson(route('api.v1.members.destroy', [ + 'organization' => $data->organization->getKey(), + 'member' => $data->member->getKey(), + 'delete_related' => 'true', + ])); + + // Assert + $response->assertStatus(204); + $this->assertDatabaseMissing(Member::class, [ + 'id' => $data->member->getKey(), + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $otherTimeEntry->getKey(), + ]); + $this->assertDatabaseMissing(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + ]); + Event::assertDispatched(function (MemberRemoved $event) use ($data): bool { + return $event->organization->is($data->organization) && + $event->member->is($data->member); + }, 1); + } + public function test_destroy_member_succeeds_if_data_is_valid(): void { // Arrange