Skip to content

Commit

Permalink
Fix bug in time-entry.update-multiple; Add computed property for clie…
Browse files Browse the repository at this point in the history
…nt_id
  • Loading branch information
korridor committed Oct 8, 2024
1 parent b3b84db commit d5a4df7
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 1 deletion.
4 changes: 4 additions & 0 deletions app/Http/Controllers/Api/V1/TimeEntryController.php
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,10 @@ public function updateMultiple(Organization $organization, TimeEntryUpdateMultip
$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);
}
Expand Down
40 changes: 40 additions & 0 deletions app/Models/TimeEntry.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
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;
Expand Down Expand Up @@ -79,6 +80,7 @@ class TimeEntry extends Model implements AuditableContract
*/
protected array $computed = [
'billable_rate',
'client_id',
];

/**
Expand All @@ -95,6 +97,44 @@ public function getBillableRateComputed(): ?int
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);
Expand Down
49 changes: 48 additions & 1 deletion tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1883,6 +1883,53 @@ public function test_update_multiple_endpoint_fails_if_user_has_no_permission_to
$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
Expand Down Expand Up @@ -2096,7 +2143,7 @@ public function test_update_multiple_updates_all_time_entries_and_fails_for_time
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(),
Expand Down
31 changes: 31 additions & 0 deletions tests/Unit/Model/TimeEntryModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,35 @@ public function test_scope_has_tag_filter_by_tag(): void
$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);
}
}

0 comments on commit d5a4df7

Please sign in to comment.