From 1dc35f1f5550ef37df1da684d8f35b8c23132ad0 Mon Sep 17 00:00:00 2001
From: Constantin Graf <constantin@solidtime.io>
Date: Mon, 15 Jul 2024 14:01:33 +0200
Subject: [PATCH] Removed option to update billable rate without updating time
 entries

---
 app/Http/Controllers/Api/V1/MemberController.php         | 8 +++-----
 app/Http/Controllers/Api/V1/OrganizationController.php   | 3 ++-
 app/Http/Controllers/Api/V1/ProjectController.php        | 3 ++-
 app/Http/Controllers/Api/V1/ProjectMemberController.php  | 3 ++-
 app/Http/Controllers/Api/V1/TimeEntryController.php      | 1 +
 app/Http/Requests/V1/Member/MemberUpdateRequest.php      | 9 ---------
 .../V1/Organization/OrganizationUpdateRequest.php        | 9 ---------
 app/Http/Requests/V1/Project/ProjectUpdateRequest.php    | 9 ---------
 .../V1/ProjectMember/ProjectMemberUpdateRequest.php      | 9 ---------
 app/Jobs/Test/TestJob.php                                | 2 ++
 tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php        | 8 ++++----
 tests/Unit/Endpoint/Api/V1/OrganizationEndpointTest.php  | 1 -
 tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php       | 7 +++----
 tests/Unit/Endpoint/Api/V1/ProjectMemberEndpointTest.php | 8 +++-----
 14 files changed, 22 insertions(+), 58 deletions(-)

diff --git a/app/Http/Controllers/Api/V1/MemberController.php b/app/Http/Controllers/Api/V1/MemberController.php
index 49c63dcc..cde0bc71 100644
--- a/app/Http/Controllers/Api/V1/MemberController.php
+++ b/app/Http/Controllers/Api/V1/MemberController.php
@@ -71,8 +71,10 @@ public function update(Organization $organization, Member $member, MemberUpdateR
     {
         $this->checkPermission($organization, 'members:update', $member);
 
-        if ($request->has('billable_rate')) {
+        if ($request->has('billable_rate') && $member->billable_rate !== $request->getBillableRate()) {
             $member->billable_rate = $request->getBillableRate();
+
+            $billableRateService->updateTimeEntriesBillableRateForMember($member);
         }
         if ($request->has('role') && $member->role !== $request->getRole()->value) {
             $newRole = $request->getRole();
@@ -95,10 +97,6 @@ public function update(Organization $organization, Member $member, MemberUpdateR
         }
         $member->save();
 
-        if ($request->getBillableRateUpdateTimeEntries()) {
-            $billableRateService->updateTimeEntriesBillableRateForMember($member);
-        }
-
         return new MemberResource($member);
     }
 
diff --git a/app/Http/Controllers/Api/V1/OrganizationController.php b/app/Http/Controllers/Api/V1/OrganizationController.php
index 61e4e6fd..c1f628c8 100644
--- a/app/Http/Controllers/Api/V1/OrganizationController.php
+++ b/app/Http/Controllers/Api/V1/OrganizationController.php
@@ -38,10 +38,11 @@ public function update(Organization $organization, OrganizationUpdateRequest $re
         $this->checkPermission($organization, 'organizations:update');
 
         $organization->name = $request->input('name');
+        $oldBillableRate = $organization->billable_rate;
         $organization->billable_rate = $request->getBillableRate();
         $organization->save();
 
-        if ($request->getBillableRateUpdateTimeEntries()) {
+        if ($oldBillableRate !== $request->getBillableRate()) {
             $billableRateService->updateTimeEntriesBillableRateForOrganization($organization);
         }
 
diff --git a/app/Http/Controllers/Api/V1/ProjectController.php b/app/Http/Controllers/Api/V1/ProjectController.php
index 8e53c603..aa56df42 100644
--- a/app/Http/Controllers/Api/V1/ProjectController.php
+++ b/app/Http/Controllers/Api/V1/ProjectController.php
@@ -117,11 +117,12 @@ public function update(Organization $organization, Project $project, ProjectUpda
         if ($request->has('is_archived')) {
             $project->archived_at = $request->getIsArchived() ? Carbon::now() : null;
         }
+        $oldBillableRate = $project->billable_rate;
         $project->billable_rate = $request->getBillableRate();
         $project->client_id = $request->input('client_id');
         $project->save();
 
-        if ($request->getBillableRateUpdateTimeEntries()) {
+        if ($oldBillableRate !== $request->getBillableRate()) {
             $billableRateService->updateTimeEntriesBillableRateForProject($project);
         }
 
diff --git a/app/Http/Controllers/Api/V1/ProjectMemberController.php b/app/Http/Controllers/Api/V1/ProjectMemberController.php
index 804ef8df..0701fb8b 100644
--- a/app/Http/Controllers/Api/V1/ProjectMemberController.php
+++ b/app/Http/Controllers/Api/V1/ProjectMemberController.php
@@ -91,10 +91,11 @@ public function store(Organization $organization, Project $project, ProjectMembe
     public function update(Organization $organization, ProjectMember $projectMember, ProjectMemberUpdateRequest $request, BillableRateService $billableRateService): JsonResource
     {
         $this->checkPermission($organization, 'project-members:update', projectMember: $projectMember);
+        $oldBillableRate = $projectMember->billable_rate;
         $projectMember->billable_rate = $request->getBillableRate();
         $projectMember->save();
 
-        if ($request->getBillableRateUpdateTimeEntries()) {
+        if ($oldBillableRate !== $request->getBillableRate()) {
             $billableRateService->updateTimeEntriesBillableRateForProjectMember($projectMember);
         }
 
diff --git a/app/Http/Controllers/Api/V1/TimeEntryController.php b/app/Http/Controllers/Api/V1/TimeEntryController.php
index 2b49ecf4..db8d907a 100644
--- a/app/Http/Controllers/Api/V1/TimeEntryController.php
+++ b/app/Http/Controllers/Api/V1/TimeEntryController.php
@@ -257,6 +257,7 @@ public function update(Organization $organization, TimeEntry $timeEntry, TimeEnt
 
         $timeEntry->fill($request->validated());
         $timeEntry->description = $request->input('description', $timeEntry->description) ?? '';
+        $timeEntry->setComputedAttributeValue('billable_rate');
         $timeEntry->save();
 
         return new TimeEntryResource($timeEntry);
diff --git a/app/Http/Requests/V1/Member/MemberUpdateRequest.php b/app/Http/Requests/V1/Member/MemberUpdateRequest.php
index cf881f57..9f4ca23c 100644
--- a/app/Http/Requests/V1/Member/MemberUpdateRequest.php
+++ b/app/Http/Requests/V1/Member/MemberUpdateRequest.php
@@ -32,9 +32,6 @@ public function rules(): array
                 'integer',
                 'min:0',
             ],
-            'billable_rate_update_time_entries' => [
-                'boolean',
-            ],
         ];
     }
 
@@ -45,12 +42,6 @@ public function getBillableRate(): ?int
         return $input !== null && $input !== 0 ? (int) $this->input('billable_rate') : null;
     }
 
-    public function getBillableRateUpdateTimeEntries(): bool
-    {
-        return $this->has('billable_rate_update_time_entries') &&
-            $this->boolean('billable_rate_update_time_entries');
-    }
-
     public function getRole(): Role
     {
         return Role::from($this->input('role'));
diff --git a/app/Http/Requests/V1/Organization/OrganizationUpdateRequest.php b/app/Http/Requests/V1/Organization/OrganizationUpdateRequest.php
index 466b57dd..7c25fa37 100644
--- a/app/Http/Requests/V1/Organization/OrganizationUpdateRequest.php
+++ b/app/Http/Requests/V1/Organization/OrganizationUpdateRequest.php
@@ -31,9 +31,6 @@ public function rules(): array
                 'integer',
                 'min:0',
             ],
-            'billable_rate_update_time_entries' => [
-                'boolean',
-            ],
         ];
     }
 
@@ -43,10 +40,4 @@ public function getBillableRate(): ?int
 
         return $input !== null && $input !== 0 ? (int) $this->input('billable_rate') : null;
     }
-
-    public function getBillableRateUpdateTimeEntries(): bool
-    {
-        return $this->has('billable_rate_update_time_entries') &&
-            $this->boolean('billable_rate_update_time_entries');
-    }
 }
diff --git a/app/Http/Requests/V1/Project/ProjectUpdateRequest.php b/app/Http/Requests/V1/Project/ProjectUpdateRequest.php
index 4f9254b1..9c1dab7e 100644
--- a/app/Http/Requests/V1/Project/ProjectUpdateRequest.php
+++ b/app/Http/Requests/V1/Project/ProjectUpdateRequest.php
@@ -62,9 +62,6 @@ public function rules(): array
                 'integer',
                 'min:0',
             ],
-            'billable_rate_update_time_entries' => [
-                'boolean',
-            ],
         ];
     }
 
@@ -81,10 +78,4 @@ public function getBillableRate(): ?int
 
         return $input !== null && $input !== 0 ? (int) $this->input('billable_rate') : null;
     }
-
-    public function getBillableRateUpdateTimeEntries(): bool
-    {
-        return $this->has('billable_rate_update_time_entries') &&
-            $this->boolean('billable_rate_update_time_entries');
-    }
 }
diff --git a/app/Http/Requests/V1/ProjectMember/ProjectMemberUpdateRequest.php b/app/Http/Requests/V1/ProjectMember/ProjectMemberUpdateRequest.php
index 8c281728..664a2d56 100644
--- a/app/Http/Requests/V1/ProjectMember/ProjectMemberUpdateRequest.php
+++ b/app/Http/Requests/V1/ProjectMember/ProjectMemberUpdateRequest.php
@@ -26,9 +26,6 @@ public function rules(): array
                 'integer',
                 'min:0',
             ],
-            'billable_rate_update_time_entries' => [
-                'boolean',
-            ],
         ];
     }
 
@@ -38,10 +35,4 @@ public function getBillableRate(): ?int
 
         return $input !== null && $input !== 0 ? (int) $this->input('billable_rate') : null;
     }
-
-    public function getBillableRateUpdateTimeEntries(): bool
-    {
-        return $this->has('billable_rate_update_time_entries') &&
-            $this->boolean('billable_rate_update_time_entries');
-    }
 }
diff --git a/app/Jobs/Test/TestJob.php b/app/Jobs/Test/TestJob.php
index e93187d7..d04b38c9 100644
--- a/app/Jobs/Test/TestJob.php
+++ b/app/Jobs/Test/TestJob.php
@@ -23,6 +23,7 @@ class TestJob implements ShouldQueue
     private User $user;
 
     private string $message;
+
     private bool $fail;
 
     /**
@@ -37,6 +38,7 @@ public function __construct(User $user, string $message, bool $fail = false)
 
     /**
      * Execute the job.
+     *
      * @throws Exception
      */
     public function handle(): void
diff --git a/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php b/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php
index 029254b9..d25bd79b 100644
--- a/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php
+++ b/tests/Unit/Endpoint/Api/V1/MemberEndpointTest.php
@@ -91,20 +91,21 @@ public function test_update_member_succeeds_if_data_is_valid(): void
         $data = $this->createUserWithPermission([
             'members:update',
         ]);
-        $member = Member::factory()->forOrganization($data->organization)->role(Role::Admin)->create();
+        $member = Member::factory()->forOrganization($data->organization)->withBillableRate()->role(Role::Admin)->create();
         $this->assertBillableRateServiceIsUnused();
         Passport::actingAs($data->user);
 
         // Act
         $response = $this->putJson(route('api.v1.members.update', [$data->organization->id, $member]), [
-            'billable_rate' => 10001,
+            'billable_rate' => $member->billable_rate,
             'role' => Role::Employee->value,
         ]);
 
         // Assert
         $response->assertStatus(200);
+        $oldBillableRate = $member->billable_rate;
         $member->refresh();
-        $this->assertSame(10001, $member->billable_rate);
+        $this->assertSame($oldBillableRate, $member->billable_rate);
         $this->assertSame(Role::Employee->value, $member->role);
     }
 
@@ -124,7 +125,6 @@ public function test_update_member_can_update_billable_rate_of_member_and_update
         // Act
         $response = $this->putJson(route('api.v1.members.update', [$data->organization->getKey(), $data->member]), [
             'billable_rate' => 10001,
-            'billable_rate_update_time_entries' => true,
         ]);
 
         // Assert
diff --git a/tests/Unit/Endpoint/Api/V1/OrganizationEndpointTest.php b/tests/Unit/Endpoint/Api/V1/OrganizationEndpointTest.php
index c01cd7b7..8cd282ee 100644
--- a/tests/Unit/Endpoint/Api/V1/OrganizationEndpointTest.php
+++ b/tests/Unit/Endpoint/Api/V1/OrganizationEndpointTest.php
@@ -142,7 +142,6 @@ public function test_update_endpoint_can_update_billable_rate_of_organization_an
         $response = $this->withoutExceptionHandling()->putJson(route('api.v1.organizations.update', [$data->organization->getKey()]), [
             'name' => $organizationFake->name,
             'billable_rate' => $organizationFake->billable_rate,
-            'billable_rate_update_time_entries' => true,
         ]);
 
         // Assert
diff --git a/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php b/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php
index c5258802..f9978bac 100644
--- a/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php
+++ b/tests/Unit/Endpoint/Api/V1/ProjectEndpointTest.php
@@ -507,7 +507,7 @@ public function test_update_endpoint_updates_project(): void
         $this->assertFalse($project->is_archived);
     }
 
-    public function test_update_endpoint_can_update_projects_billable_rate(): void
+    public function test_update_endpoint_does_not_update_billable_rates_of_time_entries_if_billable_rate_is_unchanged(): void
     {
         // Arrange
         $data = $this->createUserWithPermission([
@@ -523,7 +523,7 @@ public function test_update_endpoint_can_update_projects_billable_rate(): void
             'name' => $projectFake->name,
             'color' => $projectFake->color,
             'is_billable' => $projectFake->is_billable,
-            'billable_rate' => 10002,
+            'billable_rate' => $project->billable_rate,
         ]);
 
         // Assert
@@ -531,7 +531,7 @@ public function test_update_endpoint_can_update_projects_billable_rate(): void
         $this->assertDatabaseHas(Project::class, [
             'name' => $projectFake->name,
             'color' => $projectFake->color,
-            'billable_rate' => 10002,
+            'billable_rate' => $project->billable_rate,
         ]);
     }
 
@@ -556,7 +556,6 @@ public function test_update_endpoint_can_update_projects_billable_rate_and_updat
             'color' => $projectFake->color,
             'is_billable' => $projectFake->is_billable,
             'billable_rate' => 10003,
-            'billable_rate_update_time_entries' => true,
         ]);
 
         // Assert
diff --git a/tests/Unit/Endpoint/Api/V1/ProjectMemberEndpointTest.php b/tests/Unit/Endpoint/Api/V1/ProjectMemberEndpointTest.php
index bb9153ef..f8a4f819 100644
--- a/tests/Unit/Endpoint/Api/V1/ProjectMemberEndpointTest.php
+++ b/tests/Unit/Endpoint/Api/V1/ProjectMemberEndpointTest.php
@@ -281,28 +281,27 @@ public function test_update_endpoint_fails_if_user_has_no_permission_to_update_p
         $response->assertForbidden();
     }
 
-    public function test_update_endpoint_updates_project_member(): void
+    public function test_update_endpoint_updates_project_member_with_unchanged_billable_rate(): void
     {
         // Arrange
         $data = $this->createUserWithPermission([
             'project-members:update',
         ]);
         $project = Project::factory()->forOrganization($data->organization)->create();
-        $billableRate = 1001;
         $projectMember = ProjectMember::factory()->forProject($project)->create();
         $this->assertBillableRateServiceIsUnused();
         Passport::actingAs($data->user);
 
         // Act
         $response = $this->putJson(route('api.v1.project-members.update', [$data->organization->getKey(), $projectMember->getKey()]), [
-            'billable_rate' => $billableRate,
+            'billable_rate' => $projectMember->billable_rate,
         ]);
 
         // Assert
         $response->assertStatus(200);
         $this->assertDatabaseHas(ProjectMember::class, [
             'id' => $projectMember->getKey(),
-            'billable_rate' => $billableRate,
+            'billable_rate' => $projectMember->billable_rate,
             'member_id' => $projectMember->member_id,
         ]);
     }
@@ -326,7 +325,6 @@ public function test_update_endpoints_can_update_billable_rate_and_update_time_e
         // Act
         $response = $this->putJson(route('api.v1.project-members.update', [$data->organization->getKey(), $projectMember->getKey()]), [
             'billable_rate' => $billableRate,
-            'billable_rate_update_time_entries' => true,
         ]);
 
         // Assert