From 4c27f1a2de0bcc4dc05d2aca87474c0f59e6571d Mon Sep 17 00:00:00 2001 From: Constantin Graf Date: Tue, 15 Oct 2024 11:00:14 +0200 Subject: [PATCH] Fix bugs in computed attribute calculation --- .../Controllers/Api/V1/ProjectController.php | 13 ++- .../Api/V1/TimeEntryController.php | 14 ++++ .../Endpoint/Api/V1/ProjectEndpointTest.php | 79 +++++++++++++++++++ .../Endpoint/Api/V1/TimeEntryEndpointTest.php | 51 ++++++++++++ 4 files changed, 156 insertions(+), 1 deletion(-) diff --git a/app/Http/Controllers/Api/V1/ProjectController.php b/app/Http/Controllers/Api/V1/ProjectController.php index 6977d9ec..0f9e5bb9 100644 --- a/app/Http/Controllers/Api/V1/ProjectController.php +++ b/app/Http/Controllers/Api/V1/ProjectController.php @@ -14,6 +14,7 @@ use App\Http\Resources\V1\Project\ProjectResource; use App\Models\Organization; use App\Models\Project; use App\Models\ProjectMember; +use App\Models\TimeEntry; use App\Service\BillableRateService; use Illuminate\Auth\Access\AuthorizationException; use Illuminate\Http\JsonResponse; @@ -130,13 +131,23 @@ class ProjectController extends Controller $project->estimated_time = $request->getEstimatedTime(); } $oldBillableRate = $project->billable_rate; + $clientIdChanged = false; $project->billable_rate = $request->getBillableRate(); - $project->client_id = $request->input('client_id'); + if ($project->client_id !== $request->input('client_id')) { + $project->client_id = $request->input('client_id'); + $clientIdChanged = true; + } $project->save(); if ($oldBillableRate !== $request->getBillableRate()) { $billableRateService->updateTimeEntriesBillableRateForProject($project); } + if ($clientIdChanged) { + TimeEntry::query() + ->whereBelongsTo($organization, 'organization') + ->whereBelongsTo($project, 'project') + ->update(['client_id' => $project->client_id]); + } return new ProjectResource($project, true); } diff --git a/app/Http/Controllers/Api/V1/TimeEntryController.php b/app/Http/Controllers/Api/V1/TimeEntryController.php index 2d6ad425..401f1a2e 100644 --- a/app/Http/Controllers/Api/V1/TimeEntryController.php +++ b/app/Http/Controllers/Api/V1/TimeEntryController.php @@ -450,6 +450,10 @@ class TimeEntryController extends Controller $ids = $request->validated('ids'); $timeEntries = TimeEntry::query() ->whereBelongsTo($organization, 'organization') + ->with([ + 'project', + 'task', + ]) ->whereIn('id', $ids) ->get(); @@ -473,7 +477,17 @@ class TimeEntryController extends Controller } + $project = $timeEntry->project; + $task = $timeEntry->task; + $timeEntry->delete(); + + if ($project !== null) { + RecalculateSpentTimeForProject::dispatch($project); + } + if ($task !== null) { + RecalculateSpentTimeForTask::dispatch($task); + } $success->push($id); } diff --git a/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php b/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php index cde9112a..ed7e413c 100644 --- a/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php @@ -620,6 +620,85 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_update_endpoint_updates_the_client_id_of_the_associated_time_entries_if_the_client_of_the_project_changed(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'projects:update', + ]); + $clientOld = Client::factory()->forOrganization($data->organization)->create(); + $clientNew = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forOrganization($data->organization)->forClient($clientOld)->create(); + $projectFake = Project::factory()->make(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [ + 'name' => $projectFake->name, + 'color' => $projectFake->color, + 'is_billable' => $projectFake->is_billable, + 'client_id' => $clientNew->getKey(), + ]); + + // Assert + $response->assertStatus(200); + $timeEntry->refresh(); + $this->assertSame($clientNew->getKey(), $timeEntry->client_id); + } + + public function test_update_endpoint_updates_the_client_id_of_the_associated_time_entries_if_the_client_of_the_project_is_removed(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'projects:update', + ]); + $client = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forOrganization($data->organization)->forClient($client)->create(); + $projectFake = Project::factory()->make(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [ + 'name' => $projectFake->name, + 'color' => $projectFake->color, + 'is_billable' => $projectFake->is_billable, + 'client_id' => null, + ]); + + // Assert + $response->assertStatus(200); + $timeEntry->refresh(); + $this->assertNull($timeEntry->client_id); + } + + public function test_update_endpoint_updates_the_client_id_of_the_associated_time_entries_if_the_client_of_the_project_is_added(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'projects:update', + ]); + $clientNew = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forOrganization($data->organization)->forClient(null)->create(); + $projectFake = Project::factory()->make(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.projects.update', [$data->organization->getKey(), $project->getKey()]), [ + 'name' => $projectFake->name, + 'color' => $projectFake->color, + 'is_billable' => $projectFake->is_billable, + 'client_id' => $clientNew->getKey(), + ]); + + // Assert + $response->assertStatus(200); + $timeEntry->refresh(); + $this->assertSame($clientNew->getKey(), $timeEntry->client_id); + } + public function test_update_endpoint_updates_project_if_name_is_used_in_other_organization(): void { // Arrange diff --git a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php index 0d0c53f9..0f8a57fa 100644 --- a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php @@ -1821,6 +1821,57 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_destroy_multiple_recalculates_project_and_task_spend_time_after_deleting_time_entries(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:delete:all', + ]); + $project = Project::factory()->forOrganization($data->organization)->create(); + $task = Task::factory()->forOrganization($data->organization)->create(); + $timeEntryWithProject = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->forMember($data->member)->create(); + $timeEntryWithTask = TimeEntry::factory()->forOrganization($data->organization)->forTask($task)->forMember($data->member)->create(); + $timeEntryWithProjectAndTask = TimeEntry::factory()->forOrganization($data->organization)->forProject($project)->forTask($task)->forMember($data->member)->create(); + $timeEntryWithoutProjectAndTask = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create(); + Passport::actingAs($data->user); + Queue::fake([ + RecalculateSpentTimeForProject::class, + RecalculateSpentTimeForTask::class, + ]); + + // Act + $response = $this->deleteJson(route('api.v1.time-entries.destroy-multiple', [$data->organization->getKey()]), [ + 'ids' => [ + $timeEntryWithProject->getKey(), + $timeEntryWithTask->getKey(), + $timeEntryWithProjectAndTask->getKey(), + $timeEntryWithoutProjectAndTask->getKey(), + ], + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $response->assertExactJson([ + 'success' => [ + $timeEntryWithProject->getKey(), + $timeEntryWithTask->getKey(), + $timeEntryWithProjectAndTask->getKey(), + $timeEntryWithoutProjectAndTask->getKey(), + ], + 'error' => [ + ], + ]); + Queue::assertPushed(RecalculateSpentTimeForProject::class, 3); + Queue::assertPushed(RecalculateSpentTimeForTask::class, 2); + Queue::assertPushed(RecalculateSpentTimeForProject::class, function (RecalculateSpentTimeForProject $job) use ($project) { + return $job->project->is($project); + }); + Queue::assertPushed(RecalculateSpentTimeForTask::class, function (RecalculateSpentTimeForTask $job) use ($task) { + return $job->task->is($task); + }); + } + public function test_destroy_endpoint_recalculates_project_and_task_spend_time_after_deleting_time_entry(): void { // Arrange