refactor : simplify codebase and fix critical issues
Backend: - Add MCP Serializer to centralize entity-to-array conversion (~300 lines deduped) - Fix race condition in task/ticket number generation (SELECT FOR UPDATE + transaction) - Add unique constraint on task (project_id, number) with migration - Fix MIME type validation: use server-detected finfo instead of client-supplied type - Add allowlist of permitted MIME types for uploads - Fix TaskDocumentDownloadController: allow ROLE_CLIENT access, add priority:1 - Fix notification sent even when ticket status unchanged - Remove redundant exception constructors - Simplify services (BookStackApi double fetch, TokenEncryptor, GiteaApi) - Consolidate duplicate checks in processors Frontend: - Fix useApi isHandlingUnauthorized scope (module-level to prevent double 401 redirect) - Fix client-tickets toast key copy-paste bug - Merge duplicated tasks service methods (getByProject + getByProjectArchived) - Extract shared uploadWithRelation helper in task-documents service - Extract formatFileSize utility from duplicated component code - Extract status transition logic into useClientTicketHelpers composable - Remove dead code (unused router, handleLogout, empty script blocks) - Merge duplicate watchers and onMounted calls - Normalize arrow functions to function declarations per convention Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -51,12 +51,13 @@ final readonly class ClientTicketNumberProcessor implements ProcessorInterface
|
||||
}
|
||||
}
|
||||
|
||||
$nextNumber = $this->clientTicketRepository->findNextNumberForProject($project);
|
||||
$data->setNumber($nextNumber);
|
||||
$now = new DateTimeImmutable();
|
||||
|
||||
$data->setNumber($this->clientTicketRepository->findNextNumberForProjectForUpdate($project));
|
||||
$data->setSubmittedBy($user);
|
||||
$data->setStatus('new');
|
||||
$data->setCreatedAt(new DateTimeImmutable());
|
||||
$data->setUpdatedAt(new DateTimeImmutable());
|
||||
$data->setCreatedAt($now);
|
||||
$data->setUpdatedAt($now);
|
||||
|
||||
$this->entityManager->persist($data);
|
||||
$this->entityManager->flush();
|
||||
|
||||
@@ -54,17 +54,27 @@ final readonly class ClientTicketProvider implements ProviderInterface
|
||||
// Apply filters from query parameters
|
||||
$filters = $context['filters'] ?? [];
|
||||
if (isset($filters['project'])) {
|
||||
$projectId = is_numeric($filters['project']) ? (int) $filters['project'] : (int) basename($filters['project']);
|
||||
$qb->andWhere('ct.project = :project')->setParameter('project', $projectId);
|
||||
$qb->andWhere('ct.project = :project')
|
||||
->setParameter('project', self::extractId($filters['project']))
|
||||
;
|
||||
}
|
||||
if (isset($filters['status'])) {
|
||||
$qb->andWhere('ct.status = :status')->setParameter('status', $filters['status']);
|
||||
}
|
||||
if (isset($filters['submittedBy']) && $this->security->isGranted('ROLE_ADMIN')) {
|
||||
$submittedById = is_numeric($filters['submittedBy']) ? (int) $filters['submittedBy'] : (int) basename($filters['submittedBy']);
|
||||
$qb->andWhere('ct.submittedBy = :submittedBy')->setParameter('submittedBy', $submittedById);
|
||||
$qb->andWhere('ct.submittedBy = :submittedBy')
|
||||
->setParameter('submittedBy', self::extractId($filters['submittedBy']))
|
||||
;
|
||||
}
|
||||
|
||||
return $qb->getQuery()->getResult();
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract an entity ID from a value that may be a numeric ID or an IRI string.
|
||||
*/
|
||||
private static function extractId(string $value): int
|
||||
{
|
||||
return is_numeric($value) ? (int) $value : (int) basename($value);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,18 +35,21 @@ final readonly class ClientTicketStatusProcessor implements ProcessorInterface
|
||||
|
||||
$originalData = $context['previous_data'] ?? null;
|
||||
|
||||
// ROLE_CLIENT: can only edit content fields, not status
|
||||
if (!$this->security->isGranted('ROLE_ADMIN') && $originalData instanceof ClientTicket) {
|
||||
$data->setStatus($originalData->getStatus());
|
||||
$data->setStatusComment($originalData->getStatusComment());
|
||||
}
|
||||
$statusChanged = false;
|
||||
|
||||
if ($originalData instanceof ClientTicket) {
|
||||
// ROLE_CLIENT: can only edit content fields, not status
|
||||
if (!$this->security->isGranted('ROLE_ADMIN')) {
|
||||
$data->setStatus($originalData->getStatus());
|
||||
$data->setStatusComment($originalData->getStatusComment());
|
||||
}
|
||||
|
||||
$oldStatus = $originalData->getStatus();
|
||||
$newStatus = $data->getStatus();
|
||||
|
||||
if ($oldStatus !== $newStatus) {
|
||||
$forbidden = self::FORBIDDEN_TRANSITIONS[$oldStatus] ?? [];
|
||||
$statusChanged = true;
|
||||
$forbidden = self::FORBIDDEN_TRANSITIONS[$oldStatus] ?? [];
|
||||
if (in_array($newStatus, $forbidden, true)) {
|
||||
throw new BadRequestHttpException(sprintf('Transition from "%s" to "%s" is not allowed.', $oldStatus, $newStatus));
|
||||
}
|
||||
@@ -62,7 +65,9 @@ final readonly class ClientTicketStatusProcessor implements ProcessorInterface
|
||||
$this->entityManager->persist($data);
|
||||
$this->entityManager->flush();
|
||||
|
||||
$this->notificationService->createForStatusChange($data);
|
||||
if ($statusChanged) {
|
||||
$this->notificationService->createForStatusChange($data);
|
||||
}
|
||||
|
||||
return $data;
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
|
||||
|
||||
final readonly class GiteaBranchNameProvider implements ProviderInterface
|
||||
{
|
||||
/** @see GiteaBranchProcessor::ALLOWED_TYPES */
|
||||
private const array ALLOWED_TYPES = ['feature', 'fix', 'refactor', 'hotfix', 'chore'];
|
||||
|
||||
public function __construct(
|
||||
|
||||
@@ -24,6 +24,35 @@ final readonly class TaskDocumentProcessor implements ProcessorInterface
|
||||
{
|
||||
private const MAX_FILE_SIZE = 50 * 1024 * 1024; // 50 MB
|
||||
|
||||
private const ALLOWED_MIME_TYPES = [
|
||||
'image/jpeg', 'image/png', 'image/gif', 'image/webp', 'image/svg+xml',
|
||||
'application/pdf',
|
||||
'application/msword',
|
||||
'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
|
||||
'application/vnd.ms-excel',
|
||||
'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
'application/vnd.ms-powerpoint',
|
||||
'application/vnd.openxmlformats-officedocument.presentationml.presentation',
|
||||
'text/plain', 'text/csv',
|
||||
'application/zip', 'application/x-rar-compressed', 'application/gzip',
|
||||
'application/json', 'application/xml', 'text/xml',
|
||||
];
|
||||
|
||||
private const MIME_TO_EXTENSION = [
|
||||
'image/jpeg' => 'jpg', 'image/png' => 'png', 'image/gif' => 'gif',
|
||||
'image/webp' => 'webp', 'image/svg+xml' => 'svg',
|
||||
'application/pdf' => 'pdf',
|
||||
'application/msword' => 'doc',
|
||||
'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => 'docx',
|
||||
'application/vnd.ms-excel' => 'xls',
|
||||
'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => 'xlsx',
|
||||
'application/vnd.ms-powerpoint' => 'ppt',
|
||||
'application/vnd.openxmlformats-officedocument.presentationml.presentation' => 'pptx',
|
||||
'text/plain' => 'txt', 'text/csv' => 'csv',
|
||||
'application/zip' => 'zip', 'application/x-rar-compressed' => 'rar', 'application/gzip' => 'gz',
|
||||
'application/json' => 'json', 'application/xml' => 'xml', 'text/xml' => 'xml',
|
||||
];
|
||||
|
||||
public function __construct(
|
||||
private EntityManagerInterface $entityManager,
|
||||
private Security $security,
|
||||
@@ -52,50 +81,48 @@ final readonly class TaskDocumentProcessor implements ProcessorInterface
|
||||
throw new BadRequestHttpException('File size exceeds 50 MB limit.');
|
||||
}
|
||||
|
||||
$taskIri = $request->request->get('task');
|
||||
$clientTicketIri = $request->request->get('clientTicket');
|
||||
$taskIri = $request->request->get('task', '');
|
||||
$clientTicketIri = $request->request->get('clientTicket', '');
|
||||
|
||||
if ((null === $taskIri || '' === $taskIri) && (null === $clientTicketIri || '' === $clientTicketIri)) {
|
||||
if ('' === $taskIri && '' === $clientTicketIri) {
|
||||
throw new BadRequestHttpException('Either task or clientTicket IRI is required.');
|
||||
}
|
||||
|
||||
$task = null;
|
||||
$clientTicket = null;
|
||||
|
||||
if (null !== $taskIri && '' !== $taskIri) {
|
||||
// Extract task ID from IRI (e.g., "/api/tasks/42" -> 42)
|
||||
$taskId = (int) basename((string) $taskIri);
|
||||
$task = $this->entityManager->getRepository(Task::class)->find($taskId);
|
||||
if ('' !== $taskIri) {
|
||||
$task = $this->entityManager->getRepository(Task::class)->find((int) basename($taskIri));
|
||||
|
||||
if (null === $task) {
|
||||
throw new BadRequestHttpException('Task not found.');
|
||||
}
|
||||
}
|
||||
|
||||
if (null !== $clientTicketIri && '' !== $clientTicketIri) {
|
||||
$clientTicketId = (int) basename((string) $clientTicketIri);
|
||||
$clientTicket = $this->entityManager->getRepository(ClientTicket::class)->find($clientTicketId);
|
||||
if ('' !== $clientTicketIri) {
|
||||
$clientTicket = $this->entityManager->getRepository(ClientTicket::class)->find((int) basename($clientTicketIri));
|
||||
|
||||
if (null === $clientTicket) {
|
||||
throw new BadRequestHttpException('Client ticket not found.');
|
||||
}
|
||||
|
||||
// Ownership validation for ROLE_CLIENT
|
||||
if (!$this->security->isGranted('ROLE_ADMIN')) {
|
||||
$currentUser = $this->security->getUser();
|
||||
if ($clientTicket->getSubmittedBy() !== $currentUser) {
|
||||
throw new AccessDeniedHttpException('You can only upload documents to your own tickets.');
|
||||
}
|
||||
if (!$this->security->isGranted('ROLE_ADMIN') && $clientTicket->getSubmittedBy() !== $this->security->getUser()) {
|
||||
throw new AccessDeniedHttpException('You can only upload documents to your own tickets.');
|
||||
}
|
||||
}
|
||||
|
||||
// Capture file metadata BEFORE move() — move invalidates the temp file
|
||||
// Use server-detected MIME type (finfo), not the client-supplied one
|
||||
$originalName = $file->getClientOriginalName();
|
||||
$extension = $file->getClientOriginalExtension() ?: 'bin';
|
||||
$mimeType = $file->getClientMimeType() ?? 'application/octet-stream';
|
||||
$mimeType = $file->getMimeType() ?: 'application/octet-stream';
|
||||
$fileSize = $file->getSize();
|
||||
$uuid = Uuid::v4()->toRfc4122();
|
||||
$fileName = $uuid.'.'.$extension;
|
||||
|
||||
if (!in_array($mimeType, self::ALLOWED_MIME_TYPES, true)) {
|
||||
throw new BadRequestHttpException(sprintf('File type "%s" is not allowed.', $mimeType));
|
||||
}
|
||||
|
||||
$extension = self::MIME_TO_EXTENSION[$mimeType] ?? 'bin';
|
||||
$uuid = Uuid::v4()->toRfc4122();
|
||||
$fileName = $uuid.'.'.$extension;
|
||||
|
||||
if (!is_dir($this->uploadDir)) {
|
||||
mkdir($this->uploadDir, 0o775, true);
|
||||
|
||||
@@ -9,6 +9,7 @@ use ApiPlatform\Metadata\Post;
|
||||
use ApiPlatform\State\ProcessorInterface;
|
||||
use App\Entity\Task;
|
||||
use App\Repository\TaskRepository;
|
||||
use Doctrine\ORM\EntityManagerInterface;
|
||||
use Symfony\Component\DependencyInjection\Attribute\Autowire;
|
||||
|
||||
/**
|
||||
@@ -23,6 +24,7 @@ final readonly class TaskNumberProcessor implements ProcessorInterface
|
||||
#[Autowire(service: 'api_platform.doctrine.orm.state.persist_processor')]
|
||||
private ProcessorInterface $persistProcessor,
|
||||
private TaskRepository $taskRepository,
|
||||
private EntityManagerInterface $entityManager,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -31,8 +33,12 @@ final readonly class TaskNumberProcessor implements ProcessorInterface
|
||||
public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
|
||||
{
|
||||
if ($operation instanceof Post && null !== $data->getProject()) {
|
||||
$maxNumber = $this->taskRepository->findMaxNumberByProject($data->getProject());
|
||||
$data->setNumber($maxNumber + 1);
|
||||
return $this->entityManager->wrapInTransaction(function () use ($data, $operation, $uriVariables, $context) {
|
||||
$maxNumber = $this->taskRepository->findMaxNumberByProjectForUpdate($data->getProject());
|
||||
$data->setNumber($maxNumber + 1);
|
||||
|
||||
return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||
});
|
||||
}
|
||||
|
||||
return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||
|
||||
@@ -29,10 +29,10 @@ final readonly class UserPasswordHasherProcessor implements ProcessorInterface
|
||||
*/
|
||||
public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
|
||||
{
|
||||
if (null !== $data->getPassword() && !str_starts_with($data->getPassword(), '$')) {
|
||||
$data->setPassword(
|
||||
$this->passwordHasher->hashPassword($data, $data->getPassword())
|
||||
);
|
||||
$plainPassword = $data->getPassword();
|
||||
|
||||
if (null !== $plainPassword && !str_starts_with($plainPassword, '$')) {
|
||||
$data->setPassword($this->passwordHasher->hashPassword($data, $plainPassword));
|
||||
}
|
||||
|
||||
return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
|
||||
|
||||
Reference in New Issue
Block a user