Added endpoint to update multiple time entries at once

This commit is contained in:
Constantin Graf
2024-05-19 12:05:08 +02:00
parent b6f5f77781
commit 2e0db46fff
6 changed files with 542 additions and 3 deletions

View File

@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Http\Controllers\Api\V1;
use App\Models\Member;
use App\Models\Organization;
use App\Models\User;
use App\Service\PermissionStore;
@@ -28,6 +29,21 @@ class Controller extends \App\Http\Controllers\Controller
}
}
/**
* @param array<string> $permissions
*
* @throws AuthorizationException
*/
protected function checkAnyPermission(Organization $organization, array $permissions): void
{
foreach ($permissions as $permission) {
if ($this->permissionStore->has($organization, $permission)) {
return;
}
}
throw new AuthorizationException();
}
protected function hasPermission(Organization $organization, string $permission): bool
{
return $this->permissionStore->has($organization, $permission);
@@ -47,4 +63,19 @@ class Controller extends \App\Http\Controllers\Controller
return $user;
}
/**
* @throws AuthorizationException
*/
protected function member(Organization $organization): Member
{
$user = $this->user();
$member = Member::query()->whereBelongsTo($organization, 'organization')->whereBelongsTo($user, 'user')->first();
if ($member === null) {
Log::error('This function should only be called in authenticated context after checking the user is a member of the organization');
throw new AuthorizationException();
}
return $member;
}
}

View File

@@ -10,6 +10,7 @@ use App\Exceptions\Api\TimeEntryStillRunningApiException;
use App\Http\Requests\V1\TimeEntry\TimeEntryAggregateRequest;
use App\Http\Requests\V1\TimeEntry\TimeEntryIndexRequest;
use App\Http\Requests\V1\TimeEntry\TimeEntryStoreRequest;
use App\Http\Requests\V1\TimeEntry\TimeEntryUpdateMultipleRequest;
use App\Http\Requests\V1\TimeEntry\TimeEntryUpdateRequest;
use App\Http\Resources\V1\TimeEntry\TimeEntryCollection;
use App\Http\Resources\V1\TimeEntry\TimeEntryResource;
@@ -359,6 +360,56 @@ class TimeEntryController extends Controller
return new TimeEntryResource($timeEntry);
}
/**
* @throws AuthorizationException
*/
public function updateMultiple(Organization $organization, TimeEntryUpdateMultipleRequest $request): JsonResponse
{
$this->checkAnyPermission($organization, ['time-entries:update:all', 'time-entries:update:own']);
$canAccessAll = $this->hasPermission($organization, 'time-entries:update:all');
$ids = $request->get('ids');
$timeEntries = TimeEntry::query()
->whereBelongsTo($organization, 'organization')
->whereIn('id', $ids)
->get();
$changes = $request->get('changes');
if (isset($changes['member_id']) && ! $canAccessAll && $this->member($organization)->getKey() !== $changes['member_id']) {
throw new AuthorizationException();
}
$success = new Collection();
$error = new Collection();
foreach ($ids as $id) {
$timeEntry = $timeEntries->firstWhere('id', $id);
if ($timeEntry === null) {
// Note: ID wrong or time entry in different organization
$error->push($id);
continue;
}
if (! $canAccessAll && $timeEntry->user_id !== Auth::id()) {
$error->push($id);
continue;
}
$timeEntry->fill($changes);
$timeEntry->save();
$success->push($id);
}
return response()->json([
'success' => $success->toArray(),
'error' => $error->toArray(),
]);
}
/**
* Delete time entry
*

View File

@@ -0,0 +1,102 @@
<?php
declare(strict_types=1);
namespace App\Http\Requests\V1\TimeEntry;
use App\Models\Member;
use App\Models\Organization;
use App\Models\Project;
use App\Models\Tag;
use App\Models\Task;
use Illuminate\Contracts\Validation\ValidationRule;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Foundation\Http\FormRequest;
use Korridor\LaravelModelValidationRules\Rules\ExistsEloquent;
/**
* @property Organization $organization Organization from model binding
*/
class TimeEntryUpdateMultipleRequest extends FormRequest
{
/**
* Get the validation rules that apply to the request.
*
* @return array<string, array<string|ValidationRule>>
*/
public function rules(): array
{
return [
'ids' => [
'required',
'array',
],
'ids.*' => [
'string',
'uuid',
],
'changes' => [
'required',
'array',
],
// ID of the organization member that the time entry should belong to
'changes.member_id' => [
'string',
'uuid',
new ExistsEloquent(Member::class, null, function (Builder $builder): Builder {
/** @var Builder<Member> $builder */
return $builder->whereBelongsTo($this->organization, 'organization');
}),
],
// ID of the project that the time entry should belong to
'changes.project_id' => [
'nullable',
'string',
'uuid',
'required_with:task_id',
new ExistsEloquent(Project::class, null, function (Builder $builder): Builder {
/** @var Builder<Project> $builder */
return $builder->whereBelongsTo($this->organization, 'organization');
}),
],
// ID of the task that the time entry should belong to
'changes.task_id' => [
'nullable',
'string',
'uuid',
new ExistsEloquent(Task::class, null, function (Builder $builder): Builder {
/** @var Builder<Task> $builder */
return $builder->whereBelongsTo($this->organization, 'organization');
}),
(new ExistsEloquent(Task::class, null, function (Builder $builder): Builder {
/** @var Builder<Task> $builder */
return $builder->whereBelongsTo($this->organization, 'organization')
->where('project_id', $this->input('changes.project_id'));
}))->withMessage(__('validation.task_belongs_to_project')),
],
// Whether time entry is billable
'changes.billable' => [
'boolean',
],
// Description of time entry
'changes.description' => [
'nullable',
'string',
'max:500',
],
// List of tag IDs
'changes.tags' => [
'nullable',
'array',
],
'changes.tags.*' => [
'string',
'uuid',
new ExistsEloquent(Tag::class, null, function (Builder $builder): Builder {
/** @var Builder<Tag> $builder */
return $builder->whereBelongsTo($this->organization, 'organization');
}),
],
];
}
}

View File

@@ -76,6 +76,7 @@ Route::middleware([
Route::get('/organizations/{organization}/time-entries/aggregate', [TimeEntryController::class, 'aggregate'])->name('aggregate');
Route::post('/organizations/{organization}/time-entries', [TimeEntryController::class, 'store'])->name('store');
Route::put('/organizations/{organization}/time-entries/{timeEntry}', [TimeEntryController::class, 'update'])->name('update');
Route::patch('/organizations/{organization}/time-entries', [TimeEntryController::class, 'updateMultiple'])->name('update-multiple');
Route::delete('/organizations/{organization}/time-entries/{timeEntry}', [TimeEntryController::class, 'destroy'])->name('destroy');
});

View File

@@ -8,6 +8,7 @@ use App\Models\Member;
use App\Models\Organization;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Str;
use Laravel\Jetstream\Jetstream;
use Tests\TestCase;
@@ -19,9 +20,10 @@ class ApiEndpointTestAbstract extends TestCase
* @param array<string> $permissions
* @return object{user: User, organization: Organization, member: Member}
*/
protected function createUserWithPermission(array $permissions, bool $isOwner = false): object
protected function createUserWithPermission(array $permissions = [], bool $isOwner = false): object
{
Jetstream::role('custom-test', 'Custom Test', $permissions)
$roleName = 'custom-test-'.Str::uuid();
Jetstream::role($roleName, 'Custom Test', $permissions)
->description('Role custom for testing');
$user = User::factory()->create();
if ($isOwner) {
@@ -30,7 +32,7 @@ class ApiEndpointTestAbstract extends TestCase
$organization = Organization::factory()->create();
}
$member = Member::factory()->forUser($user)->forOrganization($organization)->create([
'role' => 'custom-test',
'role' => $roleName,
]);
return (object) [

View File

@@ -5,8 +5,10 @@ declare(strict_types=1);
namespace Tests\Unit\Endpoint\Api\V1;
use App\Enums\Role;
use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException;
use App\Models\Member;
use App\Models\Project;
use App\Models\Task;
use App\Models\TimeEntry;
use App\Models\User;
use Carbon\Carbon;
@@ -860,6 +862,32 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
]);
}
public function test_update_endpoint_fails_if_user_tries_to_reactivate_a_time_entry(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:update:own',
]);
$timeEntry = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->create();
$timeEntryFake = TimeEntry::factory()->forOrganization($data->organization)->make();
Passport::actingAs($data->user);
// Act
$response = $this->putJson(route('api.v1.time-entries.update', [$data->organization->getKey(), $timeEntry->getKey()]), [
'description' => $timeEntryFake->description,
'start' => $timeEntryFake->start->toIso8601ZuluString(),
'end' => null,
'tags' => $timeEntryFake->tags,
'member_id' => $data->member->getKey(),
'task_id' => $timeEntryFake->task_id,
]);
// Assert
$response->assertStatus(400);
$response->assertJsonPath('error', true);
$response->assertJsonPath('message', __('exceptions.api.'.TimeEntryCanNotBeRestartedApiException::KEY));
}
public function test_update_endpoint_updates_time_entry_of_other_user_in_organization(): void
{
// Arrange
@@ -999,4 +1027,328 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
'id' => $timeEntry->getKey(),
]);
}
public function test_update_multiple_endpoint_fails_if_user_has_no_permission_to_update_own_time_entries_or_all_time_entries(): void
{
// Arrange
$data = $this->createUserWithPermission();
$timeEntries = TimeEntry::factory()->forOrganization($data->organization)->forMember($data->member)->createMany(3);
$timeEntriesFake = TimeEntry::factory()->forOrganization($data->organization)->make();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => $timeEntries->pluck('id')->toArray(),
'changes' => [
'description' => $timeEntriesFake->description,
],
]);
// Assert
$response->assertValid();
$response->assertForbidden();
}
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
$data = $this->createUserWithPermission([
'time-entries:update: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();
$timeEntriesFake = TimeEntry::factory()->forOrganization($data->organization)->make();
$wrongId = Str::uuid();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => [
$ownTimeEntry->getKey(),
$otherTimeEntry->getKey(),
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
'changes' => [
'description' => $timeEntriesFake->description,
],
]);
// Assert
$response->assertValid();
$response->assertStatus(200);
$response->assertExactJson([
'success' => [
$ownTimeEntry->getKey(),
],
'error' => [
$otherTimeEntry->getKey(),
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $ownTimeEntry->getKey(),
'description' => $timeEntriesFake->description,
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherOrganizationTimeEntry->getKey(),
'description' => $otherOrganizationTimeEntry->description,
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherTimeEntry->getKey(),
'description' => $otherTimeEntry->description,
]);
}
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_and_full_changeset(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:update: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();
$timeEntriesFake = TimeEntry::factory()->forOrganization($data->organization)->withTags($data->organization)->make();
$project = Project::factory()->forOrganization($data->organization)->create();
$task = Task::factory()->forProject($project)->forOrganization($data->organization)->create();
$wrongId = Str::uuid();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => [
$ownTimeEntry->getKey(),
$otherTimeEntry->getKey(),
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
'changes' => [
'member_id' => $data->member->getKey(),
'project_id' => $project->getKey(),
'task_id' => $task->getKey(),
'billable' => $timeEntriesFake->billable,
'description' => $timeEntriesFake->description,
'tags' => $timeEntriesFake->tags,
],
]);
// Assert
$response->assertValid();
$response->assertStatus(200);
$response->assertExactJson([
'success' => [
$ownTimeEntry->getKey(),
],
'error' => [
$otherTimeEntry->getKey(),
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $ownTimeEntry->getKey(),
'member_id' => $data->member->getKey(),
'project_id' => $project->getKey(),
'task_id' => $task->getKey(),
'billable' => $timeEntriesFake->billable,
'description' => $timeEntriesFake->description,
'tags' => json_encode($timeEntriesFake->tags),
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherOrganizationTimeEntry->getKey(),
'member_id' => $otherOrganizationTimeEntry->member_id,
'project_id' => $otherOrganizationTimeEntry->project_id,
'task_id' => $otherOrganizationTimeEntry->task_id,
'billable' => $otherOrganizationTimeEntry->billable,
'description' => $otherOrganizationTimeEntry->description,
'tags' => json_encode($otherOrganizationTimeEntry->tags),
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherTimeEntry->getKey(),
'member_id' => $otherTimeEntry->member_id,
'project_id' => $otherTimeEntry->project_id,
'task_id' => $otherTimeEntry->task_id,
'billable' => $otherTimeEntry->billable,
'description' => $otherTimeEntry->description,
'tags' => json_encode($otherTimeEntry->tags),
]);
}
public function test_update_multiple_updates_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:update: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();
$timeEntriesFake = TimeEntry::factory()->forOrganization($data->organization)->make();
$wrongId = Str::uuid();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => [
$ownTimeEntry->getKey(),
$otherTimeEntry->getKey(),
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
'changes' => [
'description' => $timeEntriesFake->description,
],
]);
// Assert
$response->assertValid();
$response->assertStatus(200);
$response->assertExactJson([
'success' => [
$ownTimeEntry->getKey(),
$otherTimeEntry->getKey(),
],
'error' => [
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $ownTimeEntry->getKey(),
'description' => $timeEntriesFake->description,
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherOrganizationTimeEntry->getKey(),
'description' => $otherOrganizationTimeEntry->description,
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherTimeEntry->getKey(),
'description' => $timeEntriesFake->description,
]);
}
public function test_update_multiple_updates_all_time_entries_and_fails_for_time_entries_of_other_users_and_and_other_organizations_with_all_time_entries_permission_and_full_changeset(): void
{
// Arrange
$data = $this->createUserWithPermission([
'time-entries:update: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();
$timeEntriesFake = TimeEntry::factory()->forOrganization($data->organization)->withTags($data->organization)->make();
$project = Project::factory()->forOrganization($data->organization)->create();
$task = Task::factory()->forProject($project)->forOrganization($data->organization)->create();
$wrongId = Str::uuid();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => [
$ownTimeEntry->getKey(),
$otherTimeEntry->getKey(),
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
'changes' => [
'member_id' => $otherMember->getKey(),
'project_id' => $project->getKey(),
'task_id' => $task->getKey(),
'billable' => $timeEntriesFake->billable,
'description' => $timeEntriesFake->description,
'tags' => $timeEntriesFake->tags,
],
]);
// Assert
$response->assertValid();
$response->assertStatus(200);
$response->assertExactJson([
'success' => [
$ownTimeEntry->getKey(),
$otherTimeEntry->getKey(),
],
'error' => [
$otherOrganizationTimeEntry->getKey(),
$wrongId,
],
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $ownTimeEntry->getKey(),
'member_id' => $otherMember->getKey(),
'project_id' => $project->getKey(),
'task_id' => $task->getKey(),
'billable' => $timeEntriesFake->billable,
'description' => $timeEntriesFake->description,
'tags' => json_encode($timeEntriesFake->tags),
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherOrganizationTimeEntry->getKey(),
'member_id' => $otherOrganizationTimeEntry->member_id,
'project_id' => $otherOrganizationTimeEntry->project_id,
'task_id' => $otherOrganizationTimeEntry->task_id,
'billable' => $otherOrganizationTimeEntry->billable,
'description' => $otherOrganizationTimeEntry->description,
'tags' => json_encode($otherOrganizationTimeEntry->tags),
]);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $otherTimeEntry->getKey(),
'member_id' => $otherMember->getKey(),
'project_id' => $project->getKey(),
'task_id' => $task->getKey(),
'billable' => $timeEntriesFake->billable,
'description' => $timeEntriesFake->description,
'tags' => json_encode($timeEntriesFake->tags),
]);
}
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
$data = $this->createUserWithPermission([
'time-entries:update:own',
]);
$otherUser = User::factory()->create();
$otherMember = Member::factory()->forOrganization($data->organization)->forUser($otherUser)->role(Role::Employee)->create();
$ownTimeEntry = TimeEntry::factory()->forMember($data->member)->create();
Passport::actingAs($data->user);
// Act
$response = $this->patchJson(route('api.v1.time-entries.update-multiple', [$data->organization->getKey()]), [
'ids' => [
$ownTimeEntry->getKey(),
],
'changes' => [
'member_id' => $otherMember->getKey(),
],
]);
// Assert
$response->assertValid();
$response->assertStatus(403);
$this->assertDatabaseHas(TimeEntry::class, [
'id' => $ownTimeEntry->getKey(),
'member_id' => $ownTimeEntry->member_id,
]);
}
}