Fix bugs in computed attribute calculation

solidtime-io · Oct 15, 2024 · 1b2bcb9 · 1b2bcb9
1 parent 62018ea
commit 1b2bcb9
Show file tree

Hide file tree

Showing 4 changed files with 156 additions and 1 deletion.
diff --git a/app/Http/Controllers/Api/V1/ProjectController.php b/app/Http/Controllers/Api/V1/ProjectController.php
@@ -14,6 +14,7 @@
 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 @@ public function update(Organization $organization, Project $project, ProjectUpda
             $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
@@ -450,6 +450,10 @@ public function destroyMultiple(Organization $organization, TimeEntryDestroyMult
         $ids = $request->validated('ids');
         $timeEntries = TimeEntry::query()
             ->whereBelongsTo($organization, 'organization')
+            ->with([
+                'project',
+                'task',
+            ])
             ->whereIn('id', $ids)
             ->get();
 
@@ -473,7 +477,17 @@ public function destroyMultiple(Organization $organization, TimeEntryDestroyMult
 
             }
 
+            $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
@@ -620,6 +620,85 @@ public function test_update_endpoint_fails_if_name_is_already_used_in_organizati
         ]);
     }
 
+    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
@@ -1821,6 +1821,57 @@ public function test_destroy_multiple_deletes_all_time_entries_and_fails_for_tim
         ]);
     }
 
+    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