diff --git a/app/Http/Controllers/Api/V1/TimeEntryController.php b/app/Http/Controllers/Api/V1/TimeEntryController.php index 383e9ab9..3eb18c79 100644 --- a/app/Http/Controllers/Api/V1/TimeEntryController.php +++ b/app/Http/Controllers/Api/V1/TimeEntryController.php @@ -679,6 +679,10 @@ class TimeEntryController extends Controller $timeEntry->member()->associate($newMember); $timeEntry->user()->associate($newMember->user); } + // If project is changed, but task is not, we remove the old task from the time entry + if ($request->has('project_id') && ! $request->has('task_id') && $oldTask !== null && $oldTask->project_id !== $project?->getKey()) { + $timeEntry->task()->disassociate(); + } $timeEntry->description = $request->input('description', $timeEntry->description) ?? ''; $timeEntry->setComputedAttributeValue('billable_rate'); $timeEntry->save(); @@ -790,7 +794,7 @@ class TimeEntryController extends Controller $timeEntry->user_id = $newMember->user_id; } // If project is changed, but task is not, we remove the old task from the time entry - if ($oldProject !== null && $project !== null && $oldProject->isNot($project) && $task === null) { + if ($request->has('changes.project_id') && ! $request->has('changes.task_id') && $oldTask !== null && $oldTask->project_id !== $project?->getKey()) { $timeEntry->task()->disassociate(); } if ($overwriteClient) { diff --git a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php index 733693cb..8ba9b619 100644 --- a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php @@ -2780,6 +2780,90 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract }); } + public function test_update_endpoint_removes_task_if_project_is_changed_without_setting_a_new_task(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:own', + 'projects:view:all', + ]); + $project1 = Project::factory()->forOrganization($data->organization)->create(); + $project2 = Project::factory()->forOrganization($data->organization)->create(); + $task1 = Task::factory()->forProject($project1)->forOrganization($data->organization)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project1)->forTask($task1)->forMember($data->member)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'project_id' => $project2->getKey(), + ]); + + // Assert + $response->assertValid(); + $this->assertResponseCode($response, 200); + $response->assertJsonPath('data.project_id', $project2->getKey()); + $response->assertJsonPath('data.task_id', null); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'project_id' => $project2->getKey(), + 'task_id' => null, + ]); + } + + public function test_update_endpoint_removes_task_if_project_is_removed_without_removing_the_task(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:own', + 'projects:view:all', + ]); + $project = Project::factory()->forOrganization($data->organization)->create(); + $task = Task::factory()->forProject($project)->forOrganization($data->organization)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->forTask($task)->forMember($data->member)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'project_id' => null, + ]); + + // Assert + $response->assertValid(); + $this->assertResponseCode($response, 200); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'project_id' => null, + 'task_id' => null, + ]); + } + + public function test_update_endpoint_keeps_task_if_project_is_set_to_the_project_of_the_task(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:own', + 'projects:view:all', + ]); + $project = Project::factory()->forOrganization($data->organization)->create(); + $task = Task::factory()->forProject($project)->forOrganization($data->organization)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->forTask($task)->forMember($data->member)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'project_id' => $project->getKey(), + ]); + + // Assert + $response->assertValid(); + $this->assertResponseCode($response, 200); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'project_id' => $project->getKey(), + 'task_id' => $task->getKey(), + ]); + } + public function test_update_endpoint_fails_if_employee_tries_to_update_time_entry_to_private_project_without_access(): void { // Arrange @@ -3807,6 +3891,44 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_update_multiple_removes_task_from_time_entries_if_project_is_removed_without_removing_the_task(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:own', + 'projects:view:all', + ]); + $project = Project::factory()->forOrganization($data->organization)->create(); + $task = Task::factory()->forProject($project)->forOrganization($data->organization)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->forTask($task)->forMember($data->member)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [ + 'ids' => [ + $timeEntry->getKey(), + ], + 'changes' => [ + 'project_id' => null, + ], + ]); + + // Assert + $response->assertValid(); + $this->assertResponseCode($response, 200); + $response->assertExactJson([ + 'success' => [ + $timeEntry->getKey(), + ], + 'error' => [], + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'project_id' => null, + 'task_id' => null, + ]); + } + public function test_update_multiple_updates_own_time_entries_and_fails_for_time_entries_of_other_users_and_and_other_organizations_with_own_time_entries_permission(): void { // Arrange