From ec239f20f209927c4ded19a82a605bb90fe785d6 Mon Sep 17 00:00:00 2001 From: Constantin Graf Date: Wed, 29 May 2024 17:20:37 +0200 Subject: [PATCH] Added client to time entries --- .../Api/V1/TimeEntryController.php | 20 +- .../V1/TimeEntry/TimeEntryUpdateRequest.php | 2 - app/Models/TimeEntry.php | 12 ++ .../Importers/ClockifyTimeEntriesImporter.php | 1 + .../Importers/TogglTimeEntriesImporter.php | 1 + database/factories/TimeEntryFactory.php | 3 + ...26_add_client_id_to_time_entries_table.php | 43 ++++ database/seeders/DatabaseSeeder.php | 6 + .../Endpoint/Api/V1/TimeEntryEndpointTest.php | 193 ++++++++++++++++++ .../TimeEntryAggregationServiceTest.php | 98 ++++++++- 10 files changed, 372 insertions(+), 7 deletions(-) create mode 100644 database/migrations/2024_05_22_151226_add_client_id_to_time_entries_table.php diff --git a/app/Http/Controllers/Api/V1/TimeEntryController.php b/app/Http/Controllers/Api/V1/TimeEntryController.php index ce38ab74..cb5317c2 100644 --- a/app/Http/Controllers/Api/V1/TimeEntryController.php +++ b/app/Http/Controllers/Api/V1/TimeEntryController.php @@ -15,6 +15,7 @@ use App\Http\Resources\V1\TimeEntry\TimeEntryCollection; use App\Http\Resources\V1\TimeEntry\TimeEntryResource; use App\Models\Member; use App\Models\Organization; +use App\Models\Project; use App\Models\TimeEntry; use App\Service\TimeEntryAggregationService; use App\Service\TimeEntryFilter; @@ -214,8 +215,11 @@ class TimeEntryController extends Controller throw new TimeEntryStillRunningApiException(); } + $client = $request->get('project_id') !== null ? Project::findOrFail($request->get('project_id'))->client : null; + $timeEntry = new TimeEntry(); $timeEntry->fill($request->validated()); + $timeEntry->client()->associate($client); $timeEntry->user_id = $member->user_id; $timeEntry->description = $request->get('description') ?? ''; $timeEntry->organization()->associate($organization); @@ -246,6 +250,11 @@ class TimeEntryController extends Controller throw new TimeEntryCanNotBeRestartedApiException(); } + if ($request->has('project_id')) { + $client = $request->get('project_id') !== null ? Project::findOrFail($request->get('project_id'))->client : null; + $timeEntry->client()->associate($client); + } + $timeEntry->fill($request->validated()); $timeEntry->description = $request->get('description', $timeEntry->description) ?? ''; $timeEntry->save(); @@ -274,6 +283,13 @@ class TimeEntryController extends Controller throw new AuthorizationException(); } + $client = null; + $overwriteClient = false; + if ($request->has('changes.project_id')) { + $client = $request->input('changes.project_id') !== null ? Project::findOrFail($request->input('changes.project_id'))?->client : null; + $overwriteClient = true; + } + $success = new Collection(); $error = new Collection(); @@ -291,8 +307,10 @@ class TimeEntryController extends Controller continue; } - $timeEntry->fill($changes); + if ($overwriteClient) { + $timeEntry->client()->associate($client); + } $timeEntry->save(); $success->push($id); } diff --git a/app/Http/Requests/V1/TimeEntry/TimeEntryUpdateRequest.php b/app/Http/Requests/V1/TimeEntry/TimeEntryUpdateRequest.php index 6fd1e7e1..0f125cd2 100644 --- a/app/Http/Requests/V1/TimeEntry/TimeEntryUpdateRequest.php +++ b/app/Http/Requests/V1/TimeEntry/TimeEntryUpdateRequest.php @@ -64,12 +64,10 @@ class TimeEntryUpdateRequest extends FormRequest ], // Start of time entry (ISO 8601 format, UTC timezone) 'start' => [ - 'required', 'date_format:Y-m-d\TH:i:s\Z', ], // End of time entry (ISO 8601 format, UTC timezone) 'end' => [ - 'present', 'nullable', 'date_format:Y-m-d\TH:i:s\Z', 'after:start', diff --git a/app/Models/TimeEntry.php b/app/Models/TimeEntry.php index a2001d5a..4ac5b84b 100644 --- a/app/Models/TimeEntry.php +++ b/app/Models/TimeEntry.php @@ -31,6 +31,8 @@ use Korridor\LaravelComputedAttributes\ComputedAttributes; * @property-read Organization $organization * @property string|null $project_id * @property-read Project|null $project + * @property string|null $client_id + * @property-read Client|null $client * @property string|null $task_id * @property-read Task|null $task * @@ -124,4 +126,14 @@ class TimeEntry extends Model { return $this->belongsTo(Task::class, 'task_id'); } + + /** + * This relation can be reconstructed via the task relation. It is only here for performance reasons. + * + * @return BelongsTo + */ + public function client(): BelongsTo + { + return $this->belongsTo(Client::class, 'client_id'); + } } diff --git a/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php b/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php index 7f15e06c..bee9cf1c 100644 --- a/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php +++ b/app/Service/Import/Importers/ClockifyTimeEntriesImporter.php @@ -93,6 +93,7 @@ class ClockifyTimeEntriesImporter extends DefaultImporter $timeEntry->member_id = $memberId; $timeEntry->task_id = $taskId; $timeEntry->project_id = $projectId; + $timeEntry->client_id = $clientId; $timeEntry->organization_id = $this->organization->id; if (strlen($record['Description']) > 500) { throw new ImportException('Time entry description is too long'); diff --git a/app/Service/Import/Importers/TogglTimeEntriesImporter.php b/app/Service/Import/Importers/TogglTimeEntriesImporter.php index 13e43228..21b3f888 100644 --- a/app/Service/Import/Importers/TogglTimeEntriesImporter.php +++ b/app/Service/Import/Importers/TogglTimeEntriesImporter.php @@ -93,6 +93,7 @@ class TogglTimeEntriesImporter extends DefaultImporter $timeEntry->member_id = $memberId; $timeEntry->task_id = $taskId; $timeEntry->project_id = $projectId; + $timeEntry->client_id = $clientId; $timeEntry->organization_id = $this->organization->id; $timeEntry->description = $record['Description']; if (! in_array($record['Billable'], ['Yes', 'No'], true)) { diff --git a/database/factories/TimeEntryFactory.php b/database/factories/TimeEntryFactory.php index c565a78b..f875f25b 100644 --- a/database/factories/TimeEntryFactory.php +++ b/database/factories/TimeEntryFactory.php @@ -155,6 +155,7 @@ class TimeEntryFactory extends Factory return $this->state(function (array $attributes) use ($project) { return [ 'project_id' => $project?->getKey(), + 'client_id' => $project?->client_id, ]; }); } @@ -164,6 +165,8 @@ class TimeEntryFactory extends Factory return $this->state(function (array $attributes) use ($task) { return [ 'task_id' => $task?->getKey(), + 'project_id' => $task?->project?->getKey(), + 'client_id' => $task?->project?->client?->getKey(), ]; }); } diff --git a/database/migrations/2024_05_22_151226_add_client_id_to_time_entries_table.php b/database/migrations/2024_05_22_151226_add_client_id_to_time_entries_table.php new file mode 100644 index 00000000..db65130f --- /dev/null +++ b/database/migrations/2024_05_22_151226_add_client_id_to_time_entries_table.php @@ -0,0 +1,43 @@ +foreignUuid('client_id') + ->nullable() + ->constrained('clients') + ->cascadeOnDelete() + ->cascadeOnUpdate(); + }); + DB::statement(' + update time_entries + set client_id = clients.id + from projects + join clients on projects.client_id = clients.id + where time_entries.project_id = projects.id + '); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('time_entries', function (Blueprint $table) { + $table->dropForeign(['client_id']); + $table->dropColumn('client_id'); + }); + } +}; diff --git a/database/seeders/DatabaseSeeder.php b/database/seeders/DatabaseSeeder.php index b81a7333..1da89ff6 100644 --- a/database/seeders/DatabaseSeeder.php +++ b/database/seeders/DatabaseSeeder.php @@ -89,6 +89,12 @@ class DatabaseSeeder extends Seeder ProjectMember::factory()->forProject($bigCompanyProject)->forMember($userAcmeAdminMember)->create(); ProjectMember::factory()->forProject($bigCompanyProject)->forMember($userWithMultipleOrganizationsAcmeMember)->create(); + TimeEntry::factory() + ->count(3) + ->forMember($userAcmeEmployeeMember) + ->forProject($bigCompanyProject) + ->create(); + Task::factory()->forOrganization($organizationAcme)->forProject($bigCompanyProject)->create(); $internalProject = Project::factory()->forOrganization($organizationAcme)->create([ diff --git a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php index 7365950f..b9b44c49 100644 --- a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php @@ -6,6 +6,7 @@ namespace Tests\Unit\Endpoint\Api\V1; use App\Enums\Role; use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException; +use App\Models\Client; use App\Models\Member; use App\Models\Project; use App\Models\Tag; @@ -924,6 +925,36 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_store_endpoint_can_create_new_time_entry_with_project_and_automatically_set_client(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:create:own', + ]); + $client = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forOrganization($data->organization)->forClient($client)->create(); + $timeEntryFake = TimeEntry::factory()->forOrganization($data->organization)->make(); + Passport::actingAs($data->user); + + // Act + $response = $this->postJson(route('api.v1.time-entries.store', [$data->organization->getKey()]), [ + 'billable' => $timeEntryFake->billable, + 'start' => $timeEntryFake->start->toIso8601ZuluString(), + 'member_id' => $data->member->getKey(), + 'project_id' => $project->getKey(), + ]); + + // Assert + $response->assertStatus(201); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $response->json('data.id'), + 'member_id' => $data->member->getKey(), + 'task_id' => null, + 'project_id' => $project->getKey(), + 'client_id' => $client->getKey(), + ]); + } + public function test_store_endpoint_fails_if_user_has_no_permission_to_create_time_entries_for_others(): void { // Arrange @@ -1228,6 +1259,66 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_update_endpoint_can_update_project_and_automatically_set_client(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + $user = User::factory()->create(); + $client = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forOrganization($data->organization)->forClient($client)->create(); + $member = Member::factory()->forOrganization($data->organization)->forUser($user)->role(Role::Employee)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($member)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'project_id' => $project->getKey(), + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'member_id' => $member->getKey(), + 'task_id' => $timeEntry->task_id, + 'project_id' => $project->getKey(), + 'client_id' => $client->getKey(), + ]); + } + + public function test_update_endpoint_can_removed_project_from_time_entry_and_automatically_remove_client(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + $user = User::factory()->create(); + $client = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forOrganization($data->organization)->forClient($client)->create(); + $member = Member::factory()->forOrganization($data->organization)->forUser($user)->role(Role::Employee)->create(); + $timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($member)->forProject($project)->create(); + Passport::actingAs($data->user); + + // Act + $response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [ + 'project_id' => null, + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry->getKey(), + 'member_id' => $member->getKey(), + 'task_id' => null, + 'project_id' => null, + 'client_id' => null, + ]); + } + public function test_destroy_endpoint_fails_if_user_tries_to_delete_time_entry_in_organization_that_they_does_belong_to(): void { // Arrange @@ -1630,6 +1721,108 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_update_multiple_can_update_project_and_sets_client_automatically(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + + $oldClient = Client::factory()->forOrganization($data->organization)->create(); + $oldProject = Project::factory()->forOrganization($data->organization)->forClient($oldClient)->create(); + $timeEntry1 = TimeEntry::factory()->forMember($data->member)->forProject($oldProject)->create(); + $timeEntry2 = TimeEntry::factory()->forMember($data->member)->create(); + $client = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forClient($client)->forOrganization($data->organization)->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' => $project->getKey(), + ], + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $response->assertExactJson([ + 'success' => [ + $timeEntry1->getKey(), + $timeEntry2->getKey(), + ], + 'error' => [ + ], + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry1->getKey(), + 'client_id' => $client->getKey(), + 'project_id' => $project->getKey(), + 'task_id' => null, + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry2->getKey(), + 'client_id' => $client->getKey(), + 'project_id' => $project->getKey(), + 'task_id' => null, + ]); + } + + public function test_update_multiple_can_remove_project_from_time_entries_and_sets_client_automatically(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:update:all', + ]); + + $oldClient = Client::factory()->forOrganization($data->organization)->create(); + $oldProject = Project::factory()->forOrganization($data->organization)->forClient($oldClient)->create(); + $timeEntry1 = TimeEntry::factory()->forMember($data->member)->forProject($oldProject)->create(); + $timeEntry2 = TimeEntry::factory()->forMember($data->member)->create(); + $client = Client::factory()->forOrganization($data->organization)->create(); + $project = Project::factory()->forClient($client)->forOrganization($data->organization)->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' => null, + ], + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $response->assertExactJson([ + 'success' => [ + $timeEntry1->getKey(), + $timeEntry2->getKey(), + ], + 'error' => [ + ], + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry1->getKey(), + 'client_id' => null, + 'project_id' => null, + 'task_id' => null, + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $timeEntry2->getKey(), + 'client_id' => null, + 'project_id' => null, + 'task_id' => null, + ]); + } + public function test_update_multiple_updates_own_time_entries_fails_if_member_id_is_not_your_own_and_you_dont_have_update_all_permission(): void { // Arrange diff --git a/tests/Unit/Service/TimeEntryAggregationServiceTest.php b/tests/Unit/Service/TimeEntryAggregationServiceTest.php index 83d31e66..dd50d77f 100644 --- a/tests/Unit/Service/TimeEntryAggregationServiceTest.php +++ b/tests/Unit/Service/TimeEntryAggregationServiceTest.php @@ -6,6 +6,8 @@ namespace Tests\Unit\Service; use App\Enums\TimeEntryAggregationType; use App\Enums\Weekday; +use App\Models\Client; +use App\Models\Project; use App\Models\TimeEntry; use App\Service\TimeEntryAggregationService; use Illuminate\Support\Carbon; @@ -21,7 +23,7 @@ class TimeEntryAggregationServiceTest extends TestCaseWithDatabase $this->service = app(TimeEntryAggregationService::class); } - public function test_aggregate_time_entries_by_day_and_project_returns_empty_array_if_no_time_entries_given(): void + public function test_aggregate_time_entries_empty_state_by_day_and_project_returns_empty_array_if_no_time_entries_given(): void { // Arrange $query = TimeEntry::query(); @@ -47,7 +49,7 @@ class TimeEntryAggregationServiceTest extends TestCaseWithDatabase ], $result); } - public function test_aggregate_time_entries_by_day_and_project_with_filled_gaps(): void + public function test_aggregate_time_entries_empty_state_by_day_and_project_with_filled_gaps(): void { // Arrange $query = TimeEntry::query(); @@ -88,7 +90,7 @@ class TimeEntryAggregationServiceTest extends TestCaseWithDatabase ], $result); } - public function test_aggregate_time_entries_by_user_and_project_with_filled_gaps(): void + public function test_aggregate_time_entries_empty_state_by_user_and_project_with_filled_gaps(): void { // Arrange $query = TimeEntry::query(); @@ -114,7 +116,7 @@ class TimeEntryAggregationServiceTest extends TestCaseWithDatabase ], $result); } - public function test_aggregate_time_entries_by_user_and_day_with_filled_gaps(): void + public function test_aggregate_time_entries_empty_state_by_user_and_day_with_filled_gaps(): void { // Arrange $query = TimeEntry::query(); @@ -139,4 +141,92 @@ class TimeEntryAggregationServiceTest extends TestCaseWithDatabase 'grouped_data' => [], ], $result); } + + public function test_aggregate_time_entries_by_client_and_project(): void + { + // Arrange + $client1 = Client::factory()->create(); + $client2 = Client::factory()->create(); + $project1 = Project::factory()->forClient($client1)->create(); + $project2 = Project::factory()->forClient($client2)->create(); + $project3 = Project::factory()->create(); + $timeEntry1 = TimeEntry::factory()->startWithDuration(now(), 10)->forProject($project1)->create(); + $timeEntry2 = TimeEntry::factory()->startWithDuration(now(), 10)->forProject($project2)->create(); + $timeEntry3 = TimeEntry::factory()->startWithDuration(now(), 10)->forProject($project3)->create(); + $timeEntry4 = TimeEntry::factory()->startWithDuration(now(), 10)->create(); + $query = TimeEntry::query(); + + // Act + $result = $this->service->getAggregatedTimeEntries( + $query, + TimeEntryAggregationType::Client, + TimeEntryAggregationType::Project, + 'Europe/Vienna', + Weekday::Monday, + false, + null, + null + ); + + // Assert + $this->assertEqualsCanonicalizing([ + 'seconds' => 40, + 'cost' => 0, + 'grouped_type' => 'client', + 'grouped_data' => [ + [ + 'key' => null, + 'seconds' => 20, + 'cost' => 0, + 'grouped_type' => 'project', + 'grouped_data' => [ + [ + 'key' => null, + 'seconds' => 10, + 'cost' => 0, + 'grouped_type' => null, + 'grouped_data' => null, + ], + [ + 'key' => $project3->getKey(), + 'seconds' => 10, + 'cost' => 0, + 'grouped_type' => null, + 'grouped_data' => null, + ], + ], + ], + [ + 'key' => $client1->getKey(), + 'seconds' => 10, + 'cost' => 0, + 'grouped_type' => 'project', + 'grouped_data' => [ + [ + 'key' => $project1->getKey(), + 'seconds' => 10, + 'cost' => 0, + 'grouped_type' => null, + 'grouped_data' => null, + ], + ], + ], + [ + 'key' => $client2->getKey(), + 'seconds' => 10, + 'cost' => 0, + 'grouped_type' => 'project', + 'grouped_data' => [ + [ + 'key' => $project2->getKey(), + 'seconds' => 10, + 'cost' => 0, + 'grouped_type' => null, + 'grouped_data' => null, + ], + ], + ], + ], + ], $result); + } }