Added usage check to delete endpoints

This commit is contained in:
Constantin Graf
2024-04-18 21:42:29 +02:00
committed by Constantin Graf
parent 40f1159ea5
commit ad6146c483
18 changed files with 326 additions and 6 deletions

View File

@@ -0,0 +1,33 @@
<?php
declare(strict_types=1);
namespace App\Exceptions\Api;
class EntityStillInUseApiException extends ApiException
{
private string $modelToDelete;
private string $modelInUse;
public function __construct(string $modelToDelete, string $modelInUse)
{
parent::__construct('', 0, null);
$this->modelToDelete = $modelToDelete;
$this->modelInUse = $modelInUse;
}
public const string KEY = 'entity_still_in_use';
/**
* Get the translated message for the exception.
*/
#[\Override]
public function getTranslatedMessage(): string
{
return __('exceptions.api.'.$this->getKey(), [
'modelToDelete' => __('validation.entities.'.$this->modelToDelete),
'modelInUse' => __('validation.entities.'.$this->modelInUse),
]);
}
}

View File

@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Http\Controllers\Api\V1;
use App\Exceptions\Api\EntityStillInUseApiException;
use App\Http\Requests\V1\Tag\TagStoreRequest;
use App\Http\Requests\V1\Tag\TagUpdateRequest;
use App\Http\Resources\V1\Client\ClientCollection;
@@ -83,7 +84,7 @@ class ClientController extends Controller
/**
* Delete client
*
* @throws AuthorizationException
* @throws AuthorizationException|EntityStillInUseApiException
*
* @operationId deleteClient
*/
@@ -91,6 +92,10 @@ class ClientController extends Controller
{
$this->checkPermission($organization, 'clients:delete', $client);
if ($client->projects()->exists()) {
throw new EntityStillInUseApiException('client', 'project');
}
$client->delete();
return response()->json(null, 204);

View File

@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Http\Controllers\Api\V1;
use App\Exceptions\Api\EntityStillInUseApiException;
use App\Exceptions\Api\UserNotPlaceholderApiException;
use App\Http\Requests\V1\Member\MemberIndexRequest;
use App\Http\Requests\V1\Member\MemberUpdateRequest;
@@ -12,6 +13,8 @@ use App\Http\Resources\V1\Member\MemberPivotResource;
use App\Http\Resources\V1\Member\MemberResource;
use App\Models\Membership;
use App\Models\Organization;
use App\Models\ProjectMember;
use App\Models\TimeEntry;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Request;
@@ -68,7 +71,7 @@ class MemberController extends Controller
/**
* Remove a member of the organization.
*
* @throws AuthorizationException
* @throws AuthorizationException|EntityStillInUseApiException
*
* @operationId removeMember
*/
@@ -76,6 +79,13 @@ class MemberController extends Controller
{
$this->checkPermission($organization, 'members:delete', $membership);
if (TimeEntry::query()->where('user_id', $membership->user_id)->whereBelongsTo($organization, 'organization')->exists()) {
throw new EntityStillInUseApiException('member', 'time_entry');
}
if (ProjectMember::query()->whereBelongsToOrganization($organization)->where('user_id', $membership->user_id)->exists()) {
throw new EntityStillInUseApiException('member', 'project_member');
}
$membership->delete();
return response()

View File

@@ -4,17 +4,20 @@ declare(strict_types=1);
namespace App\Http\Controllers\Api\V1;
use App\Exceptions\Api\EntityStillInUseApiException;
use App\Http\Requests\V1\Project\ProjectStoreRequest;
use App\Http\Requests\V1\Project\ProjectUpdateRequest;
use App\Http\Resources\V1\Project\ProjectCollection;
use App\Http\Resources\V1\Project\ProjectResource;
use App\Models\Organization;
use App\Models\Project;
use App\Models\ProjectMember;
use App\Models\User;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\Resources\Json\JsonResource;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\DB;
class ProjectController extends Controller
{
@@ -113,7 +116,7 @@ class ProjectController extends Controller
/**
* Delete project
*
* @throws AuthorizationException
* @throws AuthorizationException|EntityStillInUseApiException
*
* @operationId deleteProject
*/
@@ -121,7 +124,20 @@ class ProjectController extends Controller
{
$this->checkPermission($organization, 'projects:delete', $project);
if ($project->tasks()->exists()) {
throw new EntityStillInUseApiException('project', 'task');
}
if ($project->timeEntries()->exists()) {
throw new EntityStillInUseApiException('project', 'time_entry');
}
DB::transaction(function () use (&$project) {
$project->members()->each(function (ProjectMember $member) {
$member->delete();
});
$project->delete();
});
return response()
->json(null, 204);

View File

@@ -4,12 +4,14 @@ declare(strict_types=1);
namespace App\Http\Controllers\Api\V1;
use App\Exceptions\Api\EntityStillInUseApiException;
use App\Http\Requests\V1\Tag\TagStoreRequest;
use App\Http\Requests\V1\Tag\TagUpdateRequest;
use App\Http\Resources\V1\Tag\TagCollection;
use App\Http\Resources\V1\Tag\TagResource;
use App\Models\Organization;
use App\Models\Tag;
use App\Models\TimeEntry;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Http\JsonResponse;
@@ -83,7 +85,7 @@ class TagController extends Controller
/**
* Delete tag
*
* @throws AuthorizationException
* @throws AuthorizationException|EntityStillInUseApiException
*
* @operationId deleteTag
*/
@@ -91,6 +93,10 @@ class TagController extends Controller
{
$this->checkPermission($organization, 'tags:delete', $tag);
if (TimeEntry::query()->hasTag($tag)->whereBelongsTo($organization, 'organization')->exists()) {
throw new EntityStillInUseApiException('tag', 'time_entry');
}
$tag->delete();
return response()->json(null, 204);

View File

@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Http\Controllers\Api\V1;
use App\Exceptions\Api\EntityStillInUseApiException;
use App\Http\Requests\V1\Task\TaskIndexRequest;
use App\Http\Requests\V1\Task\TaskStoreRequest;
use App\Http\Requests\V1\Task\TaskUpdateRequest;
@@ -104,7 +105,7 @@ class TaskController extends Controller
/**
* Delete task
*
* @throws AuthorizationException
* @throws AuthorizationException|EntityStillInUseApiException
*
* @operationId deleteTask
*/
@@ -112,6 +113,10 @@ class TaskController extends Controller
{
$this->checkPermission($organization, 'tasks:delete', $task);
if ($task->timeEntries()->exists()) {
throw new EntityStillInUseApiException('task', 'time_entry');
}
$task->delete();
return response()

View File

@@ -74,6 +74,14 @@ class Project extends Model
return $this->hasMany(Task::class);
}
/**
* @return HasMany<TimeEntry>
*/
public function timeEntries(): HasMany
{
return $this->hasMany(TimeEntry::class, 'project_id');
}
/**
* @param Builder<Project> $builder
*/

View File

@@ -7,6 +7,7 @@ namespace App\Models;
use App\Service\BillableRateService;
use Carbon\CarbonInterval;
use Database\Factories\TimeEntryFactory;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Concerns\HasUuids;
use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Model;
@@ -31,6 +32,7 @@ use Korridor\LaravelComputedAttributes\ComputedAttributes;
* @property string|null $task_id
* @property-read Task|null $task
*
* @method Builder<TimeEntry> hasTag(Tag $tag)
* @method static TimeEntryFactory factory()
*/
class TimeEntry extends Model
@@ -73,6 +75,14 @@ class TimeEntry extends Model
return $this->end === null ? null : $this->start->diffAsCarbonInterval($this->end);
}
/**
* @param Builder<TimeEntry> $builder
*/
public function scopeHasTag(Builder $builder, Tag $tag): void
{
$builder->whereJsonContains('tags', $tag->getKey());
}
/**
* @return BelongsTo<User, TimeEntry>
*/

View File

@@ -149,6 +149,14 @@ class User extends Authenticatable implements FilamentUser, MustVerifyEmail
return $this->hasMany(TimeEntry::class);
}
/**
* @return HasMany<ProjectMember>
*/
public function projectMembers(): HasMany
{
return $this->hasMany(ProjectMember::class, 'user_id');
}
/**
* @param Builder<User> $builder
*/

View File

@@ -2,6 +2,7 @@
declare(strict_types=1);
use App\Exceptions\Api\EntityStillInUseApiException;
use App\Exceptions\Api\InactiveUserCanNotBeUsedApiException;
use App\Exceptions\Api\TimeEntryCanNotBeRestartedApiException;
use App\Exceptions\Api\TimeEntryStillRunningApiException;
@@ -15,5 +16,6 @@ return [
TimeEntryCanNotBeRestartedApiException::KEY => 'Time entry is already stopped and can not be restarted',
InactiveUserCanNotBeUsedApiException::KEY => 'Inactive user can not be used',
UserIsAlreadyMemberOfProjectApiException::KEY => 'User is already a member of the project',
EntityStillInUseApiException::KEY => 'The :modelToDelete is still used by a :modelInUse and can not be deleted.',
],
];

View File

@@ -202,4 +202,16 @@ return [
'currency' => 'The :attribute field must be a valid currency code (ISO 4217).',
'organization' => 'The :attribute does not exist.',
'task_belongs_to_project' => 'The :attribute is not part of the given project.',
'entities' => [
'organization' => 'organization',
'project' => 'project',
'task' => 'task',
'time_entry' => 'time entry',
'user' => 'user',
'client' => 'client',
'member' => 'member',
'project_member' => 'project member',
'tag' => 'tag',
],
];

View File

@@ -6,6 +6,7 @@ namespace Tests\Unit\Endpoint\Api\V1;
use App\Models\Client;
use App\Models\Organization;
use App\Models\Project;
use Illuminate\Testing\Fluent\AssertableJson;
use Laravel\Passport\Passport;
@@ -199,6 +200,27 @@ class ClientEndpointTest extends ApiEndpointTestAbstract
]);
}
public function test_destroy_endpoint_fails_if_client_is_still_in_use_by_project(): void
{
// Arrange
$data = $this->createUserWithPermission([
'clients:delete',
]);
$client = Client::factory()->forOrganization($data->organization)->create();
$project = Project::factory()->forOrganization($data->organization)->forClient($client)->create();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.clients.destroy', [$data->organization->getKey(), $client->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The client is still used by a project and can not be deleted.');
$this->assertDatabaseHas(Client::class, [
'id' => $client->getKey(),
]);
}
public function test_destroy_endpoint_deletes_client(): void
{
// Arrange

View File

@@ -6,6 +6,9 @@ namespace Tests\Unit\Endpoint\Api\V1;
use App\Models\Membership;
use App\Models\Organization;
use App\Models\Project;
use App\Models\ProjectMember;
use App\Models\TimeEntry;
use App\Models\User;
use Laravel\Passport\Passport;
@@ -154,6 +157,47 @@ class MemberEndpointTest extends ApiEndpointTestAbstract
$response->assertStatus(403);
}
public function test_destroy_endpoint_fails_if_member_is_still_in_use_by_a_time_entry(): void
{
// Arrange
$data = $this->createUserWithPermission([
'members:delete',
]);
TimeEntry::factory()->forUser($data->user)->forOrganization($data->organization)->create();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.members.destroy', [$data->organization->getKey(), $data->member->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The member is still used by a time entry and can not be deleted.');
$this->assertDatabaseHas(Membership::class, [
'id' => $data->member->getKey(),
]);
}
public function test_destroy_endpoint_fails_if_member_is_still_in_use_by_a_project_member(): void
{
// Arrange
$data = $this->createUserWithPermission([
'members:delete',
]);
$project = Project::factory()->forOrganization($data->organization)->create();
ProjectMember::factory()->forProject($project)->forUser($data->user)->create();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.members.destroy', [$data->organization->getKey(), $data->member->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The member is still used by a project member and can not be deleted.');
$this->assertDatabaseHas(Membership::class, [
'id' => $data->member->getKey(),
]);
}
public function test_destroy_member_succeeds_if_data_is_valid(): void
{
// Arrange

View File

@@ -7,6 +7,8 @@ namespace Tests\Unit\Endpoint\Api\V1;
use App\Models\Client;
use App\Models\Organization;
use App\Models\Project;
use App\Models\Task;
use App\Models\TimeEntry;
use Laravel\Passport\Passport;
class ProjectEndpointTest extends ApiEndpointTestAbstract
@@ -333,6 +335,48 @@ class ProjectEndpointTest extends ApiEndpointTestAbstract
$response->assertForbidden();
}
public function test_destroy_endpoint_fails_if_project_is_still_in_use_by_a_task(): void
{
// Arrange
$data = $this->createUserWithPermission([
'projects:delete',
]);
$project = Project::factory()->forOrganization($data->organization)->create();
$task = Task::factory()->forProject($project)->forOrganization($data->organization)->create();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.projects.destroy', [$data->organization->getKey(), $project->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The project is still used by a task and can not be deleted.');
$this->assertDatabaseHas(Project::class, [
'id' => $project->getKey(),
]);
}
public function test_destroy_endpoint_fails_if_project_is_still_in_use_by_a_time_entry(): void
{
// Arrange
$data = $this->createUserWithPermission([
'projects:delete',
]);
$project = Project::factory()->forOrganization($data->organization)->create();
$timeEntry = TimeEntry::factory()->forProject($project)->forOrganization($data->organization)->create();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.projects.destroy', [$data->organization->getKey(), $project->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The project is still used by a time entry and can not be deleted.');
$this->assertDatabaseHas(Project::class, [
'id' => $project->getKey(),
]);
}
public function test_destroy_endpoint_deletes_project(): void
{
// Arrange

View File

@@ -6,6 +6,7 @@ namespace Tests\Unit\Endpoint\Api\V1;
use App\Models\Organization;
use App\Models\Tag;
use App\Models\TimeEntry;
use Illuminate\Testing\Fluent\AssertableJson;
use Laravel\Passport\Passport;
@@ -199,6 +200,29 @@ class TagEndpointTest extends ApiEndpointTestAbstract
]);
}
public function test_destroy_endpoint_fails_if_tag_is_still_in_use_by_a_time_entry(): void
{
// Arrange
$data = $this->createUserWithPermission([
'tags:delete',
]);
$tag = Tag::factory()->forOrganization($data->organization)->create();
TimeEntry::factory()->forUser($data->user)->forOrganization($data->organization)->create([
'tags' => [$tag->getKey()],
]);
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.tags.destroy', [$data->organization->getKey(), $tag->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The tag is still used by a time entry and can not be deleted.');
$this->assertDatabaseHas(Tag::class, [
'id' => $tag->getKey(),
]);
}
public function test_destroy_endpoint_deletes_tag(): void
{
// Arrange

View File

@@ -7,6 +7,7 @@ namespace Tests\Unit\Endpoint\Api\V1;
use App\Models\Project;
use App\Models\ProjectMember;
use App\Models\Task;
use App\Models\TimeEntry;
use Laravel\Passport\Passport;
class TaskEndpointTest extends ApiEndpointTestAbstract
@@ -303,6 +304,27 @@ class TaskEndpointTest extends ApiEndpointTestAbstract
]);
}
public function test_destroy_endpoint_fails_if_task_is_still_in_use_by_a_time_entry(): void
{
// Arrange
$data = $this->createUserWithPermission([
'tasks:delete',
]);
$task = Task::factory()->forOrganization($data->organization)->create();
TimeEntry::factory()->forUser($data->user)->forTask($task)->forOrganization($data->organization)->create();
Passport::actingAs($data->user);
// Act
$response = $this->deleteJson(route('api.v1.tasks.destroy', [$data->organization->getKey(), $task->getKey()]));
// Assert
$response->assertStatus(400);
$response->assertJsonPath('message', 'The task is still used by a time entry and can not be deleted.');
$this->assertDatabaseHas(Task::class, [
'id' => $task->getKey(),
]);
}
public function test_delete_endpoint_fails_if_user_has_no_permission_to_delete_tasks(): void
{
// Arrange

View File

@@ -6,6 +6,7 @@ namespace Tests\Unit\Model;
use App\Models\Organization;
use App\Models\Project;
use App\Models\Tag;
use App\Models\Task;
use App\Models\TimeEntry;
use App\Models\User;
@@ -117,4 +118,30 @@ class TimeEntryModelTest extends ModelTestAbstract
'start' => '2021-01-01 13:00:00',
]);
}
public function test_scope_has_tag_filter_by_tag(): void
{
// Arrange
$tag1 = Tag::factory()->create();
$tag2 = Tag::factory()->create();
$timeEntry1 = TimeEntry::factory()->create([
'tags' => [$tag1->getKey()],
]);
$timeEntry2 = TimeEntry::factory()->create([
'tags' => [$tag2->getKey()],
]);
$timeEntry3 = TimeEntry::factory()->create([
'tags' => ['something-else'],
]);
$timeEntry4 = TimeEntry::factory()->create([
'tags' => null,
]);
// Act
$result = TimeEntry::hasTag($tag1)->get();
// Assert
$this->assertCount(1, $result);
$this->assertTrue($result->first()->is($timeEntry1));
}
}

View File

@@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Tests\Unit\Model;
use App\Models\Organization;
use App\Models\ProjectMember;
use App\Models\TimeEntry;
use App\Models\User;
use App\Providers\Filament\AdminPanelProvider;
@@ -92,6 +93,27 @@ class UserModelTest extends ModelTestAbstract
$this->assertTrue($timeEntriesRel->first()->is($timeEntries->first()));
}
public function test_it_has_many_project_members(): void
{
// Arrange
$user = User::factory()->create();
$otherUser = User::factory()->create();
$projectMembers = ProjectMember::factory()->forUser($user)->createMany(3);
$otherProjectMembers = ProjectMember::factory()->forUser($otherUser)->createMany(3);
// Act
$user->refresh();
$projectMembersRel = $user->projectMembers;
// Assert
$this->assertNotNull($projectMembersRel);
$this->assertCount(3, $projectMembersRel);
$this->assertEqualsCanonicalizing(
$projectMembers->pluck('id')->toArray(),
$projectMembersRel->pluck('id')->toArray()
);
}
public function test_scope_active_returns_only_non_placeholder_users(): void
{
// Arrange