diff --git a/app/Console/Kernel.php b/app/Console/Kernel.php index 285d5aec..8aaced22 100644 --- a/app/Console/Kernel.php +++ b/app/Console/Kernel.php @@ -28,7 +28,7 @@ class Kernel extends ConsoleKernel $schedule->command('self-host:database-consistency') ->when(fn (): bool => config('scheduling.tasks.self_hosting_database_consistency')) - ->twiceDaily(); + ->everySixHours(); } /** diff --git a/app/Exceptions/Api/InvitationForTheEmailAlreadyExistsApiException.php b/app/Exceptions/Api/InvitationForTheEmailAlreadyExistsApiException.php new file mode 100644 index 00000000..8e6de813 --- /dev/null +++ b/app/Exceptions/Api/InvitationForTheEmailAlreadyExistsApiException.php @@ -0,0 +1,10 @@ +whereBelongsTo($organization, 'organization')->whereBelongsTo($user, 'user')->first(); if ($member === null) { - Log::error('This function should only be called in authenticated context after checking the user is a member of the organization'); + Log::error('This function should only be called in authenticated context after checking the user is a member of the organization', [ + 'user' => $user->getKey(), + 'organization' => $organization->getKey(), + ]); throw new AuthorizationException; } diff --git a/app/Http/Requests/V1/Invitation/InvitationStoreRequest.php b/app/Http/Requests/V1/Invitation/InvitationStoreRequest.php index 1440c91d..19235bc0 100644 --- a/app/Http/Requests/V1/Invitation/InvitationStoreRequest.php +++ b/app/Http/Requests/V1/Invitation/InvitationStoreRequest.php @@ -7,11 +7,8 @@ namespace App\Http\Requests\V1\Invitation; use App\Enums\Role; use App\Http\Requests\V1\BaseFormRequest; use App\Models\Organization; -use App\Models\OrganizationInvitation; use Illuminate\Contracts\Validation\ValidationRule; -use Illuminate\Database\Eloquent\Builder; use Illuminate\Validation\Rule; -use Korridor\LaravelModelValidationRules\Rules\UniqueEloquent; /** * @property Organization $organization @@ -29,10 +26,6 @@ class InvitationStoreRequest extends BaseFormRequest 'email' => [ 'required', 'email', - UniqueEloquent::make(OrganizationInvitation::class, 'email', function (Builder $builder): Builder { - /** @var Builder $builder */ - return $builder->whereBelongsTo($this->organization, 'organization'); - })->withCustomTranslation('validation.invitation_already_exists'), ], 'role' => [ 'required', diff --git a/app/Service/InvitationService.php b/app/Service/InvitationService.php index e06f2278..4fc92c7a 100644 --- a/app/Service/InvitationService.php +++ b/app/Service/InvitationService.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Service; use App\Enums\Role; +use App\Exceptions\Api\InvitationForTheEmailAlreadyExistsApiException; use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException; use App\Mail\OrganizationInvitationMail; use App\Models\Member; @@ -16,7 +17,7 @@ use Laravel\Jetstream\Events\InvitingTeamMember; class InvitationService { /** - * @throws UserIsAlreadyMemberOfOrganizationApiException + * @throws UserIsAlreadyMemberOfOrganizationApiException|InvitationForTheEmailAlreadyExistsApiException */ public function inviteUser(Organization $organization, string $email, Role $role): OrganizationInvitation { @@ -28,6 +29,13 @@ class InvitationService throw new UserIsAlreadyMemberOfOrganizationApiException; } + if (OrganizationInvitation::query() + ->where('email', $email) + ->whereBelongsTo($organization, 'organization') + ->exists()) { + throw new InvitationForTheEmailAlreadyExistsApiException; + } + InvitingTeamMember::dispatch($organization, $email, $role->value); $invitation = new OrganizationInvitation; diff --git a/app/Service/MemberService.php b/app/Service/MemberService.php index 3a45a12a..e5c1b1ee 100644 --- a/app/Service/MemberService.php +++ b/app/Service/MemberService.php @@ -67,6 +67,14 @@ class MemberService throw new CanNotRemoveOwnerFromOrganization; } + $user = $member->user; + $isPlaceholder = $user->is_placeholder; + + if (! $isPlaceholder && $user->current_team_id === $member->organization_id) { + $user->currentTeam()->disassociate(); + $user->save(); + } + 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(); @@ -80,6 +88,14 @@ class MemberService } $member->delete(); + + if ($isPlaceholder) { + $user->delete(); + } else { + $this->userService->makeSureUserHasAtLeastOneOrganization($user); + $this->userService->makeSureUserHasCurrentOrganization($user); + } + MemberRemoved::dispatch($member, $organization); } diff --git a/lang/en/exceptions.php b/lang/en/exceptions.php index 832a9898..b5af171d 100644 --- a/lang/en/exceptions.php +++ b/lang/en/exceptions.php @@ -9,6 +9,7 @@ use App\Exceptions\Api\ChangingRoleToPlaceholderIsNotAllowed; use App\Exceptions\Api\EntityStillInUseApiException; use App\Exceptions\Api\FeatureIsNotAvailableInFreePlanApiException; use App\Exceptions\Api\InactiveUserCanNotBeUsedApiException; +use App\Exceptions\Api\InvitationForTheEmailAlreadyExistsApiException; use App\Exceptions\Api\OnlyOwnerCanChangeOwnership; use App\Exceptions\Api\OnlyPlaceholdersCanBeMergedIntoAnotherMember; use App\Exceptions\Api\OrganizationHasNoSubscriptionButMultipleMembersException; @@ -45,6 +46,7 @@ return [ ChangingRoleOfPlaceholderIsNotAllowed::KEY => 'Changing role of placeholder is not allowed', OnlyPlaceholdersCanBeMergedIntoAnotherMember::KEY => 'Only placeholders can be merged into another member', ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException::KEY => 'This placeholder can not be invited use the merge tool instead', + InvitationForTheEmailAlreadyExistsApiException::KEY => 'The email has already been invited to the organization. Please wait for the user to accept the invitation or resend the invitation email.', ], 'unknown_error_in_admin_panel' => 'An unknown error occurred. Please check the logs.', ]; diff --git a/tests/Unit/Endpoint/Api/V1/InvitationEndpointTest.php b/tests/Unit/Endpoint/Api/V1/InvitationEndpointTest.php index 7bf229f7..0aaa8e57 100644 --- a/tests/Unit/Endpoint/Api/V1/InvitationEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/InvitationEndpointTest.php @@ -129,26 +129,31 @@ class InvitationEndpointTest extends ApiEndpointTestAbstract $response->assertJsonPath('message', 'User is already a member of the organization'); } - public function test_store_fails_if_user_invites_user_who_is_already_invited_to_organization(): void + public function test_store_fails_if_an_invitation_with_the_same_email_already_exists(): void { // Arrange $data = $this->createUserWithPermission([ 'invitations:create', ]); Passport::actingAs($data->user); - $invitation = OrganizationInvitation::factory()->forOrganization($data->organization)->create(); + $email = 'user@email.test'; + $invitation = OrganizationInvitation::factory()->forOrganization($data->organization)->create([ + 'email' => $email, + ]); // Act $response = $this->postJson(route('api.v1.invitations.store', $data->organization->getKey()), [ - 'email' => $invitation->email, + 'email' => $email, 'role' => Role::Employee->value, ]); // Assert - $response->assertInvalid([ - 'email' => 'The email has already been invited to the organization. Please wait for the user to accept the invitation or resend the invitation email.', + $response->assertStatus(400); + $response->assertExactJson([ + 'error' => true, + 'key' => 'invitation_for_the_email_already_exists', + 'message' => 'The email has already been invited to the organization. Please wait for the user to accept the invitation or resend the invitation email.', ]); - $response->assertStatus(422); } public function test_store_works_if_user_invites_user_who_is_also_a_placeholder(): void diff --git a/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php b/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php index dd157d6c..7981dbc8 100644 --- a/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php @@ -10,6 +10,7 @@ use App\Events\MemberRemoved; use App\Http\Controllers\Api\V1\MemberController; use App\Models\Member; use App\Models\Organization; +use App\Models\OrganizationInvitation; use App\Models\Project; use App\Models\ProjectMember; use App\Models\TimeEntry; @@ -653,6 +654,103 @@ class MemberEndpointTest extends ApiEndpointTestAbstract Event::assertNotDispatched(MemberRemoved::class); } + public function test_destroy_endpoint_also_deletes_user_if_member_is_placeholder(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'members:delete', + ]); + $user = User::factory()->placeholder()->create(); + $member = Member::factory()->forUser($user)->forOrganization($data->organization)->role(Role::Placeholder)->create(); + Passport::actingAs($data->user); + Event::fake([ + MemberRemoved::class, + ]); + + // Act + $response = $this->deleteJson(route('api.v1.members.destroy', [$data->organization->getKey(), $member->getKey()])); + + // Assert + $response->assertStatus(204); + $this->assertDatabaseMissing(Member::class, [ + 'id' => $member->getKey(), + ]); + $this->assertDatabaseMissing(User::class, [ + 'id' => $user->getKey(), + ]); + Event::assertDispatched(function (MemberRemoved $event) use ($data, $member): bool { + return $event->organization->is($data->organization) && + $event->member->is($member); + }, 1); + } + + public function test_destroy_endpoint_sets_current_organization_to_organization_the_user_is_still_member_of(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'members:delete', + ]); + $user = $data->user; + $otherOrganization = Organization::factory()->create(); + $otherMember = Member::factory()->forOrganization($otherOrganization)->forUser($user)->role(Role::Employee)->create(); + Passport::actingAs($user); + Event::fake([ + MemberRemoved::class, + ]); + + // Act + $response = $this->deleteJson(route('api.v1.members.destroy', [$data->organization->getKey(), $data->member->getKey()])); + + // Assert + $response->assertStatus(204); + $this->assertDatabaseMissing(Member::class, [ + 'id' => $data->member->getKey(), + ]); + $user->refresh(); + $this->assertSame($otherOrganization->getKey(), $user->currentOrganization->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_creates_new_organization_and_sets_the_current_organization_to_it_if_user_is_not_member_of_any_other_organization(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'members:delete', + ]); + $organization = $data->organization; + $user = $data->user; + Passport::actingAs($user); + Event::fake([ + MemberRemoved::class, + ]); + $this->assertDatabaseCount(Organization::class, 1); + + // Act + $response = $this->deleteJson(route('api.v1.members.destroy', [$data->organization->getKey(), $data->member->getKey()])); + + // Assert + $response->assertStatus(204); + $this->assertDatabaseCount(Organization::class, 2); + $newOrganization = Organization::where('id', '!=', $organization->getKey())->first(); + $this->assertNotNull($newOrganization); + $this->assertDatabaseMissing(Member::class, [ + 'id' => $data->member->getKey(), + ]); + $this->assertDatabaseHas(Member::class, [ + 'organization_id' => $newOrganization->getKey(), + 'user_id' => $user->getKey(), + ]); + $user->refresh(); + $this->assertNotNull($user->currentOrganization); + 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_project_member_and_delete_related_is_active(): void { // Arrange @@ -937,6 +1035,37 @@ class MemberEndpointTest extends ApiEndpointTestAbstract $response->assertForbidden(); } + public function test_invite_placeholder_fails_if_there_is_already_an_invitation_with_the_same_email(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'members:invite-placeholder', + 'invitations:create', + ]); + $placeholder = User::factory()->placeholder()->create([ + 'email' => 'user@mail.test', + ]); + $placeholderMember = Member::factory()->forUser($placeholder)->forOrganization($data->organization)->role(Role::Placeholder)->create(); + OrganizationInvitation::factory()->forOrganization($data->organization)->create([ + 'email' => $placeholder->email, + ]); + Passport::actingAs($data->user); + + // Act + $response = $this->postJson(route('api.v1.members.invite-placeholder', [ + 'organization' => $data->organization->id, + 'member' => $placeholderMember->id, + ])); + + // Assert + $response->assertStatus(400); + $response->assertExactJson([ + 'error' => true, + 'key' => 'invitation_for_the_email_already_exists', + 'message' => 'The email has already been invited to the organization. Please wait for the user to accept the invitation or resend the invitation email.', + ]); + } + public function test_invite_placeholder_returns_400_if_user_is_not_placeholder(): void { // Arrange