Fix bug in time-entry.update-multiple; Add computed property for client_id

app/Http/Controllers/Api/V1/TimeEntryController.php

@@ -373,6 +373,10 @@ class TimeEntryController extends Controller
             $oldTask = $timeEntry->task;
 
             $timeEntry->fill($changes);
+            // 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) {
+                $timeEntry->task()->disassociate();
+            }
             if ($overwriteClient) {
                 $timeEntry->client()->associate($client);
             }

app/Models/TimeEntry.php

@@ -13,6 +13,7 @@ use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Support\Carbon;
 use Korridor\LaravelComputedAttributes\ComputedAttributes;
 use OwenIt\Auditing\Contracts\Auditable as AuditableContract;
@@ -79,6 +80,7 @@ class TimeEntry extends Model implements AuditableContract
      */
     protected array $computed = [
         'billable_rate',
+        'client_id',
     ];
 
     /**
@@ -95,6 +97,44 @@ class TimeEntry extends Model implements AuditableContract
         return app(BillableRateService::class)->getBillableRateForTimeEntry($this);
     }
 
+    public function getClientIdComputed(): ?string
+    {
+        return $this->project_id === null ? null : $this->project->client_id;
+    }
+
+    /**
+     * This scope will be applied during the computed property generation with artisan computed-attributes:generate.
+     *
+     * @param  Builder<TimeEntry>  $builder
+     * @param  array<string>  $attributes  Attributes that will be generated.
+     * @return Builder<TimeEntry>
+     */
+    public function scopeComputedAttributesGenerate(Builder $builder, array $attributes): Builder
+    {
+        if (in_array('client_id', $attributes, true)) {
+            $builder->with([
+                'project' => function (Relation $builder): void {
+                    /** @var Builder<Project> $builder */
+                    $builder->select('id', 'client_id');
+                },
+            ]);
+        }
+
+        return $builder;
+    }
+
+    /**
+     * This scope will be applied during the computed property validation with artisan computed-attributes:validate.
+     *
+     * @param  Builder<TimeEntry>  $builder
+     * @param  array<string>  $attributes  Attributes that will be validated.
+     * @return Builder<TimeEntry>
+     */
+    public function scopeComputedAttributesValidate(Builder $builder, array $attributes): Builder
+    {
+        return $this->scopeComputedAttributesGenerate($builder, $attributes);
+    }
+
     public function getDuration(): ?CarbonInterval
     {
         return $this->end === null ? null : $this->start->diffAsCarbonInterval($this->end);

tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php

@@ -1883,6 +1883,53 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
         $response->assertForbidden();
     }
 
+    public function test_update_multiple_remove_task_from_time_entries_only_if_project_is_set_to_a_new_value_without_setting_a_new_task(): void
+    {
+        // Arrange
+        $data = $this->createUserWithPermission([
+            'time-entries:update:own',
+        ]);
+        $project1 = Project::factory()->forOrganization($data->organization)->create();
+        $project2 = Project::factory()->forOrganization($data->organization)->create();
+        $task1 = Task::factory()->forProject($project1)->forOrganization($data->organization)->create();
+        $task2 = Task::factory()->forProject($project2)->forOrganization($data->organization)->create();
+        $timeEntry1 = TimeEntry::factory()->forOrganization($data->organization)->forProject($project1)->forTask($task1)->forMember($data->member)->create();
+        $timeEntry2 = TimeEntry::factory()->forOrganization($data->organization)->forProject($project2)->forTask($task2)->forMember($data->member)->create();
+        Passport::actingAs($data->user);
+
+        // Act
+        $response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
+            'ids' => [
+                $timeEntry1->getKey(),
+                $timeEntry2->getKey(),
+            ],
+            'changes' => [
+                'project_id' => $project2->getKey(),
+            ],
+        ]);
+
+        // Assert
+        $response->assertValid();
+        $response->assertStatus(200);
+        $response->assertExactJson([
+            'success' => [
+                $timeEntry1->getKey(),
+                $timeEntry2->getKey(),
+            ],
+            'error' => [],
+        ]);
+        $this->assertDatabaseHas(TimeEntry::class, [
+            'id' => $timeEntry1->getKey(),
+            'project_id' => $project2->getKey(),
+            'task_id' => null,
+        ]);
+        $this->assertDatabaseHas(TimeEntry::class, [
+            'id' => $timeEntry2->getKey(),
+            'project_id' => $project2->getKey(),
+            'task_id' => $task2->getKey(),
+        ]);
+    }
+
     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
@@ -2096,7 +2143,7 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
         Passport::actingAs($data->user);
 
         // Act
-        $response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
+        $response = $this->withoutExceptionHandling()->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
             'ids' => [
                 $ownTimeEntry->getKey(),
                 $otherTimeEntry->getKey(),

tests/Unit/Model/TimeEntryModelTest.php

@@ -148,4 +148,35 @@ class TimeEntryModelTest extends ModelTestAbstract
         $this->assertCount(1, $result);
         $this->assertTrue($result->first()->is($timeEntry1));
     }
+
+    public function test_computed_client_id_returns_null_when_no_project_is_assigned(): void
+    {
+        // Arrange
+        $timeEntry = TimeEntry::factory()->forProject(null)->create();
+        $timeEntry->client_id = null;
+        $timeEntry->save();
+
+        // Act
+        $timeEntry->setComputedAttributeValue('client_id');
+        $clientId = $timeEntry->client_id;
+
+        // Assert
+        $this->assertNull($clientId);
+    }
+
+    public function test_computed_client_id_returns_project_client_id(): void
+    {
+        // Arrange
+        $project = Project::factory()->create();
+        $timeEntry = TimeEntry::factory()->forProject($project)->create();
+        $timeEntry->client_id = null;
+        $timeEntry->save();
+
+        // Act
+        $timeEntry->setComputedAttributeValue('client_id');
+        $clientId = $timeEntry->client_id;
+
+        // Assert
+        $this->assertSame($project->client_id, $clientId);
+    }
 }