From 071895791c2802ea626143655effcd97e92b38c2 Mon Sep 17 00:00:00 2001 From: Constantin Graf Date: Thu, 29 Aug 2024 17:21:48 +0200 Subject: [PATCH] Add endpoint to delete multiple time entries --- .../Api/V1/TimeEntryController.php | 49 ++++++ .../TimeEntryDestroyMultipleRequest.php | 34 +++++ routes/api.php | 1 + .../Endpoint/Api/V1/TimeEntryEndpointTest.php | 140 ++++++++++++++++++ 4 files changed, 224 insertions(+) create mode 100644 app/Http/Requests/V1/TimeEntry/TimeEntryDestroyMultipleRequest.php diff --git a/app/Http/Controllers/Api/V1/TimeEntryController.php b/app/Http/Controllers/Api/V1/TimeEntryController.php index 4487d942..1bd74d2d 100644 --- a/app/Http/Controllers/Api/V1/TimeEntryController.php +++ b/app/Http/Controllers/Api/V1/TimeEntryController.php @@ -7,6 +7,7 @@ namespace App\Http\Controllers\Api\V1; use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException; use App\Exceptions\Api\TimeEntryStillRunningApiException; use App\Http\Requests\V1\TimeEntry\TimeEntryAggregateRequest; +use App\Http\Requests\V1\TimeEntry\TimeEntryDestroyMultipleRequest; use App\Http\Requests\V1\TimeEntry\TimeEntryIndexRequest; use App\Http\Requests\V1\TimeEntry\TimeEntryStoreRequest; use App\Http\Requests\V1\TimeEntry\TimeEntryUpdateMultipleRequest; @@ -429,4 +430,52 @@ class TimeEntryController extends Controller return response() ->json(null, 204); } + + /** + * Delete multiple time entries + * + * @throws AuthorizationException + * + * @operationId deleteTimeEntries + */ + public function destroyMultiple(Organization $organization, TimeEntryDestroyMultipleRequest $request): JsonResponse + { + $this->checkAnyPermission($organization, ['time-entries:delete:all', 'time-entries:delete:own']); + $canDeleteAll = $this->hasPermission($organization, 'time-entries:delete:all'); + + $ids = $request->validated('ids'); + $timeEntries = TimeEntry::query() + ->whereBelongsTo($organization, 'organization') + ->whereIn('id', $ids) + ->get(); + + $success = new Collection(); + $error = new Collection(); + + foreach ($ids as $id) { + /** @var TimeEntry|null $timeEntry */ + $timeEntry = $timeEntries->firstWhere('id', $id); + if ($timeEntry === null) { + // Note: ID wrong or time entry in different organization + $error->push($id); + + continue; + } + + if (! $canDeleteAll && $timeEntry->user_id !== Auth::id()) { + $error->push($id); + + continue; + + } + + $timeEntry->delete(); + $success->push($id); + } + + return response()->json([ + 'success' => $success->toArray(), + 'error' => $error->toArray(), + ]); + } } diff --git a/app/Http/Requests/V1/TimeEntry/TimeEntryDestroyMultipleRequest.php b/app/Http/Requests/V1/TimeEntry/TimeEntryDestroyMultipleRequest.php new file mode 100644 index 00000000..40c4ab3c --- /dev/null +++ b/app/Http/Requests/V1/TimeEntry/TimeEntryDestroyMultipleRequest.php @@ -0,0 +1,34 @@ +> + */ + public function rules(): array + { + return [ + 'ids' => [ + 'required', + 'array', + ], + 'ids.*' => [ + 'string', + 'uuid', + ], + ]; + } +} diff --git a/routes/api.php b/routes/api.php index 6d8f6673..28ea0a41 100644 --- a/routes/api.php +++ b/routes/api.php @@ -92,6 +92,7 @@ Route::middleware([ Route::put('/organizations/{organization}/time-entries/{timeEntry}', [TimeEntryController::class, 'update'])->name('update')->middleware('check-organization-blocked'); Route::patch('/organizations/{organization}/time-entries', [TimeEntryController::class, 'updateMultiple'])->name('update-multiple')->middleware('check-organization-blocked'); Route::delete('/organizations/{organization}/time-entries/{timeEntry}', [TimeEntryController::class, 'destroy'])->name('destroy'); + Route::delete('/organizations/{organization}/time-entries', [TimeEntryController::class, 'destroyMultiple'])->name('destroy-multiple'); }); Route::name('users.time-entries.')->group(static function (): void { diff --git a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php index bb745a87..c5bc5884 100644 --- a/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php +++ b/tests/Unit/Endpoint/Api/V1/TimeEntryEndpointTest.php @@ -1660,6 +1660,146 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract ]); } + public function test_destroy_multiple_endpoint_fails_if_user_has_no_permission_to_delete_own_time_entries_or_all_time_entries(): void + { + // Arrange + $data = $this->createUserWithPermission(); + $timeEntries = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->createMany(3); + Passport::actingAs($data->user); + + // Act + $response = $this->deleteJson(route('api.v1.time-entries.destroy-multiple', [$data->organization->getKey()]), [ + 'ids' => $timeEntries->pluck('id')->toArray(), + ]); + + // Assert + $response->assertValid(); + $response->assertForbidden(); + } + + public function test_destroy_multiple_endpoint_fails_if_ids_contains_non_uuid_id(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:delete:own', + ]); + Passport::actingAs($data->user); + + // Act + $response = $this->deleteJson(route('api.v1.time-entries.destroy-multiple', [$data->organization->getKey()]), [ + 'ids' => [ + Str::uuid(), + 'non-uuid', + ], + ]); + + // Assert + $response->assertStatus(422); + $response->assertJsonValidationErrors([ + 'ids.1' => ['The ids.1 field must be a valid UUID.'], + ]); + } + + public function test_destroy_multiple_endpoint_own_time_entries_and_fails_for_time_entries_of_other_users_and_and_other_organizations_with_own_time_entries_permission(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:delete:own', + ]); + $otherData = $this->createUserWithPermission(); + $otherUser = User::factory()->create(); + $otherMember = Member::factory()->forOrganization($data->organization)->forUser($otherUser)->role(Role::Employee)->create(); + + $ownTimeEntry = TimeEntry::factory()->forMember($data->member)->create(); + $otherTimeEntry = TimeEntry::factory()->forMember($otherMember)->create(); + $otherOrganizationTimeEntry = TimeEntry::factory()->forMember($otherData->member)->create(); + $wrongId = Str::uuid(); + Passport::actingAs($data->user); + + // Act + $response = $this->deleteJson(route('api.v1.time-entries.destroy-multiple', [$data->organization->getKey()]), [ + 'ids' => [ + $ownTimeEntry->getKey(), + $otherTimeEntry->getKey(), + $otherOrganizationTimeEntry->getKey(), + $wrongId, + ], + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $response->assertExactJson([ + 'success' => [ + $ownTimeEntry->getKey(), + ], + 'error' => [ + $otherTimeEntry->getKey(), + $otherOrganizationTimeEntry->getKey(), + $wrongId, + ], + ]); + $this->assertDatabaseMissing(TimeEntry::class, [ + 'id' => $ownTimeEntry->getKey(), + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $otherTimeEntry->getKey(), + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $otherOrganizationTimeEntry->getKey(), + ]); + } + + public function test_destroy_multiple_deletes_all_time_entries_and_fails_for_time_entries_of_other_users_and_and_other_organizations_with_all_time_entries_permission(): void + { + // Arrange + $data = $this->createUserWithPermission([ + 'time-entries:delete:all', + ]); + $otherData = $this->createUserWithPermission(); + $otherUser = User::factory()->create(); + $otherMember = Member::factory()->forOrganization($data->organization)->forUser($otherUser)->role(Role::Employee)->create(); + + $ownTimeEntry = TimeEntry::factory()->forMember($data->member)->create(); + $otherTimeEntry = TimeEntry::factory()->forMember($otherMember)->create(); + $otherOrganizationTimeEntry = TimeEntry::factory()->forMember($otherData->member)->create(); + $wrongId = Str::uuid(); + Passport::actingAs($data->user); + + // Act + $response = $this->deleteJson(route('api.v1.time-entries.destroy-multiple', [$data->organization->getKey()]), [ + 'ids' => [ + $ownTimeEntry->getKey(), + $otherTimeEntry->getKey(), + $otherOrganizationTimeEntry->getKey(), + $wrongId, + ], + ]); + + // Assert + $response->assertValid(); + $response->assertStatus(200); + $response->assertExactJson([ + 'success' => [ + $ownTimeEntry->getKey(), + $otherTimeEntry->getKey(), + ], + 'error' => [ + $otherOrganizationTimeEntry->getKey(), + $wrongId, + ], + ]); + $this->assertDatabaseMissing(TimeEntry::class, [ + 'id' => $ownTimeEntry->getKey(), + ]); + $this->assertDatabaseMissing(TimeEntry::class, [ + 'id' => $otherTimeEntry->getKey(), + ]); + $this->assertDatabaseHas(TimeEntry::class, [ + 'id' => $otherOrganizationTimeEntry->getKey(), + ]); + } + public function test_destroy_endpoint_recalculates_project_and_task_spend_time_after_deleting_time_entry(): void { // Arrange