Removed option to update billable rate without updating time entries

solidtime-io · Jul 15, 2024 · 1dc35f1 · 1dc35f1
1 parent be50397
commit 1dc35f1
Show file tree

Hide file tree

Showing 14 changed files with 22 additions and 58 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,15 +523,15 @@ 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
         $response->assertStatus(200);
         $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
@@ -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