cloud/project-manager
3
0
Fork 0
Code Issues Pull Requests Actions Packages Releases Activity

De-duplicate controller boilerplate: owned-project lookup, reorder validation

Browse Source 
requireOwnedProjectId() was copy-pasted identically in CardController
and CardStatusController; ProjectController's own
requireOwnedProject() was the same lookup, just returning the full
row instead of the id. New abstract ProjectScopedController (extends
Controller) holds one copy of both, and all three controllers now
extend it instead of Controller directly, forwarding their
ProjectRepository to its constructor.

Separately, CardController::reorder() (card_ids) and
CardStatusController::reorder() (status_ids) each had the same inline
'must be an array of ids, no duplicates' check. Both now call a new
Validator::intIdArray(), which does the same shape check once.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
aneurin
2026-09-04 21:45:17 +01:00
co-authored by Claude Sonnet 5
parent accc6a273c
commit a85bb182c7
5 changed files with 95 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Http/Controllers/CardController.php
+7 -24
View File
@@ -18,15 +18,16 @@ use Psr\Http\Message\ServerRequestInterface as Request;
* ordering routes are addressed globally (by card id, or by an explicit
* project_id/status_id column) since a card need not have a project.
*/
final class CardController extends Controller
final class CardController extends ProjectScopedController
{
private const TEXT_MAX = 1000;
public function __construct(
private readonly ProjectRepository $projects,
ProjectRepository $projects,
private readonly CardRepository $cards,
private readonly CardStatusRepository $statuses,
) {
parent::__construct($projects);
}
/**
@@ -168,14 +169,10 @@ final class CardController extends Controller
[$projectId, $statusId] = $this->targetColumn($ownerId, $body);
$order = $body['card_ids'] ?? null;
if (!is_array($order) || array_filter($order, static fn ($id) => !is_int($id)) !== []) {
throw new ApiException('card_ids must be an array of card IDs.', 422);
}
/** @var int[] $order */
if (count($order) !== count(array_unique($order))) {
throw new ApiException('card_ids must not contain duplicates.', 422);
}
$validator = new Validator($body);
$order = $validator->intIdArray('card_ids');
$validator->assert();
if (array_diff($order, $this->cards->idsOwnedBy($ownerId)) !== []) {
throw new ApiException('Every card_id must be a card you own.', 422);
}
@@ -234,20 +231,6 @@ final class CardController extends Controller
return $this->statuses->allForProject($projectId)[0]['id'];
}
/**
* @param array<string, string> $args
*/
private function requireOwnedProjectId(Request $request, array $args): int
{
$project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']);
if ($project === null) {
throw new ApiException('Project not found.', 404);
}
return $project['id'];
}
/**
* @param array<string, string> $args
* @return array{id: int, owner_id: int, project_id: int|null, text: string, complete: bool, position: int, status_id: int|null, status: array{id: int, name: string}|null, created_at: string, updated_at: string}
src/Http/Controllers/CardStatusController.php
+6 -24
View File
@@ -18,15 +18,16 @@ use Psr\Http\Message\ServerRequestInterface as Request;
* one status, since a project card is required to have one (see the CHECK
* constraint on `cards`); deleting the last one is rejected.
*/
final class CardStatusController extends Controller
final class CardStatusController extends ProjectScopedController
{
private const NAME_MAX = 100;
public function __construct(
private readonly ProjectRepository $projects,
ProjectRepository $projects,
private readonly CardStatusRepository $statuses,
private readonly CardRepository $cards,
) {
parent::__construct($projects);
}
/**
@@ -68,14 +69,9 @@ final class CardStatusController extends Controller
$projectId = $this->requireOwnedProjectId($request, $args);
$body = $this->body($request);
$order = $body['status_ids'] ?? null;
if (!is_array($order) || array_filter($order, static fn ($id) => !is_int($id)) !== []) {
throw new ApiException('status_ids must be an array of status IDs.', 422);
}
/** @var int[] $order */
if (count($order) !== count(array_unique($order))) {
throw new ApiException('status_ids must not contain duplicates.', 422);
}
$validator = new Validator($body);
$order = $validator->intIdArray('status_ids');
$validator->assert();
$existingIds = array_column($this->statuses->allForProject($projectId), 'id');
if (array_diff($order, $existingIds) !== [] || array_diff($existingIds, $order) !== []) {
@@ -147,20 +143,6 @@ final class CardStatusController extends Controller
return $response->withStatus(204);
}
/**
* @param array<string, string> $args
*/
private function requireOwnedProjectId(Request $request, array $args): int
{
$project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']);
if ($project === null) {
throw new ApiException('Project not found.', 404);
}
return $project['id'];
}
/**
* @param array{id: int, project_id: int, name: string, position: int, created_at: string, updated_at: string} $status
* @return array<string, mixed>
src/Http/Controllers/ProjectController.php
+3 -20
View File
@@ -15,16 +15,17 @@ use Psr\Http\Message\ServerRequestInterface as Request;
* CRUD for the authenticated user's projects. A project is only ever visible to
* its owner; anything else responds 404.
*/
final class ProjectController extends Controller
final class ProjectController extends ProjectScopedController
{
private const TITLE_MAX = 255;
private const DESCRIPTION_MAX = 2000;
private const MAX_PROJECTS_PER_OWNER = 100;
public function __construct(
private readonly ProjectRepository $projects,
ProjectRepository $projects,
private readonly CardStatusRepository $statuses,
) {
parent::__construct($projects);
}
/**
@@ -105,24 +106,6 @@ final class ProjectController extends Controller
return $response->withStatus(204);
}
/**
* Load the project named in the route, or 404 if it is missing or not owned
* by the authenticated user.
*
* @param array<string, string> $args
* @return array{id: int, owner_id: int, title: string, description: string, card_count: int, completed_count: int, created_at: string, updated_at: string}
*/
private function requireOwnedProject(Request $request, array $args): array
{
$project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']);
if ($project === null) {
throw new ApiException('Project not found.', 404);
}
return $project;
}
/**
* @param array{id: int, owner_id: int, title: string, description: string, card_count: int, completed_count: int, created_at: string, updated_at: string} $project
* @return array<string, mixed>
src/Http/Controllers/ProjectScopedController.php
+47
View File
@@ -0,0 +1,47 @@
<?php
declare(strict_types=1);
namespace App\Http\Controllers;
use App\Exception\ApiException;
use App\Repository\ProjectRepository;
use Psr\Http\Message\ServerRequestInterface as Request;
/**
* Shared helper for controllers whose routes are nested under
* /projects/{projectId}: looking up the project named in the route, 404-ing
* if it's missing or not owned by the caller.
*/
abstract class ProjectScopedController extends Controller
{
public function __construct(protected readonly ProjectRepository $projects)
{
}
/**
* The project named in the route, or 404 if it is missing or not owned
* by the authenticated user.
*
* @param array<string, string> $args
* @return array{id: int, owner_id: int, title: string, description: string, card_count: int, completed_count: int, created_at: string, updated_at: string}
*/
protected function requireOwnedProject(Request $request, array $args): array
{
$project = $this->projects->findOwnedBy((int) $args['projectId'], $this->user($request)['id']);
if ($project === null) {
throw new ApiException('Project not found.', 404);
}
return $project;
}
/**
* @param array<string, string> $args
*/
protected function requireOwnedProjectId(Request $request, array $args): int
{
return $this->requireOwnedProject($request, $args)['id'];
}
}