diff --git a/app/Exceptions/Api/TimeEntryMemberCanNotBeChangedWhileRunningApiException.php b/app/Exceptions/Api/TimeEntryMemberCanNotBeChangedWhileRunningApiException.php new file mode 100644 index 00000000..b39367d7 --- /dev/null +++ b/app/Exceptions/Api/TimeEntryMemberCanNotBeChangedWhileRunningApiException.php @@ -0,0 +1,15 @@ +has('end') ? ($request->input('end') !== null ? Carbon::parse($request->input('end')) : null) : $timeEntry->end; $this->assertNoOverlap($organization, $effectiveMember, $effectiveStart, $effectiveEnd, $timeEntry); + // The new member could already have a running time entry + if ($effectiveEnd === null && $newMember !== null && $newMember->getKey() !== $timeEntry->member_id) { + throw new TimeEntryMemberCanNotBeChangedWhileRunningApiException; + } + $oldProject = $timeEntry->project; $oldTask = $timeEntry->task; @@ -819,6 +825,12 @@ class TimeEntryController extends Controller continue; } + // The new member could already have a running time entry + if ($newMember !== null && $timeEntry->end === null && $newMember->getKey() !== $timeEntry->member_id) { + $error->push($id); + + continue; + } $oldProject = $timeEntry->project; $oldTask = $timeEntry->task; diff --git a/lang/en/exceptions.php b/lang/en/exceptions.php index 485eb145..110125b0 100644 --- a/lang/en/exceptions.php +++ b/lang/en/exceptions.php @@ -19,6 +19,7 @@ use App\Exceptions\Api\PdfRendererIsNotConfiguredException; use App\Exceptions\Api\PersonalAccessClientIsNotConfiguredException; use App\Exceptions\Api\ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException; use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException; +use App\Exceptions\Api\TimeEntryMemberCanNotBeChangedWhileRunningApiException; use App\Exceptions\Api\TimeEntryStillRunningApiException; use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException; use App\Exceptions\Api\UserIsAlreadyMemberOfProjectApiException; @@ -31,6 +32,7 @@ return [ TimeEntryStillRunningApiException::KEY => 'Time entry is still running', UserNotPlaceholderApiException::KEY => 'The given user is not a placeholder', TimeEntryCanNotBeRestartedApiException::KEY => 'Time entry is already stopped and can not be restarted', + TimeEntryMemberCanNotBeChangedWhileRunningApiException::KEY => 'The member of a running time entry can not be changed', InactiveUserCanNotBeUsedApiException::KEY => 'Inactive user can not be used', UserIsAlreadyMemberOfOrganizationApiException::KEY => 'User is already a member of the organization', UserIsAlreadyMemberOfProjectApiException::KEY => 'User is already a member of the project', diff --git a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php index 459d2d68..cfa79fff 100644 --- a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php @@ -3237,6 +3237,80 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_update_endpoint_fails_if_member_of_running_time_entry_is_changed(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + $otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->active()->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'member_id' => $otherMember->getKey(), + ]); + + // Assert + $response->assertStatus(400); + $response->assertJsonPath('key', 'time_entry_member_can_not_be_changed_while_running'); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'member_id' => $data->member->getKey(), + 'user_id' => $data->user->getKey(), + ]); + } + + public function test_update_endpoint_can_update_running_time_entry_with_unchanged_member_id(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:own', + ]); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->active()->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'member_id' => $data->member->getKey(), + 'description' => 'New description', + ]); + + // Assert + $response->assertStatus(200); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'description' => 'New description', + ]); + } + + public function test_update_endpoint_can_stop_running_time_entry_and_change_member_in_the_same_request(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + $otherUser = User::factory()->create(); + $otherMember = Member::factory()->forOrganization($data->organization)->forUser($otherUser)->role(Role::Employee)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->active()->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'member_id' => $otherMember->getKey(), + 'end' => Carbon::now()->toIso8601ZuluString(), + ]); + + // Assert + $response->assertStatus(200); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'member_id' => $otherMember->getKey(), + 'user_id' => $otherUser->getKey(), + ]); + } + public function test_update_endpoint_can_update_project_and_automatically_set_client(): void { // Arrange @@ -4104,6 +4178,51 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_update_multiple_does_not_change_member_of_running_time_entries(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + $otherMember = Member::factory()->forOrganization($data->organization)->role(Role::Employee)->create(); + $runningTimeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->active()->create(); + $runningTimeEntryOfOtherMember = TimeEntry::factory()->forOrganization($data->organization)->forMember($otherMember)->active()->create(); + $finishedTimeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [ + 'ids' => [ + $runningTimeEntry->getKey(), + $runningTimeEntryOfOtherMember->getKey(), + $finishedTimeEntry->getKey(), + ], + 'changes' => [ + 'member_id' => $otherMember->getKey(), + ], + ]); + + // Assert + $response->assertStatus(200); + $response->assertExactJson([ + 'success' => [ + $runningTimeEntryOfOtherMember->getKey(), + $finishedTimeEntry->getKey(), + ], + 'error' => [ + $runningTimeEntry->getKey(), + ], + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $runningTimeEntry->getKey(), + 'member_id' => $data->member->getKey(), + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $finishedTimeEntry->getKey(), + 'member_id' => $otherMember->getKey(), + ]); + } + public function test_update_multiple_updates_user_id_when_member_id_changes(): void { // Arrange diff --git a/tests/Unit/Exceptions/Api/ApiExceptionTest.php b/tests/Unit/Exceptions/Api/ApiExceptionTest.php index e52bff0d..c698913f 100644 --- a/tests/Unit/Exceptions/Api/ApiExceptionTest.php +++ b/tests/Unit/Exceptions/Api/ApiExceptionTest.php @@ -22,6 +22,7 @@ use App\Exceptions\Api\PdfRendererIsNotConfiguredException; use App\Exceptions\Api\PersonalAccessClientIsNotConfiguredException; use App\Exceptions\Api\ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException; use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException; +use App\Exceptions\Api\TimeEntryMemberCanNotBeChangedWhileRunningApiException; use App\Exceptions\Api\TimeEntryStillRunningApiException; use App\Exceptions\Api\UserIsAlreadyMemberOfOrganizationApiException; use App\Exceptions\Api\UserIsAlreadyMemberOfProjectApiException; @@ -53,6 +54,7 @@ class ApiExceptionTest extends TestCase yield OverlappingTimeEntryApiException::class => [new OverlappingTimeEntryApiException]; yield ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException::class => [new ThisPlaceholderCanNotBeInvitedUseTheMergeToolInsteadException]; yield TimeEntryCanNotBeRestartedApiException::class => [new TimeEntryCanNotBeRestartedApiException]; + yield TimeEntryMemberCanNotBeChangedWhileRunningApiException::class => [new TimeEntryMemberCanNotBeChangedWhileRunningApiException]; yield TimeEntryStillRunningApiException::class => [new TimeEntryStillRunningApiException]; yield UserIsAlreadyMemberOfOrganizationApiException::class => [new UserIsAlreadyMemberOfOrganizationApiException]; yield UserIsAlreadyMemberOfProjectApiException::class => [new UserIsAlreadyMemberOfProjectApiException];