From f7ea333272117c89f288e1d0f53a259c468ceb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Franco?= Date: Sat, 7 Nov 2026 21:59:03 +0100 Subject: [PATCH 01/12] test(ui): characterize comment, restore and form validation output (spec 213 S0) Pins what the UI answers before the rules behind it move in the next slice: status, Location, flashes, log lines, events and history for the comment and restore routes, and the constraints of the six write forms. No src change. With TASK_BOARD_CHARACTERIZE_DIR set the tests also write the pages with ids and dates masked, so two checkouts on one vendor can be compared byte by byte. --- .../Characterization/CapturesUiOutput.php | 75 +++++ .../UiCommentsCharacterizationTest.php | 282 ++++++++++++++++++ .../UiFormsCharacterizationTest.php | 182 +++++++++++ .../UiRestoreCharacterizationTest.php | 135 +++++++++ 4 files changed, 674 insertions(+) create mode 100644 tests/Functional/Characterization/CapturesUiOutput.php create mode 100644 tests/Functional/Characterization/UiCommentsCharacterizationTest.php create mode 100644 tests/Functional/Characterization/UiFormsCharacterizationTest.php create mode 100644 tests/Functional/Characterization/UiRestoreCharacterizationTest.php diff --git a/tests/Functional/Characterization/CapturesUiOutput.php b/tests/Functional/Characterization/CapturesUiOutput.php new file mode 100644 index 00000000..be173c12 --- /dev/null +++ b/tests/Functional/Characterization/CapturesUiOutput.php @@ -0,0 +1,75 @@ +>, writes: list, refused: list} + */ + private function captured(): array + { + $response = $this->client->getResponse(); + $session = $this->client->getRequest()->getSession(); + self::assertInstanceOf(FlashBagAwareSessionInterface::class, $session); + $records = $this->logger()->records; + $this->logger()->records = []; + // Read and gone, so the next request's flashes stand alone. + $flashes = $session->getFlashBag()->all(); + $session->save(); + + return [ + 'status' => $response->getStatusCode(), + 'location' => $response->headers->get('Location'), + 'flashes' => $flashes, + 'writes' => $this->logged($records, 'TaskBoard write: {action}', 'action'), + 'refused' => $this->logged($records, 'TaskBoard write refused: {reason}', 'reason'), + ]; + } + + /** + * @param list}> $records + * + * @return list + */ + private function logged(array $records, string $message, string $key): array + { + $matching = array_filter($records, static fn (array $record): bool => $record['message'] === $message); + + return array_values(array_map(static fn (array $record): string => (string) $record['context'][$key], $matching)); + } + + private function dumpBody(string $name): void + { + $directory = getenv('TASK_BOARD_CHARACTERIZE_DIR'); + if (!is_string($directory) || $directory === '') { + return; + } + if (!is_dir($directory)) { + mkdir($directory, 0o777, true); + } + $body = (string) $this->client->getResponse()->getContent(); + $body = (string) preg_replace('/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/', '{uuid}', $body); + $body = (string) preg_replace('/\d{4}-\d{2}-\d{2}([ T]\d{2}:\d{2}(:\d{2})?)?/', '{date}', $body); + file_put_contents($directory . '/' . $name . '.html', $body); + } +} diff --git a/tests/Functional/Characterization/UiCommentsCharacterizationTest.php b/tests/Functional/Characterization/UiCommentsCharacterizationTest.php new file mode 100644 index 00000000..538e5b1f --- /dev/null +++ b/tests/Functional/Characterization/UiCommentsCharacterizationTest.php @@ -0,0 +1,282 @@ + */ + private array $events = []; + + protected function setUp(): void + { + parent::setUp(); + $this->reader = $this->user('reader@example.com'); + $this->work = $this->task($this->board(), 'Work'); + $dispatcher = self::getContainer()->get('event_dispatcher'); + self::assertInstanceOf(EventDispatcherInterface::class, $dispatcher); + // The reader may read the task and not edit it, as a host's listener decides it. + $readerId = $this->reader->getId(); + $dispatcher->addListener(TaskBoardEvents::TASK_ACCESS_CHECK, static function (TaskAccessCheckEvent $event) use ($readerId): void { + $subject = $event->getSubject(); + if ($subject instanceof AppUser && $subject->getId() === $readerId) { + $event->grant(); + $event->markReadOnly(); + } + }); + $dispatcher->addListener(TaskBoardEvents::TASK_COMMENT, function (TaskCommentEvent $event): void { + $this->events[] = 'comment:' . $event->getAction(); + }); + $dispatcher->addListener(TaskBoardEvents::TASK_MENTION, function (TaskMentionEvent $event): void { + $this->events[] = 'mention'; + }); + $this->logger()->records = []; + } + + public function testAReaderAddsACommentAndTheMentionedPersonIsNamed(): void + { + $this->login($this->reader); + + $this->comment('Hi @owner@example.com', [['id' => (string) $this->owner->getId(), 'label' => 'owner@example.com']]); + + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . $this->work->getId(), + 'flashes' => ['success' => ['Comment added.']], + 'writes' => ['task_comment'], + 'refused' => [], + ], $this->captured()); + self::assertSame(['comment:created', 'mention'], $this->events); + $stored = $this->onlyComment(); + self::assertSame('Hi @owner@example.com', $stored->getBody()); + self::assertSame([(string) $this->owner->getId()], $stored->getMentionedIds()); + } + + public function testAnEmptyCommentIsRefusedWithAFlash(): void + { + $this->login($this->reader); + + $this->comment(''); + + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . $this->work->getId(), + 'flashes' => ['error' => ['The comment was not saved: write between 1 and 5000 characters.']], + 'writes' => [], + 'refused' => [], + ], $this->captured()); + self::assertSame([], $this->events); + } + + public function testAForgedTokenOnACommentIs403AndLogged(): void + { + $this->login($this->reader); + + $this->client->request('POST', self::BASE . $this->work->getId() . '/comment', ['task_comment' => ['body' => 'x', 'mentions' => '[]', '_token' => 'forged']]); + + $captured = $this->captured(); + self::assertSame(403, $captured['status']); + self::assertSame(['csrf_invalid'], $captured['refused']); + self::assertSame([], $captured['writes']); + self::assertSame([], $this->events); + } + + public function testReadersAreRefusedWhenTheOptionIsOffAndEditorsAreNot(): void + { + self::$taskBoardConfig = ['comments' => ['readers_can_comment' => false]]; + self::ensureKernelShutdown(); + $this->events = []; + $this->setUp(); + $this->login($this->reader); + + $this->comment('Reader'); + self::assertSame(403, $this->captured()['status']); + + $this->login($this->owner); + $this->comment('Editor'); + self::assertSame(['task_comment'], $this->captured()['writes']); + self::assertSame('Editor', $this->onlyComment()->getBody()); + } + + public function testOnlyTheAuthorEditsAndOnlyNewMentionsAreNotified(): void + { + $this->login($this->reader); + $mention = [['id' => (string) $this->owner->getId(), 'label' => 'owner@example.com']]; + $this->comment('Hi @owner@example.com', $mention); + $this->captured(); + $this->events = []; + $comment = $this->onlyComment(); + + $this->client->request('POST', $this->commentUrl($comment), ['task_comment' => ['body' => 'Still @owner@example.com', 'mentions' => json_encode($mention), '_token' => PredictableCsrfTokenManager::value('task_comment')]]); + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . $this->work->getId(), + 'flashes' => ['success' => ['Comment updated.']], + 'writes' => ['task_comment_edit'], + 'refused' => [], + ], $this->captured()); + self::assertSame(['comment:edited'], $this->events); + + $this->login($this->owner); + $this->client->request('POST', $this->commentUrl($comment), ['task_comment' => ['body' => 'Hijack', 'mentions' => '[]', '_token' => PredictableCsrfTokenManager::value('task_comment')]]); + self::assertSame(403, $this->captured()['status']); + self::assertSame('Still @owner@example.com', $this->onlyComment()->getBody()); + } + + public function testTheAuthorOrAnEditorDeletesAndNobodyElse(): void + { + $other = $this->user('other@example.com'); + $this->login($this->owner); + $this->comment('By the owner'); + $this->captured(); + $comment = $this->onlyComment(); + $this->events = []; + + // A reader who did not write it. + $this->login($this->reader); + $this->client->request('POST', $this->commentUrl($comment) . '/delete', ['_token' => PredictableCsrfTokenManager::value('task_board_task_comment_delete')]); + self::assertSame(403, $this->captured()['status']); + self::assertFalse($this->onlyComment()->isDeleted()); + + // Someone with no access at all to the task. + $this->login($other); + $this->client->request('POST', $this->commentUrl($comment) . '/delete', ['_token' => PredictableCsrfTokenManager::value('task_board_task_comment_delete')]); + self::assertSame(403, $this->captured()['status']); + + // The author; no event is dispatched for a deletion. + $this->login($this->owner); + $this->client->request('POST', $this->commentUrl($comment) . '/delete', ['_token' => PredictableCsrfTokenManager::value('task_board_task_comment_delete')]); + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . $this->work->getId(), + 'flashes' => ['success' => ['Comment deleted.']], + 'writes' => ['task_comment_delete'], + 'refused' => [], + ], $this->captured()); + self::assertSame([], $this->events); + self::assertTrue($this->onlyComment()->isDeleted()); + + // Deleted: not found, for edit and delete alike. + $this->client->request('POST', $this->commentUrl($comment) . '/delete', ['_token' => PredictableCsrfTokenManager::value('task_board_task_comment_delete')]); + self::assertSame(404, $this->captured()['status']); + } + + public function testAReaderDeletesTheirOwnComment(): void + { + $this->login($this->reader); + $this->comment('Mine'); + $this->captured(); + + $this->client->request('POST', $this->commentUrl($this->onlyComment()) . '/delete', ['_token' => PredictableCsrfTokenManager::value('task_board_task_comment_delete')]); + + self::assertSame(['task_comment_delete'], $this->captured()['writes']); + self::assertTrue($this->onlyComment()->isDeleted()); + } + + public function testAPanelWriteAnswersTheFragmentWithTheViewersControls(): void + { + $this->login($this->reader); + + $this->client->xmlHttpRequest('POST', self::BASE . $this->work->getId() . '/comment', ['task_comment' => ['body' => 'From the panel', 'mentions' => '[]', '_token' => PredictableCsrfTokenManager::value('task_comment')]]); + + $captured = $this->captured(); + self::assertSame(200, $captured['status']); + self::assertNull($captured['location']); + self::assertSame(['task_comment'], $captured['writes']); + $this->dumpBody('comment-panel-reader'); + $comment = $this->onlyComment(); + $page = (string) $this->client->getResponse()->getContent(); + self::assertStringNotContainsString('commentUrl($comment), $page, 'the author can edit'); + self::assertStringContainsString($this->commentUrl($comment) . '/delete', $page, 'the author can delete'); + } + + public function testTheTaskPageShowsEachViewerTheirControls(): void + { + $this->login($this->owner); + $this->comment('By the owner'); + $this->captured(); + $comment = $this->onlyComment(); + + $this->client->request('GET', self::BASE . $this->work->getId()); + self::assertResponseIsSuccessful(); + $this->dumpBody('task-page-editor'); + $editor = (string) $this->client->getResponse()->getContent(); + self::assertStringContainsString($this->commentUrl($comment) . '"', $editor, 'the author edits'); + self::assertStringContainsString($this->commentUrl($comment) . '/delete', $editor); + self::assertStringContainsString(self::BASE . $this->work->getId() . '/comment"', $editor, 'the box'); + + $this->login($this->reader); + $this->client->request('GET', self::BASE . $this->work->getId()); + self::assertResponseIsSuccessful(); + $this->dumpBody('task-page-reader'); + $reader = (string) $this->client->getResponse()->getContent(); + self::assertFalse(str_contains($reader, $this->commentUrl($comment) . '"'), 'not the author: no edit form'); + self::assertFalse(str_contains($reader, $this->commentUrl($comment) . '/delete'), 'not an editor: no delete'); + self::assertStringContainsString(self::BASE . $this->work->getId() . '/comment"', $reader, 'readers comment by default'); + } + + public function testTheTaskPageHidesTheBoxFromReadersWhenTheOptionIsOff(): void + { + self::$taskBoardConfig = ['comments' => ['readers_can_comment' => false]]; + self::ensureKernelShutdown(); + $this->events = []; + $this->setUp(); + $this->login($this->reader); + + $this->client->request('GET', self::BASE . $this->work->getId()); + + self::assertResponseIsSuccessful(); + $this->dumpBody('task-page-reader-option-off'); + self::assertStringNotContainsString(self::BASE . $this->work->getId() . '/comment"', (string) $this->client->getResponse()->getContent()); + } + + /** + * @param list $mentions + */ + private function comment(string $body, array $mentions = []): void + { + $this->client->request('POST', self::BASE . $this->work->getId() . '/comment', ['task_comment' => ['body' => $body, 'mentions' => json_encode($mentions), '_token' => PredictableCsrfTokenManager::value('task_comment')]]); + } + + private function commentUrl(TaskComment $comment): string + { + return self::BASE . $this->work->getId() . '/comment/' . $comment->getId(); + } + + private function onlyComment(): TaskComment + { + $this->em->clear(); + $comments = $this->em->getRepository(TaskComment::class)->findAll(); + self::assertCount(1, $comments); + + return $comments[0]; + } +} diff --git a/tests/Functional/Characterization/UiFormsCharacterizationTest.php b/tests/Functional/Characterization/UiFormsCharacterizationTest.php new file mode 100644 index 00000000..20852a6b --- /dev/null +++ b/tests/Functional/Characterization/UiFormsCharacterizationTest.php @@ -0,0 +1,182 @@ +, 2: array>>}> + */ + public static function forms(): iterable + { + yield 'task' => [TaskFormType::class, ['column_choices' => []], [ + 'title' => [['class' => NotBlank::class], ['class' => Length::class, 'max' => 255]], + 'startAt' => [['class' => Callback::class]], + ]]; + yield 'board' => [TaskBoardFormType::class, ['icon_field' => true, 'icon_too_long_message' => 'long', 'icon_invalid_message' => 'invalid'], [ + 'name' => [['class' => NotBlank::class], ['class' => Length::class, 'max' => 255]], + 'slug' => [['class' => Length::class, 'max' => 255]], + 'icon' => [ + ['class' => Length::class, 'max' => BoardIcon::MAX_BYTES, 'countUnit' => Length::COUNT_BYTES, 'maxMessage' => 'long'], + ['class' => Regex::class, 'pattern' => BoardIcon::PATTERN, 'message' => 'invalid'], + ], + ]]; + yield 'column' => [BoardColumnFormType::class, [], [ + 'name' => [['class' => NotBlank::class], ['class' => Length::class, 'max' => 128]], + 'color' => [['class' => Regex::class, 'pattern' => BoardColumnFormType::COLOR_PATTERN]], + ]]; + yield 'comment' => [TaskCommentFormType::class, [], [ + 'body' => [['class' => NotBlank::class], ['class' => Length::class, 'max' => TaskComment::MAX_LENGTH]], + ]]; + yield 'time' => [TaskTimeEntryFormType::class, [], [ + 'minutes' => [['class' => NotNull::class], ['class' => Range::class, 'min' => 1, 'max' => TaskTimeEntryFormType::MAX_MINUTES]], + 'loggedAt' => [['class' => NotNull::class]], + 'note' => [['class' => Length::class, 'max' => 512]], + ]]; + yield 'link' => [TaskLinkFormType::class, [], [ + 'url' => [ + ['class' => NotBlank::class], + ['class' => Url::class, 'protocols' => ['http', 'https'], 'requireTld' => false], + ['class' => Length::class, 'max' => 2048], + ], + 'label' => [['class' => Length::class, 'max' => 255]], + 'externalId' => [['class' => Length::class, 'max' => 128]], + ]]; + } + + /** + * @param class-string $type + * @param array $options + * @param array>> $expected + */ + #[DataProvider('forms')] + public function testAFormKeepsItsConstraints(string $type, array $options, array $expected): void + { + $factory = self::getContainer()->get('form.factory'); + self::assertInstanceOf(FormFactoryInterface::class, $factory); + $form = $factory->create($type, null, $options); + + foreach ($expected as $field => $constraints) { + self::assertSame($constraints, $this->described($form->get($field), $constraints), $field); + } + } + + public function testATitleOverTheColumnSizeIsAFlashAndNothingIsStored(): void + { + $board = $this->board(); + $column = $board->getColumns()->first(); + self::assertInstanceOf(BoardColumn::class, $column); + $this->login($this->owner); + $this->logger()->records = []; + + $this->client->request('POST', '/tools/task-board/board/' . $board->getId() . '/create', ['task_form' => ['title' => str_repeat('x', 256), 'columnId' => $column->getId(), '_token' => PredictableCsrfTokenManager::value('task_form')]]); + + $captured = $this->captured(); + self::assertSame(302, $captured['status']); + self::assertSame([], $captured['writes']); + self::assertCount(1, $captured['flashes']['error'] ?? []); + self::assertSame([], $this->em->getRepository(Task::class)->findBy(['board' => $board])); + } + + public function testAJavascriptLinkIsNotStored(): void + { + $task = $this->task($this->board(), 'Linked'); + $this->login($this->owner); + $this->logger()->records = []; + + $this->client->request('POST', '/tools/task-board/task/' . $task->getId() . '/link', ['task_link_form' => ['linkType' => 'url', 'url' => 'javascript:alert(1)', '_token' => PredictableCsrfTokenManager::value('task_link_form')]]); + + self::assertSame([], $this->captured()['writes']); + self::assertSame([], $this->em->getRepository(TaskLink::class)->findBy(['task' => $task])); + } + + public function testAStartAfterTheDueDateIsAFormError(): void + { + $factory = self::getContainer()->get('form.factory'); + self::assertInstanceOf(FormFactoryInterface::class, $factory); + $form = $factory->create(TaskFormType::class, new TaskFormData(), ['column_choices' => [], 'csrf_protection' => false, 'rich_description' => false, 'tag_input' => false]); + + $form->submit(['title' => 'Dates', 'priority' => 'normal', 'startAt' => '2026-11-07', 'dueAt' => '2026-11-06']); + + self::assertFalse($form->isValid()); + self::assertSame(1, $form->get('startAt')->getErrors()->count()); + } + + public function testTheBoardPagesRenderTheirForms(): void + { + $board = $this->board(); + $task = $this->task($board, 'Rendered'); + $this->login($this->owner); + + foreach (['board' => '/board/' . $board->getId(), 'list' => '/board/' . $board->getId() . '/list', 'index' => '', 'task' => '/task/' . $task->getId(), 'import' => '/board/' . $board->getId() . '/import'] as $name => $path) { + $this->client->request('GET', '/tools/task-board' . $path); + self::assertResponseIsSuccessful($name); + $this->dumpBody('page-' . $name); + } + } + + /** + * @param FormInterface $field + * @param list> $expected what to read of each constraint + * + * @return list> + */ + private function described(FormInterface $field, array $expected): array + { + $constraints = $field->getConfig()->getOption('constraints'); + self::assertIsArray($constraints); + self::assertCount(count($expected), $constraints); + $described = []; + foreach (array_values($constraints) as $index => $constraint) { + self::assertInstanceOf(Constraint::class, $constraint); + $row = ['class' => $constraint::class]; + foreach (array_keys($expected[$index]) as $property) { + if ($property !== 'class') { + $row[$property] = $constraint->{$property}; + } + } + $described[] = $row; + } + + return $described; + } +} diff --git a/tests/Functional/Characterization/UiRestoreCharacterizationTest.php b/tests/Functional/Characterization/UiRestoreCharacterizationTest.php new file mode 100644 index 00000000..d6a59389 --- /dev/null +++ b/tests/Functional/Characterization/UiRestoreCharacterizationTest.php @@ -0,0 +1,135 @@ +board(); + $task = $this->archived($board, 'Back'); + $this->login($this->owner); + $this->logger()->records = []; + + $this->restore($task); + + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . 'board/' . $board->getId() . '/list', + 'flashes' => ['success' => ['The task was restored.']], + 'writes' => ['task_restore'], + 'refused' => [], + ], $this->captured()); + self::assertSame([[1, 0]], TestSyncLimitChecker::$calls); + $this->em->clear(); + $reloaded = $this->em->find(Task::class, $task->getId()); + self::assertInstanceOf(Task::class, $reloaded); + self::assertFalse($reloaded->isArchived()); + $history = $this->em->getRepository(TaskChangeHistory::class)->findBy(['task' => $reloaded, 'changeType' => TaskChangeType::Restored]); + self::assertCount(1, $history); + self::assertSame('manual', $history[0]->getNewValue()); + } + + public function testACompletedTaskAsksTheLimitForNothing(): void + { + $board = $this->board(); + $task = $this->archived($board, 'Done', completed: true); + $this->login($this->owner); + + $this->restore($task); + + self::assertSame([[0, 0]], TestSyncLimitChecker::$calls); + self::assertSame(['task_restore'], $this->captured()['writes']); + } + + public function testARefusedRestoreChangesNothingAndSaysSo(): void + { + $board = $this->board(); + $task = $this->archived($board, 'Over the limit'); + $this->login($this->owner); + $this->logger()->records = []; + TestSyncLimitChecker::$refuseTasks = true; + + $this->restore($task); + + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . 'board/' . $board->getId() . '/list', + 'flashes' => ['error' => ['The task was not restored: the limit of active tasks was reached.']], + 'writes' => [], + 'refused' => [], + ], $this->captured()); + $this->em->clear(); + self::assertTrue($this->em->find(Task::class, $task->getId())?->isArchived()); + self::assertSame([], $this->em->getRepository(TaskChangeHistory::class)->findBy(['changeType' => TaskChangeType::Restored])); + } + + public function testATaskThatIsNotArchivedIsLeftAloneWithoutAskingTheLimit(): void + { + $board = $this->board(); + $task = $this->task($board, 'Active'); + $this->login($this->owner); + $this->logger()->records = []; + + $this->restore($task); + + self::assertSame([ + 'status' => 302, + 'location' => self::BASE . 'board/' . $board->getId() . '/list', + 'flashes' => [], + 'writes' => [], + 'refused' => [], + ], $this->captured()); + self::assertSame([], TestSyncLimitChecker::$calls); + } + + public function testAForgedTokenIsRefusedBeforeAnything(): void + { + $board = $this->board(); + $task = $this->archived($board, 'Stays'); + $this->login($this->owner); + $this->logger()->records = []; + + $this->client->request('POST', self::BASE . 'task/' . $task->getId() . '/restore', ['_token' => 'forged']); + + $captured = $this->captured(); + self::assertSame(403, $captured['status']); + self::assertSame(['csrf_invalid'], $captured['refused']); + self::assertSame([], TestSyncLimitChecker::$calls); + } + + private function archived(TaskBoard $board, string $title, bool $completed = false): Task + { + $task = new Task(board: $board, title: $title, creator: $this->owner, column: $board->getColumns()->first() ?: null); + if ($completed) { + $task->markCompleted(); + } + $task->archive(); + $this->em->persist($task); + $this->em->flush(); + + return $task; + } + + private function restore(Task $task): void + { + $this->client->request('POST', self::BASE . 'task/' . $task->getId() . '/restore', ['_token' => PredictableCsrfTokenManager::value('task_board_task_restore')]); + } +} From 551dcc7f37556be1d31e62d848874af6446b2727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Franco?= Date: Sat, 7 Nov 2026 22:05:35 +0100 Subject: [PATCH 02/12] refactor(tasks): give comment, restore and field rules one home (spec 213 S1) A write path other than the UI's forms must apply the same rules, so they leave the controller and the form types: TaskCommentPolicy, TaskManager::restore() and Validation\TaskConstraints. tests/Functional/Characterization from the previous commit is unchanged and passes, and the rendered pages are byte-identical. Adds what the UI does not have and the API will need, with no caller yet: TaskBoardEraser::eraseTask() (leaf-first, so PreRemove keeps every minute; refuses a task that follows a provider), TaskBoardEditor, TaskManager::archive(), TaskDependencyManager::findBetween(), TaskFormData::fromTask(). --- docs/CHANGELOG.md | 15 ++ docs/SECURITY.md | 5 + docs/UPGRADING.md | 4 + docs/USAGE.md | 4 + src/Controller/TaskBoardManageController.php | 29 +-- .../TaskBoardExtension.php | 3 - src/Dto/TaskFormData.php | 19 ++ src/Event/BoardUpdatedEvent.php | 24 ++ src/Event/TaskBoardEvents.php | 3 + .../TaskLinkedToProviderException.php | 19 ++ src/Form/BoardColumnFormType.php | 15 +- src/Form/TaskBoardFormType.php | 13 +- src/Form/TaskCommentFormType.php | 5 +- src/Form/TaskFormType.php | 20 +- src/Form/TaskLinkFormType.php | 17 +- src/Form/TaskTimeEntryFormType.php | 12 +- src/Resources/config/services.yaml | 6 + src/Service/TaskBoardEditor.php | 42 ++++ src/Service/TaskBoardEraser.php | 81 +++++++ src/Service/TaskCommentPolicy.php | 37 +++ src/Service/TaskDependencyManager.php | 17 ++ src/Service/TaskManager.php | 38 ++++ src/Validation/TaskConstraints.php | 147 ++++++++++++ tests/Functional/SharedWriteServicesTest.php | 212 ++++++++++++++++++ tests/Functional/Sync/SyncEraseTaskTest.php | 47 ++++ tests/Unit/Validation/TaskConstraintsTest.php | 80 +++++++ 26 files changed, 845 insertions(+), 69 deletions(-) create mode 100644 src/Event/BoardUpdatedEvent.php create mode 100644 src/Exception/TaskLinkedToProviderException.php create mode 100644 src/Service/TaskBoardEditor.php create mode 100644 src/Service/TaskCommentPolicy.php create mode 100644 src/Validation/TaskConstraints.php create mode 100644 tests/Functional/SharedWriteServicesTest.php create mode 100644 tests/Functional/Sync/SyncEraseTaskTest.php create mode 100644 tests/Unit/Validation/TaskConstraintsTest.php diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 755b3228..f99917f1 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -85,6 +85,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +Target: 2.2.0, spec 213 (Task API). Audit status: re-verification pending. No behaviour, schema or configuration change for a host. + +### Added + +- `Service\TaskManager::restore()` and `archive()`, with their history; `restore()` asks the host's limit and throws `TaskReopenRefusedException` when it refuses. +- `Service\TaskBoardEraser::eraseTask()`: one task and its subtasks, leaf-first through Doctrine, tracked time kept and detached. Needs the board's manage decision; a task that follows a provider, or has a subtask that does, is refused with `TaskLinkedToProviderException`. +- `Service\TaskBoardEditor::update()`: name and description of a board, the slug unchanged, `TaskBoardEvents::BOARD_UPDATED` (`BoardUpdatedEvent`). +- `Service\TaskCommentPolicy`: who may write, edit and delete a comment. +- `Validation\TaskConstraints`: the rules of the fields a write takes. +- `Service\TaskDependencyManager::findBetween()`, `Dto\TaskFormData::fromTask()`. + +### Changed + +- `TaskBoardManageController` asks `TaskCommentPolicy` and `TaskManager::restore()`, and the write forms take their rules from `TaskConstraints`. Their output does not change: `tests/Functional/Characterization` pins it. The controller no longer takes `$syncLimits`, as `TaskManager` already holds the limit. + ## [2.1.0] - 2026-11-07 Target: 2.1.0, spec 214. Audit HY (2026-11-07): Pass (good) / Low at `3c105a1`. Opt-in: with `task_numbers.enabled` and `calendar.enabled` off (the defaults) no route, service, table or output changes, except the one column below. See [UPGRADING](UPGRADING.md#to-210). diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 93294d85..11f79599 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -12,6 +12,7 @@ - [Task types, tag colours and descriptions (since 1.28.0)](#task-types-tag-colours-and-descriptions-since-1280) - [Task numbers and calendar (since 2.1)](#task-numbers-and-calendar-since-21) - [UiKit migration (2.0.0)](#uikit-migration-200) +- [Shared write rules (2.2, re-verification pending)](#shared-write-rules-22-re-verification-pending) - [Logging and personal data](#logging-and-personal-data) - [Personal data in synced tasks (RGPD / LOPDGDD)](#personal-data-in-synced-tasks-rgpd--lopdgdd) - [Dependencies](#dependencies) @@ -162,6 +163,10 @@ Both opt-in. - **Avatars.** Pictures now pass UiKit's `AvatarUrlPolicy`: https without userinfo, or a same-origin path. - **Subtask nodes.** Titles reach `tree` as text, escaped by the partial, and in `data-*` attributes through `nowo_ui_safe_attrs`. +## Shared write rules (2.2, re-verification pending) + +Spec 213 gives the comment rules, the restore limit and the field constraints one home (`TaskCommentPolicy`, `TaskManager::restore()`, `TaskConstraints`), so a write path other than the forms cannot apply different ones. `TaskBoardEraser::eraseTask()` asks the board's manage decision itself, erases leaf-first so the `PreRemove` that keeps time runs for every subtask (`parent_id` is `ON DELETE CASCADE`, which would skip it), and refuses a task that follows a provider. `tests/Functional/Characterization` pins the UI's output. + ## Logging and personal data REQ-OBS-001 (since 1.21): every successful board / task write logs one `info` line, an invalid CSRF token one `warning`, diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index 72e6f6b7..d228b9c6 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -154,6 +154,10 @@ Hosting FrankenPHP with **kernel not reset** between requests: see [FRANKENPHP-W ## Unreleased +Target: 2.2.0, spec 213. Audit status: re-verification pending. No schema or configuration change, and the UI's output stays as it was. + +A host that builds `TaskBoardManageController` by hand instead of through the container: the `$syncLimits` argument is gone, and the limit a restore asks is the one given to `TaskManager` (`$limits`). + ## To 2.1.0 From **2.0.2**: diff --git a/docs/USAGE.md b/docs/USAGE.md index 9e27a065..3108f6be 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -76,6 +76,10 @@ Authenticated users open the manage UI (default `/tools/task-board`): | Log time | `task_time` | Since 1.15: minutes, day and note (POST + CSRF) | | Remove time entry | `task_time_remove` | Since 1.15 (POST + CSRF) | +### Deleting and renaming outside the UI (since 2.2) + +The UI has no delete task or rename board. A host calling the services can: `TaskBoardEraser::eraseTask($task, $actor)` deletes a task and its subtasks and keeps tracked time, detached (a task that follows a provider is refused); `TaskBoardEditor::update($board, $name, $description, $actor)` keeps the slug and dispatches `BOARD_UPDATED`. Check access first with `TaskBoardResourceAccessChecker`, and validate with `Validation\TaskConstraints`, the rules the forms use. + ## TimeTrack integration (optional) When TimeTrack is installed and configured with `nowo_task_board.task_provider`: diff --git a/src/Controller/TaskBoardManageController.php b/src/Controller/TaskBoardManageController.php index 7ea1e974..45ba7187 100644 --- a/src/Controller/TaskBoardManageController.php +++ b/src/Controller/TaskBoardManageController.php @@ -80,6 +80,7 @@ use Nowo\TaskBoardBundle\Service\TaskBoardQuota; use Nowo\TaskBoardBundle\Service\TaskCalendarBuilder; use Nowo\TaskBoardBundle\Service\TaskCommentManager; +use Nowo\TaskBoardBundle\Service\TaskCommentPolicy; use Nowo\TaskBoardBundle\Service\TaskDependencyManager; use Nowo\TaskBoardBundle\Service\TaskGanttBuilder; use Nowo\TaskBoardBundle\Service\TaskLinkAttacher; @@ -91,8 +92,6 @@ use Nowo\TaskBoardBundle\Support\TaskTree; use Nowo\TaskBoardBundle\Support\UserIdResolver; use Nowo\TaskBoardBundle\Support\UserLabel; -use Nowo\TaskBoardBundle\Sync\Hook\LimitRefusal; -use Nowo\TaskBoardBundle\Sync\Hook\SyncLimitCheckerInterface; use Nowo\TaskBoardBundle\Tenancy\FirstBoardProvisionerInterface; use Nowo\TaskBoardBundle\Tenancy\TaskBoardTenant; use Nowo\TaskBoardBundle\Tenancy\TaskBoardTenantContext; @@ -204,8 +203,6 @@ public function __construct( // invalid board create (`board.create_dialog_header`). private readonly ?TaskBoardDefaultViewResolverInterface $defaultViewResolver = null, private readonly ?string $createDialogHeader = null, - // Since 1.27.0 (audit GW-L8): the host's limit of active tasks, asked when a task is restored (with `sync.enabled`). - private readonly ?SyncLimitCheckerInterface $syncLimits = null, // Since 1.30.0 (spec 211). private readonly ?BoardScopeProviderInterface $scopeProvider = null, /** @var array */ @@ -764,13 +761,13 @@ public function restoreTask(string $taskId, Request $request): RedirectResponse $task = $this->findTask($taskId, $user, true); $this->denyUnlessValidCsrfForm($request, $this->csrfActionForm('task_board_task_restore')); // Since 1.29.0: a completed task is not an active task, so restoring it adds nothing to the limit. - if ($task->isArchived() && $this->syncLimits?->check($task->getBoard(), $task->isCompleted() ? 0 : 1, 0) instanceof LimitRefusal) { + try { + if ($this->taskManager->restore($task, $user)) { + $this->logWrite('task_restore', ['task_id' => $taskId]); + $this->addFlash('success', 'task_board.flash.task_restored'); + } + } catch (TaskReopenRefusedException) { $this->addFlash('error', 'task_board.flash.task_restore_refused'); - } elseif ($task->isArchived()) { - $task->restore(); - $this->taskManager->recordRestored($task, $user); - $this->logWrite('task_restore', ['task_id' => $taskId]); - $this->addFlash('success', 'task_board.flash.task_restored'); } return $this->redirectToRoute($this->routes['list']['name'], ['boardId' => $task->getBoard()->getId()]); @@ -1367,7 +1364,7 @@ public function editComment(string $taskId, string $commentId, Request $request) $task = $this->findTask($taskId, $user); $manager = $this->commentManager ?? throw new NotFoundHttpException('Comments are not available.'); $comment = $this->findComment($task, $commentId); - if (!TaskCommentManager::isAuthor($comment, $user) || !$this->canComment($user, $task)) { + if (!$this->commentPolicy()->mayEdit($user, $task, $comment)) { throw $this->createAccessDeniedException('Only the author edits a comment.'); } @@ -1438,15 +1435,19 @@ private function findComment(Task $task, string $commentId): TaskComment return $comment; } + private function commentPolicy(): TaskCommentPolicy + { + return new TaskCommentPolicy($this->resourceAccessChecker, $this->readersCanComment); + } + private function canComment(UserInterface $user, Task $task): bool { - return $this->commentManager instanceof TaskCommentManager - && ($this->readersCanComment || $this->resourceAccessChecker->canEditTask($user, $task)); + return $this->commentManager instanceof TaskCommentManager && $this->commentPolicy()->mayComment($user, $task); } private function canDeleteComment(UserInterface $user, Task $task, TaskComment $comment): bool { - return TaskCommentManager::isAuthor($comment, $user) || $this->resourceAccessChecker->canEditTask($user, $task); + return $this->commentPolicy()->mayDelete($user, $task, $comment); } /** diff --git a/src/DependencyInjection/TaskBoardExtension.php b/src/DependencyInjection/TaskBoardExtension.php index dd1959b1..dd23470e 100644 --- a/src/DependencyInjection/TaskBoardExtension.php +++ b/src/DependencyInjection/TaskBoardExtension.php @@ -417,9 +417,6 @@ private function registerSyncPeople(ContainerBuilder $container, array $sync): v ->setArgument('$fullSyncRereadsDetails', (bool) $sync['full_sync_rereads_details']); $container->getDefinition(SyncCustomFieldWriter::class) ->setArgument('$excludeTypes', $sync['custom_fields']['exclude_types']); - // Audit GW-L8: restoring a task by hand asks the host's limit too. - $container->getDefinition(TaskBoardManageController::class) - ->setArgument('$syncLimits', new Reference(SyncLimitCheckerInterface::class)); $container->getDefinition(SyncTaskDetailsWriter::class) ->setArgument('$maxStatusCalls', (int) $sync['status_history']['max_calls_per_run']); $container->getDefinition(TaskBoardTwigExtension::class) diff --git a/src/Dto/TaskFormData.php b/src/Dto/TaskFormData.php index 5a0b2124..ba534f31 100644 --- a/src/Dto/TaskFormData.php +++ b/src/Dto/TaskFormData.php @@ -5,6 +5,7 @@ namespace Nowo\TaskBoardBundle\Dto; use DateTimeImmutable; +use Nowo\TaskBoardBundle\Entity\Task; use Nowo\TaskBoardBundle\Enum\TaskPriority; /** @@ -27,4 +28,22 @@ public function __construct( public ?DateTimeImmutable $startAt = null, ) { } + + /** + * Since 2.2.0 (spec 213): the task as it stands, for a write that changes only some fields. + */ + public static function fromTask(Task $task): self + { + return new self( + title: $task->getTitle(), + description: $task->getDescription(), + priority: $task->getPriority(), + columnId: $task->getColumn()?->getId(), + estimatedMinutes: $task->getEstimatedMinutes(), + dueAt: $task->getDueAt(), + tags: $task->getTags(), + color: $task->getColor(), + startAt: $task->getStartAt(), + ); + } } diff --git a/src/Event/BoardUpdatedEvent.php b/src/Event/BoardUpdatedEvent.php new file mode 100644 index 00000000..9f601368 --- /dev/null +++ b/src/Event/BoardUpdatedEvent.php @@ -0,0 +1,24 @@ + $changed the fields that changed: `name`, `description` + */ + public function __construct( + public readonly TaskBoard $board, + public readonly object $actor, + public readonly array $changed, + ) { + } +} diff --git a/src/Event/TaskBoardEvents.php b/src/Event/TaskBoardEvents.php index 5c504700..449007d9 100644 --- a/src/Event/TaskBoardEvents.php +++ b/src/Event/TaskBoardEvents.php @@ -127,6 +127,9 @@ final class TaskBoardEvents /** Since 2.0.0: {@see TaskBoardQuotaCheckEvent}, before a board or tasks are written. */ public const QUOTA_CHECK = 'nowo_task_board.quota_check'; + /** Since 2.2.0 (spec 213): {@see BoardUpdatedEvent}. */ + public const BOARD_UPDATED = 'nowo_task_board.board_updated'; + /** Since 1.28.0: {@see UnmatchedColumnsResolvedEvent}. */ public const SYNC_UNMATCHED_COLUMNS_RESOLVED = 'nowo_task_board.sync_unmatched_columns_resolved'; } diff --git a/src/Exception/TaskLinkedToProviderException.php b/src/Exception/TaskLinkedToProviderException.php new file mode 100644 index 00000000..9088fd98 --- /dev/null +++ b/src/Exception/TaskLinkedToProviderException.php @@ -0,0 +1,19 @@ + '', 'label' => 'task_board.form.column.name', - 'constraints' => [new NotBlank(), new Length(max: 128)], + 'constraints' => TaskConstraints::columnName(), 'attr' => ['maxlength' => 128], ]); $this->addWithDefaults($builder, 'color', ColorType::class, [ - 'label' => 'task_board.form.column.color', - 'required' => false, - // Rendered inside style="…": hex colours only (#rgb, #rgba, #rrggbb, #rrggbbaa). - 'constraints' => [new Regex(pattern: self::COLOR_PATTERN)], + 'label' => 'task_board.form.column.color', + 'required' => false, + 'constraints' => TaskConstraints::columnColor(), ]); // Since 1.11: an icon picked from `board.column_icons` (no field when the list is empty). diff --git a/src/Form/TaskBoardFormType.php b/src/Form/TaskBoardFormType.php index ed8f0f90..e7b22565 100644 --- a/src/Form/TaskBoardFormType.php +++ b/src/Form/TaskBoardFormType.php @@ -8,8 +8,8 @@ use Nowo\FormKitBundle\Form\FormOptionsTrait; use Nowo\TaskBoardBundle\Board\BoardScope; use Nowo\TaskBoardBundle\Dto\TaskBoardFormData; -use Nowo\TaskBoardBundle\Support\BoardIcon; use Nowo\TaskBoardBundle\TaskBoardBundle; +use Nowo\TaskBoardBundle\Validation\TaskConstraints; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\ChoiceType; use Symfony\Component\Form\Extension\Core\Type\FormType; @@ -20,9 +20,7 @@ use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormView; use Symfony\Component\OptionsResolver\OptionsResolver; -use Symfony\Component\Validator\Constraints\Length; use Symfony\Component\Validator\Constraints\NotBlank; -use Symfony\Component\Validator\Constraints\Regex; use function array_filter; use function array_values; @@ -48,17 +46,14 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'help' => 'task_board.form.board.icon_help', 'required' => false, 'empty_data' => null, - 'constraints' => [ - new Length(max: BoardIcon::MAX_BYTES, countUnit: Length::COUNT_BYTES, maxMessage: $options['icon_too_long_message']), - new Regex(pattern: BoardIcon::PATTERN, message: $options['icon_invalid_message']), - ], + 'constraints' => TaskConstraints::boardIcon($options['icon_too_long_message'], $options['icon_invalid_message']), ] + $options['icon_form_options']); } $this->addText($builder, 'name', [ // Since 1.21: an empty or missing name is a NotBlank error, not a TypeError on the DTO (string). 'empty_data' => '', 'label' => 'task_board.form.board.name', - 'constraints' => [new NotBlank(), new Length(max: 255)], + 'constraints' => TaskConstraints::boardName(), 'attr' => ['maxlength' => 255], ]); $this->addText($builder, 'slug', [ @@ -67,7 +62,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void // Since 1.14.1: an empty slug is '' (the board creator derives it from the name), not null. 'empty_data' => '', 'help' => 'task_board.form.board.slug_help', - 'constraints' => [new Length(max: 255)], + 'constraints' => TaskConstraints::boardSlug(), 'attr' => ['maxlength' => 255], ]); // Since 1.30.0 (spec 211): `board.description_form_type` replaces the widget with no Twig override. diff --git a/src/Form/TaskCommentFormType.php b/src/Form/TaskCommentFormType.php index 9aba8346..c834e3e7 100644 --- a/src/Form/TaskCommentFormType.php +++ b/src/Form/TaskCommentFormType.php @@ -9,14 +9,13 @@ use Nowo\TaskBoardBundle\Dto\TaskCommentFormData; use Nowo\TaskBoardBundle\Entity\TaskComment; use Nowo\TaskBoardBundle\TaskBoardBundle; +use Nowo\TaskBoardBundle\Validation\TaskConstraints; use Override; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\HiddenType; use Symfony\Component\Form\Extension\Core\Type\TextareaType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; -use Symfony\Component\Validator\Constraints\Length; -use Symfony\Component\Validator\Constraints\NotBlank; /** * Since 1.16: a task comment — the text (the `nowo-task-board-mention` controller offers @mentions while typing) and @@ -43,7 +42,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'label' => 'task_board.comment.body', 'label_attr' => ['class' => 'nowo-task-sr-only'], 'empty_data' => '', - 'constraints' => [new NotBlank(), new Length(max: TaskComment::MAX_LENGTH)], + 'constraints' => TaskConstraints::commentBody(), 'attr' => [ 'class' => 'form-control nowo-task-comment__input', 'rows' => 3, diff --git a/src/Form/TaskFormType.php b/src/Form/TaskFormType.php index 2a49f457..55fcf469 100644 --- a/src/Form/TaskFormType.php +++ b/src/Form/TaskFormType.php @@ -4,7 +4,6 @@ namespace Nowo\TaskBoardBundle\Form; -use DateTimeInterface; use Nowo\FormKitBundle\Attribute\FormKitConfig; use Nowo\FormKitBundle\Form\FormOptionsTrait; use Nowo\TagInputBundle\Form\TagType; @@ -14,6 +13,7 @@ use Nowo\TaskBoardBundle\Form\DataTransformer\MinutesToDurationTransformer; use Nowo\TaskBoardBundle\Form\DataTransformer\TagsToStringTransformer; use Nowo\TaskBoardBundle\TaskBoardBundle; +use Nowo\TaskBoardBundle\Validation\TaskConstraints; use Nowo\TiptapEditorBundle\Form\TiptapEditorType; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\ColorType; @@ -26,9 +26,6 @@ use Symfony\Component\Form\FormInterface; use Symfony\Component\OptionsResolver\OptionsResolver; use Symfony\Component\Validator\Constraints\Callback; -use Symfony\Component\Validator\Constraints\Length; -use Symfony\Component\Validator\Constraints\NotBlank; -use Symfony\Component\Validator\Constraints\Regex; use Symfony\Component\Validator\Context\ExecutionContextInterface; use function class_exists; @@ -41,10 +38,10 @@ final class TaskFormType extends AbstractType { use FormOptionsTrait; /** Since 1.15: the longest title (task_board_tasks.title is VARCHAR(255)). */ - public const TITLE_MAX = 255; + public const TITLE_MAX = TaskConstraints::TITLE_MAX; /** Since 1.14: a task colour is #rrggbb (the column holds 7 characters). */ - public const COLOR_PATTERN = '/^#[0-9a-fA-F]{6}$/D'; + public const COLOR_PATTERN = TaskConstraints::TASK_COLOR_PATTERN; public function buildForm(FormBuilderInterface $builder, array $options): void { @@ -54,10 +51,9 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $this->addText($builder, 'title', [ // Since 1.21: an empty or missing title is a NotBlank error, not a TypeError on the DTO (string). - 'empty_data' => '', - 'label' => 'task_board.form.task.title', - // Since 1.15: the column is VARCHAR(255) — a longer title is a form error, not a database error. - 'constraints' => [new NotBlank(), new Length(max: self::TITLE_MAX)], + 'empty_data' => '', + 'label' => 'task_board.form.task.title', + 'constraints' => TaskConstraints::taskTitle(), 'attr' => ['maxlength' => self::TITLE_MAX], ]); @@ -116,7 +112,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'constraints' => [new Callback(static function (mixed $startAt, ExecutionContextInterface $context): void { $form = $context->getObject(); $dueAt = $form instanceof FormInterface ? $form->getParent()?->get('dueAt')->getData() : null; - if ($startAt instanceof DateTimeInterface && $dueAt instanceof DateTimeInterface && $startAt > $dueAt) { + if (TaskConstraints::startsAfterDue($startAt, $dueAt)) { $context->buildViolation('task_board.form.task.start_after_due') ->setTranslationDomain('NowoTaskBoardBundle') ->addViolation(); @@ -153,7 +149,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $this->addWithDefaults($builder, 'color', HiddenType::class, [ 'label' => 'task_board.form.task.color', 'required' => false, - 'constraints' => [new Regex(pattern: self::COLOR_PATTERN)], + 'constraints' => TaskConstraints::taskColor(), ]); $this->addWithDefaults($builder, 'customColor', ColorType::class, [ 'label' => 'task_board.form.task.custom_color', diff --git a/src/Form/TaskLinkFormType.php b/src/Form/TaskLinkFormType.php index 90514732..d8a6a392 100644 --- a/src/Form/TaskLinkFormType.php +++ b/src/Form/TaskLinkFormType.php @@ -9,13 +9,11 @@ use Nowo\TaskBoardBundle\Dto\TaskLinkFormData; use Nowo\TaskBoardBundle\Enum\TaskLinkType; use Nowo\TaskBoardBundle\TaskBoardBundle; +use Nowo\TaskBoardBundle\Validation\TaskConstraints; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\EnumType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; -use Symfony\Component\Validator\Constraints\Length; -use Symfony\Component\Validator\Constraints\NotBlank; -use Symfony\Component\Validator\Constraints\Url; /** * @extends AbstractType @@ -38,24 +36,19 @@ public function buildForm(FormBuilderInterface $builder, array $options): void // Since 1.21: an empty or missing URL is a NotBlank error, not a TypeError on the DTO (string). 'empty_data' => '', 'label' => 'task_board.form.link.url', - 'constraints' => [ - new NotBlank(), - // Rendered as href: only http(s) — rejects javascript:, data:, vbscript:… - new Url(protocols: ['http', 'https'], requireTld: false), - new Length(max: 2048), - ], - 'attr' => ['maxlength' => 2048], + 'constraints' => TaskConstraints::linkUrl(), + 'attr' => ['maxlength' => 2048], ]); $this->addText($builder, 'label', [ 'label' => 'task_board.form.link.label', 'required' => false, - 'constraints' => [new Length(max: 255)], + 'constraints' => TaskConstraints::linkLabel(), 'attr' => ['maxlength' => 255], ]); $this->addText($builder, 'externalId', [ 'label' => 'task_board.form.link.external_id', 'required' => false, - 'constraints' => [new Length(max: 128)], + 'constraints' => TaskConstraints::linkExternalId(), 'attr' => ['maxlength' => 128], 'help' => 'task_board.form.link.external_id_help', ]); diff --git a/src/Form/TaskTimeEntryFormType.php b/src/Form/TaskTimeEntryFormType.php index 4de4e24d..cbaeb539 100644 --- a/src/Form/TaskTimeEntryFormType.php +++ b/src/Form/TaskTimeEntryFormType.php @@ -8,6 +8,7 @@ use Nowo\FormKitBundle\Form\FormOptionsTrait; use Nowo\TaskBoardBundle\Dto\TaskTimeEntryFormData; use Nowo\TaskBoardBundle\TaskBoardBundle; +use Nowo\TaskBoardBundle\Validation\TaskConstraints; use Override; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\Extension\Core\Type\DateType; @@ -15,9 +16,6 @@ use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; -use Symfony\Component\Validator\Constraints\Length; -use Symfony\Component\Validator\Constraints\NotNull; -use Symfony\Component\Validator\Constraints\Range; /** * Since 1.15: the "Log time" dialog of the task detail — minutes spent, the day and an optional note. @@ -30,7 +28,7 @@ final class TaskTimeEntryFormType extends AbstractType use FormOptionsTrait; /** At most a week of work in one entry. */ - public const MAX_MINUTES = 10080; + public const MAX_MINUTES = TaskConstraints::MAX_MINUTES; /** * @param FormBuilderInterface $builder @@ -43,21 +41,21 @@ public function buildForm(FormBuilderInterface $builder, array $options): void ->add('minutes', IntegerType::class, [ 'label' => 'task_board.time.form.minutes', 'help' => 'task_board.time.form.minutes_help', - 'constraints' => [new NotNull(), new Range(min: 1, max: self::MAX_MINUTES)], + 'constraints' => TaskConstraints::timeMinutes(), 'attr' => ['class' => 'form-control', 'min' => 1, 'max' => self::MAX_MINUTES, 'inputmode' => 'numeric'], ]) ->add('loggedAt', DateType::class, [ 'label' => 'task_board.time.form.date', 'widget' => 'single_text', 'input' => 'datetime_immutable', - 'constraints' => [new NotNull()], + 'constraints' => TaskConstraints::timeLoggedAt(), 'invalid_message' => 'task_board.form.task.due_invalid', 'attr' => ['class' => 'form-control'], ]) ->add('note', TextType::class, [ 'label' => 'task_board.time.form.note', 'required' => false, - 'constraints' => [new Length(max: 512)], + 'constraints' => TaskConstraints::timeNote(), 'attr' => ['class' => 'form-control', 'maxlength' => 512], ]); } diff --git a/src/Resources/config/services.yaml b/src/Resources/config/services.yaml index 71516f49..b9eed164 100644 --- a/src/Resources/config/services.yaml +++ b/src/Resources/config/services.yaml @@ -15,6 +15,7 @@ services: - '../../Dto/' - '../../Event/' - '../../Enum/' + - '../../Validation/' - '../../Exception/' - '../../ValueObject/' - '../../Import/Dto/' @@ -176,6 +177,11 @@ services: arguments: $defaultColumns: '%nowo_task_board.board.default_columns%' + # Since 2.2.0 (spec 213): the rules of comments, for every write path. + Nowo\TaskBoardBundle\Service\TaskCommentPolicy: + arguments: + $readersCanComment: '%nowo_task_board.comments.readers_can_comment%' + # Since 1.25: the erase cursor is signed with the application secret (audit FP-L3). Nowo\TaskBoardBundle\Service\TaskBoardEraser: arguments: diff --git a/src/Service/TaskBoardEditor.php b/src/Service/TaskBoardEditor.php new file mode 100644 index 00000000..fd97f1a4 --- /dev/null +++ b/src/Service/TaskBoardEditor.php @@ -0,0 +1,42 @@ + $board->getName() !== $name, + 'description' => $board->getDescription() !== $description, + ])); + if ($changed === []) { + return; + } + + $board->setName($name)->setDescription($description); + $this->boardRepository->save($board); + $this->eventDispatcher?->dispatch(new BoardUpdatedEvent($board, $actor, $changed), TaskBoardEvents::BOARD_UPDATED); + } +} diff --git a/src/Service/TaskBoardEraser.php b/src/Service/TaskBoardEraser.php index 6d812a63..356f4596 100644 --- a/src/Service/TaskBoardEraser.php +++ b/src/Service/TaskBoardEraser.php @@ -14,9 +14,11 @@ use Nowo\TaskBoardBundle\Event\TaskBoardEvents; use Nowo\TaskBoardBundle\Event\TaskErasedEvent; use Nowo\TaskBoardBundle\Exception\InvalidEraseCursorException; +use Nowo\TaskBoardBundle\Exception\TaskLinkedToProviderException; use Nowo\TaskBoardBundle\Security\EraseCursorSigner; use Nowo\TaskBoardBundle\Security\TaskBoardResourceAccessChecker; use Nowo\TaskBoardBundle\Sync\Entity\TaskSyncAttachment; +use Nowo\TaskBoardBundle\Sync\Entity\TaskSyncItem; use Nowo\TaskBoardBundle\Sync\TaskBoardSyncService; use Psr\Clock\ClockInterface; use Psr\Log\LoggerInterface; @@ -111,6 +113,85 @@ public function deleteBoard(TaskBoard $board, object $actor, int $batchSize = 20 return $this->erase($board, $actor, $batchSize, $cursor, true); } + /** + * Since 2.2.0 (spec 213): deletes one task and its subtasks, leaf-first through Doctrine (a database cascade on the + * parent would skip the `PreRemove` that keeps their time). Time is kept, detached. Returns how many were deleted. + * + * @throws AccessDeniedException when the actor may not manage the board + * @throws TaskLinkedToProviderException when the task or a subtask follows a provider: nothing is deleted + */ + public function eraseTask(Task $task, object $actor): int + { + $board = $task->getBoard(); + if (!$this->accessChecker->canManageBoard($actor, $board)) { + throw new AccessDeniedException('The task may not be erased.'); + } + $levels = [[$task]]; + while (($next = $this->subtasksOf($levels[array_key_last($levels)])) !== []) { + $levels[] = $next; + } + foreach ($levels as $level) { + $this->refuseLinked($level); + } + + $boardId = $board->getId(); + $deleted = 0; + foreach (array_reverse($levels) as $level) { + foreach ($level as $erased) { + $this->eventDispatcher?->dispatch(new TaskErasedEvent($erased->getId(), $boardId, $erased->getTitle(), [], count($erased->getTimeEntries())), TaskBoardEvents::TASK_ERASED); + $board->getTasks()->removeElement($erased); + $erased->getParent()?->getSubtasks()->removeElement($erased); + $this->entityManager->remove($erased); + ++$deleted; + } + $this->entityManager->flush(); + } + $this->logger->info('TaskBoard task erased', ['bundle' => 'nowo_task_board', 'board_id' => $boardId, 'tasks' => $deleted]); + + return $deleted; + } + + /** + * @param list $tasks + * + * @return list + */ + private function subtasksOf(array $tasks): array + { + /** @var list $subtasks */ + $subtasks = $this->entityManager->createQueryBuilder() + ->select('t') + ->from(Task::class, 't') + ->where('t.parent IN (:parents)') + ->setParameter('parents', $tasks) + ->getQuery() + ->getResult(); + + return $subtasks; + } + + /** + * @param list $tasks + */ + private function refuseLinked(array $tasks): void + { + if (!$this->syncService instanceof TaskBoardSyncService) { + return; + } + /** @var list $rows */ + $rows = $this->entityManager->createQueryBuilder() + ->select('IDENTITY(i.task) AS id') + ->from(TaskSyncItem::class, 'i') + ->where('i.task IN (:tasks)') + ->setParameter('tasks', $tasks) + ->setMaxResults(1) + ->getQuery() + ->getArrayResult(); + if (isset($rows[0])) { + throw TaskLinkedToProviderException::forTask((string) $rows[0]['id']); + } + } + private function erase(TaskBoard $board, object $actor, int $batchSize, ?EraseCursor $cursor, bool $deleteBoard): EraseProgress { if (!$this->accessChecker->canManageBoard($actor, $board)) { diff --git a/src/Service/TaskCommentPolicy.php b/src/Service/TaskCommentPolicy.php new file mode 100644 index 00000000..a631ed3a --- /dev/null +++ b/src/Service/TaskCommentPolicy.php @@ -0,0 +1,37 @@ +readersCanComment || $this->accessChecker->canEditTask($user, $task); + } + + public function mayEdit(object $user, Task $task, TaskComment $comment): bool + { + return TaskCommentManager::isAuthor($comment, $user) && $this->mayComment($user, $task); + } + + public function mayDelete(object $user, Task $task, TaskComment $comment): bool + { + return TaskCommentManager::isAuthor($comment, $user) || $this->accessChecker->canEditTask($user, $task); + } +} diff --git a/src/Service/TaskDependencyManager.php b/src/Service/TaskDependencyManager.php index c658c515..60c3a56b 100644 --- a/src/Service/TaskDependencyManager.php +++ b/src/Service/TaskDependencyManager.php @@ -92,6 +92,23 @@ public function remove(Task $task, string $dependencyId, object $user): bool return false; } + /** + * Since 2.2.0 (spec 213): the dependencies joining the two tasks, in either direction. + * + * @return list + */ + public function findBetween(Task $task, Task $other): array + { + $found = []; + foreach ([...$task->getOutgoingDependencies(), ...$task->getIncomingDependencies()] as $dependency) { + if ($dependency->getSourceTask()->getId() === $other->getId() || $dependency->getTargetTask()->getId() === $other->getId()) { + $found[] = $dependency; + } + } + + return $found; + } + /** * Every dependency of the given tasks, once. * diff --git a/src/Service/TaskManager.php b/src/Service/TaskManager.php index 29f2746c..aeed184b 100644 --- a/src/Service/TaskManager.php +++ b/src/Service/TaskManager.php @@ -446,6 +446,44 @@ private function nextPosition(TaskBoard $board, ?BoardColumn $column): int return $max + 1; } + /** + * Since 2.2.0 (spec 213): makes an archived task active again, asking the host's limit when it adds an active task (a + * completed one adds nothing). False when the task is not archived. + * + * @throws TaskReopenRefusedException when the host's limit refuses it: nothing changes + */ + public function restore(Task $task, object $user): bool + { + if (!$task->isArchived()) { + return false; + } + $refusal = $this->limits?->check($task->getBoard(), $task->isCompleted() ? 0 : 1, 0); + if ($refusal instanceof LimitRefusal) { + throw new TaskReopenRefusedException($refusal); + } + + $task->restore(); + $this->recordRestored($task, $user); + + return true; + } + + /** + * Since 2.2.0 (spec 213): archives a task by hand, with its history. False when it already is. + */ + public function archive(Task $task, object $user): bool + { + if ($task->isArchived()) { + return false; + } + + $task->archive(); + $this->changeRecorder->record($task, $user, TaskChangeType::Archived, null, 'manual'); + $this->taskRepository->save($task); + + return true; + } + /** * Since 1.23: records and saves a restored task. */ diff --git a/src/Validation/TaskConstraints.php b/src/Validation/TaskConstraints.php new file mode 100644 index 00000000..8ca36d84 --- /dev/null +++ b/src/Validation/TaskConstraints.php @@ -0,0 +1,147 @@ + */ + public static function taskTitle(): array + { + // Since 1.15: the column is VARCHAR(255) — a longer title is an error, not a database error. + return [new NotBlank(), new Length(max: self::TITLE_MAX)]; + } + + /** @return list */ + public static function taskColor(): array + { + return [new Regex(pattern: self::TASK_COLOR_PATTERN)]; + } + + /** @return list */ + public static function boardName(): array + { + return [new NotBlank(), new Length(max: self::BOARD_TEXT_MAX)]; + } + + /** @return list */ + public static function boardSlug(): array + { + return [new Length(max: self::BOARD_TEXT_MAX)]; + } + + /** @return list */ + public static function boardIcon(string $tooLongMessage, string $invalidMessage): array + { + return [ + new Length(max: BoardIcon::MAX_BYTES, countUnit: Length::COUNT_BYTES, maxMessage: $tooLongMessage), + new Regex(pattern: BoardIcon::PATTERN, message: $invalidMessage), + ]; + } + + /** @return list */ + public static function columnName(): array + { + return [new NotBlank(), new Length(max: self::COLUMN_NAME_MAX)]; + } + + /** @return list */ + public static function columnColor(): array + { + return [new Regex(pattern: self::COLOR_PATTERN)]; + } + + /** @return list */ + public static function commentBody(): array + { + return [new NotBlank(), new Length(max: TaskComment::MAX_LENGTH)]; + } + + /** @return list */ + public static function timeMinutes(): array + { + return [new NotNull(), new Range(min: 1, max: self::MAX_MINUTES)]; + } + + /** @return list */ + public static function timeLoggedAt(): array + { + return [new NotNull()]; + } + + /** @return list */ + public static function timeNote(): array + { + return [new Length(max: self::NOTE_MAX)]; + } + + /** @return list */ + public static function linkUrl(): array + { + return [ + new NotBlank(), + // Rendered as href: only http(s) — rejects javascript:, data:, vbscript:… + new Url(protocols: ['http', 'https'], requireTld: false), + new Length(max: self::URL_MAX), + ]; + } + + /** @return list */ + public static function linkLabel(): array + { + return [new Length(max: self::LABEL_MAX)]; + } + + /** @return list */ + public static function linkExternalId(): array + { + return [new Length(max: self::EXTERNAL_ID_MAX)]; + } + + /** The rule of a task's dates: a start after the due date is an error. Either may be absent. */ + public static function startsAfterDue(mixed $startAt, mixed $dueAt): bool + { + return $startAt instanceof DateTimeInterface && $dueAt instanceof DateTimeInterface && $startAt > $dueAt; + } +} diff --git a/tests/Functional/SharedWriteServicesTest.php b/tests/Functional/SharedWriteServicesTest.php new file mode 100644 index 00000000..998a0945 --- /dev/null +++ b/tests/Functional/SharedWriteServicesTest.php @@ -0,0 +1,212 @@ +board(); + $parent = $this->task($board, 'Parent'); + $child = $this->task($board, 'Child', parent: $parent); + $grandchild = $this->task($board, 'Grandchild', parent: $child); + $sibling = $this->task($board, 'Sibling'); + $logger = $this->service(TaskTimeLogger::class); + $onChild = $logger->log($child, $this->owner, 30, new DateTimeImmutable('2026-11-04'))->getId(); + $onLeaf = $logger->log($grandchild, $this->owner, 15, new DateTimeImmutable('2026-11-04'))->getId(); + $erased = []; + $this->dispatcher()->addListener(TaskBoardEvents::TASK_ERASED, static function (TaskErasedEvent $event) use (&$erased): void { + $erased[$event->title] = $event->manualTimeEntries; + }); + $siblingId = $sibling->getId(); + + self::assertSame(3, $this->eraser()->eraseTask($parent, $this->owner)); + + self::assertSame(['Grandchild' => 1, 'Child' => 1, 'Parent' => 0], $erased); + $this->em->clear(); + self::assertSame(1, $this->em->getRepository(Task::class)->count([])); + self::assertInstanceOf(Task::class, $this->em->find(Task::class, $siblingId)); + foreach ([$onChild => [30, 'Child'], $onLeaf => [15, 'Grandchild']] as $id => [$minutes, $title]) { + $entry = $this->em->find(TaskTimeEntry::class, $id); + self::assertInstanceOf(TaskTimeEntry::class, $entry, 'tracked time is never deleted'); + self::assertTrue($entry->isDetached()); + self::assertSame($minutes, $entry->getMinutes()); + self::assertSame($title, $entry->getTaskTitleSnapshot()); + } + } + + public function testEraseTaskNeedsTheBoardManageDecision(): void + { + $task = $this->task($this->board(), 'Stays'); + $this->dispatcher()->addListener(TaskBoardEvents::BOARD_ACCESS_CHECK, static function (BoardAccessCheckEvent $event): void { + if ($event->getAction() === BoardAccessCheckEvent::ACTION_MANAGE) { + $event->deny(); + } + }); + + try { + $this->eraser()->eraseTask($task, $this->owner); + self::fail('AccessDeniedException expected'); + } catch (AccessDeniedException) { + } + + $this->em->clear(); + self::assertInstanceOf(Task::class, $this->em->find(Task::class, $task->getId())); + } + + public function testRestoreAndArchiveRecordTheirHistoryAndAreIdempotent(): void + { + $task = $this->task($this->board(), 'Back and forth'); + $manager = $this->service(TaskManager::class); + + self::assertFalse($manager->restore($task, $this->owner), 'not archived'); + self::assertTrue($manager->archive($task, $this->owner)); + self::assertTrue($task->isArchived()); + self::assertFalse($manager->archive($task, $this->owner), 'already archived'); + self::assertTrue($manager->restore($task, $this->owner)); + self::assertFalse($task->isArchived()); + + $this->em->clear(); + $history = $this->em->getRepository(TaskChangeHistory::class); + self::assertCount(1, $history->findBy(['changeType' => TaskChangeType::Archived, 'newValue' => 'manual'])); + self::assertCount(1, $history->findBy(['changeType' => TaskChangeType::Restored, 'newValue' => 'manual'])); + } + + public function testTheCommentPolicyAnswersEachRole(): void + { + $task = $this->task($this->board(), 'Discussed'); + $reader = $this->user('reader@example.com'); + $readerId = $reader->getId(); + $this->dispatcher()->addListener(TaskBoardEvents::TASK_ACCESS_CHECK, static function (TaskAccessCheckEvent $event) use ($readerId): void { + $subject = $event->getSubject(); + if (method_exists($subject, 'getId') && $subject->getId() === $readerId) { + $event->grant(); + $event->markReadOnly(); + } + }); + $checker = $this->service(TaskBoardResourceAccessChecker::class); + $open = new TaskCommentPolicy($checker, true); + $closed = new TaskCommentPolicy($checker, false); + $byReader = new TaskComment($task, $reader, 'Mine'); + $byOwner = new TaskComment($task, $this->owner, 'Theirs'); + + self::assertTrue($open->mayComment($reader, $task)); + self::assertFalse($closed->mayComment($reader, $task)); + self::assertTrue($closed->mayComment($this->owner, $task), 'an editor always may'); + + self::assertTrue($open->mayEdit($reader, $task, $byReader)); + self::assertFalse($closed->mayEdit($reader, $task, $byReader), 'the author, while they may comment'); + self::assertFalse($open->mayEdit($this->owner, $task, $byReader), 'an editor does not edit someone else'); + + self::assertTrue($closed->mayDelete($reader, $task, $byReader), 'the author deletes'); + self::assertFalse($open->mayDelete($reader, $task, $byOwner)); + self::assertTrue($open->mayDelete($this->owner, $task, $byReader), 'an editor deletes'); + } + + public function testTheBoardEditorChangesWhatChangedAndKeepsTheSlug(): void + { + $board = $this->board(); + $slug = $board->getSlug(); + $events = []; + $editor = new TaskBoardEditor($this->service(TaskBoardRepositoryInterface::class), $this->dispatcher()); + $this->dispatcher()->addListener(TaskBoardEvents::BOARD_UPDATED, static function (BoardUpdatedEvent $event) use (&$events): void { + $events[] = $event->changed; + }); + + $editor->update($board, 'Renamed', null, $this->owner); + $editor->update($board, 'Renamed', 'Now with text', $this->owner); + $editor->update($board, 'Renamed', 'Now with text', $this->owner); + + self::assertSame([['name'], ['description']], $events); + $this->em->clear(); + $saved = $this->service(TaskBoardRepositoryInterface::class)->findById($board->getId()); + self::assertSame('Renamed', $saved?->getName()); + self::assertSame('Now with text', $saved?->getDescription()); + self::assertSame($slug, $saved?->getSlug()); + } + + public function testFindBetweenSeesBothDirectionsAndNothingElse(): void + { + $board = $this->board(); + $a = $this->task($board, 'A'); + $b = $this->task($board, 'B'); + $c = $this->task($board, 'C'); + $manager = $this->service(TaskDependencyManager::class); + $edge = $manager->add($a, $b, TaskDependencyType::Blocks, $this->owner); + + self::assertSame([$edge->getId()], array_map(static fn ($d): string => $d->getId(), $manager->findBetween($a, $b))); + self::assertSame([$edge->getId()], array_map(static fn ($d): string => $d->getId(), $manager->findBetween($b, $a))); + self::assertSame([], $manager->findBetween($a, $c)); + } + + public function testFormDataOfATaskIsTheTaskAsItStands(): void + { + $task = $this->task($this->board(), 'Full'); + $due = new DateTimeImmutable('2026-12-01'); + $task->setDescription('Text')->setPriority(TaskPriority::High)->setEstimatedMinutes(90)->setDueAt($due)->setTags(['api'])->setColor('#a1b2c3')->setStartAt($due); + + $data = TaskFormData::fromTask($task); + + self::assertSame('Full', $data->title); + self::assertSame('Text', $data->description); + self::assertSame(TaskPriority::High, $data->priority); + self::assertSame($task->getColumn()?->getId(), $data->columnId); + self::assertSame(90, $data->estimatedMinutes); + self::assertSame($due, $data->dueAt); + self::assertSame(['api'], $data->tags); + self::assertSame('#a1b2c3', $data->color); + self::assertSame($due, $data->startAt); + } + + private function eraser(): TaskBoardEraser + { + return $this->service('app.board_eraser'); + } + + private function dispatcher(): EventDispatcherInterface + { + return $this->service('event_dispatcher'); + } + + /** + * @template T of object + * + * @param class-string|string $id + * + * @return ($id is class-string ? T : mixed) + */ + private function service(string $id): mixed + { + return self::getContainer()->get($id); + } +} diff --git a/tests/Functional/Sync/SyncEraseTaskTest.php b/tests/Functional/Sync/SyncEraseTaskTest.php new file mode 100644 index 00000000..5d53756d --- /dev/null +++ b/tests/Functional/Sync/SyncEraseTaskTest.php @@ -0,0 +1,47 @@ +board(); + $this->put($this->remote('s1', 'Synced', 1)); + $this->sync($board); + $fresh = $this->freshBoard($board); + $local = $this->task($fresh, 'Local'); + $parent = $this->task($fresh, 'Local parent'); + $synced = $this->taskTitled('Synced'); + $synced->setParent($parent); + $this->em->flush(); + $eraser = self::getContainer()->get('app.board_eraser'); + self::assertInstanceOf(TaskBoardEraser::class, $eraser); + + foreach ([$synced, $parent] as $refused) { + try { + $eraser->eraseTask($refused, $this->owner); + self::fail('TaskLinkedToProviderException expected'); + } catch (TaskLinkedToProviderException) { + } + } + self::assertSame(1, $eraser->eraseTask($local, $this->owner), 'a local task on a linked board goes'); + + $this->em->clear(); + $titles = array_map(static fn (Task $task): string => $task->getTitle(), $this->em->getRepository(Task::class)->findAll()); + sort($titles); + self::assertSame(['Local parent', 'Synced'], $titles); + } +} diff --git a/tests/Unit/Validation/TaskConstraintsTest.php b/tests/Unit/Validation/TaskConstraintsTest.php new file mode 100644 index 00000000..68c1d4b6 --- /dev/null +++ b/tests/Unit/Validation/TaskConstraintsTest.php @@ -0,0 +1,80 @@ +, 1: mixed, 2: bool}> + */ + public static function values(): iterable + { + yield 'title' => [TaskConstraints::taskTitle(), 'Write the spec', true]; + yield 'title empty' => [TaskConstraints::taskTitle(), '', false]; + yield 'title too long' => [TaskConstraints::taskTitle(), str_repeat('x', 256), false]; + yield 'task colour' => [TaskConstraints::taskColor(), '#a1b2c3', true]; + yield 'task colour short form' => [TaskConstraints::taskColor(), '#abc', false]; + yield 'task colour breaking out' => [TaskConstraints::taskColor(), '#a1b2c3;x:y', false]; + yield 'board name' => [TaskConstraints::boardName(), 'Roadmap', true]; + yield 'board name empty' => [TaskConstraints::boardName(), '', false]; + yield 'board slug too long' => [TaskConstraints::boardSlug(), str_repeat('x', 256), false]; + yield 'icon' => [TaskConstraints::boardIcon('long', 'invalid'), "\u{1F680}", true]; + yield 'icon text' => [TaskConstraints::boardIcon('long', 'invalid'), 'rocket', false]; + yield 'column name' => [TaskConstraints::columnName(), 'Review', true]; + yield 'column name too long' => [TaskConstraints::columnName(), str_repeat('x', 129), false]; + yield 'column colour' => [TaskConstraints::columnColor(), '#abc', true]; + yield 'column colour script' => [TaskConstraints::columnColor(), 'red;background:url(x)', false]; + yield 'comment' => [TaskConstraints::commentBody(), 'Hello', true]; + yield 'comment too long' => [TaskConstraints::commentBody(), str_repeat('x', TaskComment::MAX_LENGTH + 1), false]; + yield 'minutes' => [TaskConstraints::timeMinutes(), 45, true]; + yield 'minutes zero' => [TaskConstraints::timeMinutes(), 0, false]; + yield 'minutes over a week' => [TaskConstraints::timeMinutes(), TaskConstraints::MAX_MINUTES + 1, false]; + yield 'minutes missing' => [TaskConstraints::timeMinutes(), null, false]; + yield 'logged at missing' => [TaskConstraints::timeLoggedAt(), null, false]; + yield 'note too long' => [TaskConstraints::timeNote(), str_repeat('x', 513), false]; + yield 'link' => [TaskConstraints::linkUrl(), 'https://example.com/spec', true]; + yield 'link without a tld' => [TaskConstraints::linkUrl(), 'http://intranet/spec', true]; + yield 'link javascript' => [TaskConstraints::linkUrl(), 'javascript:alert(1)', false]; + yield 'link data' => [TaskConstraints::linkUrl(), 'data:text/html,x', false]; + yield 'link label too long' => [TaskConstraints::linkLabel(), str_repeat('x', 256), false]; + yield 'link external id too long' => [TaskConstraints::linkExternalId(), str_repeat('x', 129), false]; + } + + /** + * @param list $constraints + */ + #[DataProvider('values')] + public function testARuleAcceptsOrRefuses(array $constraints, mixed $value, bool $valid): void + { + $violations = Validation::createValidator()->validate($value, $constraints); + + self::assertSame($valid, $violations->count() === 0); + } + + public function testAStartAfterTheDueDateIsTheOnlyDateError(): void + { + $early = new DateTimeImmutable('2026-11-06'); + $late = new DateTimeImmutable('2026-11-07'); + + self::assertTrue(TaskConstraints::startsAfterDue($late, $early)); + self::assertFalse(TaskConstraints::startsAfterDue($early, $late)); + self::assertFalse(TaskConstraints::startsAfterDue($early, $early)); + self::assertFalse(TaskConstraints::startsAfterDue(null, $early)); + self::assertFalse(TaskConstraints::startsAfterDue($early, null)); + } +} From b0abe6bc408a8f27cf1d9bce56527a3fa47237ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Franco?= Date: Sat, 7 Nov 2026 22:22:52 +0100 Subject: [PATCH 03/12] feat(api): add the Task API skeleton behind api.enabled (spec 213 S2) Off by default: no route, service, listener, parameter or table for a host that changes nothing. On: stateless routes under api.prefix, GET /v2/user and /v2/team, and what every later endpoint goes through: the request pipeline, TaskApiGuard (level, the person's access and where, per request), the capability table and one {err, ECODE} shape. The bundle stores no tokens. The hooks live in Api\Hook, the authenticator is required and every other default denies. It fails closed when the security user is not the token's user, as access checkers read the security token, and the build refuses a configuration that would open every board to every token. Audit status: re-verification pending. --- docs/API.md | 61 ++++ docs/CHANGELOG.md | 3 +- docs/CONFIGURATION.md | 5 + docs/SECURITY.md | 1 + docs/UPGRADING.md | 2 +- docs/USAGE.md | 5 + .../Controller/TaskApiIdentityController.php | 44 +++ src/Api/Hook/DenyingTaskApiAttachments.php | 22 ++ src/Api/Hook/DenyingTaskApiPlanGate.php | 16 + src/Api/Hook/DenyingTaskApiRateLimiter.php | 28 ++ src/Api/Hook/DenyingTaskApiTimeEntries.php | 34 +++ src/Api/Hook/EmptyTaskApiUsers.php | 29 ++ src/Api/Hook/EmptyTaskApiWorkspace.php | 34 +++ src/Api/Hook/TaskApiAttachmentsInterface.php | 23 ++ .../Hook/TaskApiAuthenticatorInterface.php | 22 ++ .../Hook/TaskApiIdempotencyStoreInterface.php | 26 ++ src/Api/Hook/TaskApiPlanGateInterface.php | 16 + src/Api/Hook/TaskApiRateLimiterInterface.php | 23 ++ src/Api/Hook/TaskApiTimeEntriesInterface.php | 30 ++ src/Api/Hook/TaskApiUsersInterface.php | 31 ++ src/Api/Hook/TaskApiWorkspaceInterface.php | 30 ++ src/Api/Operation/TaskApiReadOperations.php | 60 ++++ src/Api/Security/TaskApiGuard.php | 149 +++++++++ src/Api/Security/TaskApiRequestListener.php | 225 ++++++++++++++ .../Serializer/TaskApiIdentitySerializer.php | 42 +++ src/Api/TaskApiAuthFailure.php | 15 + src/Api/TaskApiCapability.php | 44 +++ src/Api/TaskApiCredential.php | 29 ++ src/Api/TaskApiLevel.php | 17 ++ src/Api/TaskApiPrincipal.php | 21 ++ src/Api/TaskApiRefusal.php | 39 +++ src/Api/TaskApiWhere.php | 86 ++++++ src/Api/Value/TaskApiAttachment.php | 19 ++ src/Api/Value/TaskApiIdempotencyClaim.php | 20 ++ src/Api/Value/TaskApiIdempotencyOutcome.php | 23 ++ src/Api/Value/TaskApiNewTimeEntry.php | 21 ++ src/Api/Value/TaskApiRateLimitState.php | 19 ++ src/Api/Value/TaskApiSpace.php | 19 ++ src/Api/Value/TaskApiTeam.php | 19 ++ src/Api/Value/TaskApiTimeEntry.php | 23 ++ src/Api/Value/TaskApiTimePage.php | 18 ++ src/Api/Value/TaskApiTimeQuery.php | 25 ++ src/Api/Value/TaskApiUser.php | 20 ++ src/DependencyInjection/Configuration.php | 38 +++ .../TaskBoardExtension.php | 53 ++++ src/Event/TaskApiWriteEvent.php | 31 ++ src/Event/TaskBoardEvents.php | 3 + src/Resources/config/services.yaml | 2 + src/Resources/config/services_api.yaml | 30 ++ src/Routing/TaskBoardRouteLoader.php | 34 +++ tests/Functional/Api/ApiDefaultsDenyTest.php | 30 ++ tests/Functional/Api/ApiGuardTest.php | 231 ++++++++++++++ tests/Functional/Api/ApiOffHostTest.php | 140 +++++++++ tests/Functional/Api/ApiPipelineTest.php | 284 ++++++++++++++++++ tests/Functional/Api/ApiTestCase.php | 85 ++++++ .../Functional/Api/ApiWithoutFirewallTest.php | 26 ++ .../Api/App/ApiFirewallAuthenticator.php | 42 +++ tests/Functional/Api/App/ApiKernel.php | 141 +++++++++ tests/Functional/Api/App/ApiTestHost.php | 159 ++++++++++ tests/Unit/Api/TaskApiExtensionTest.php | 114 +++++++ tests/Unit/Api/TaskApiValuesTest.php | 194 ++++++++++++ 61 files changed, 3073 insertions(+), 2 deletions(-) create mode 100644 docs/API.md create mode 100644 src/Api/Controller/TaskApiIdentityController.php create mode 100644 src/Api/Hook/DenyingTaskApiAttachments.php create mode 100644 src/Api/Hook/DenyingTaskApiPlanGate.php create mode 100644 src/Api/Hook/DenyingTaskApiRateLimiter.php create mode 100644 src/Api/Hook/DenyingTaskApiTimeEntries.php create mode 100644 src/Api/Hook/EmptyTaskApiUsers.php create mode 100644 src/Api/Hook/EmptyTaskApiWorkspace.php create mode 100644 src/Api/Hook/TaskApiAttachmentsInterface.php create mode 100644 src/Api/Hook/TaskApiAuthenticatorInterface.php create mode 100644 src/Api/Hook/TaskApiIdempotencyStoreInterface.php create mode 100644 src/Api/Hook/TaskApiPlanGateInterface.php create mode 100644 src/Api/Hook/TaskApiRateLimiterInterface.php create mode 100644 src/Api/Hook/TaskApiTimeEntriesInterface.php create mode 100644 src/Api/Hook/TaskApiUsersInterface.php create mode 100644 src/Api/Hook/TaskApiWorkspaceInterface.php create mode 100644 src/Api/Operation/TaskApiReadOperations.php create mode 100644 src/Api/Security/TaskApiGuard.php create mode 100644 src/Api/Security/TaskApiRequestListener.php create mode 100644 src/Api/Serializer/TaskApiIdentitySerializer.php create mode 100644 src/Api/TaskApiAuthFailure.php create mode 100644 src/Api/TaskApiCapability.php create mode 100644 src/Api/TaskApiCredential.php create mode 100644 src/Api/TaskApiLevel.php create mode 100644 src/Api/TaskApiPrincipal.php create mode 100644 src/Api/TaskApiRefusal.php create mode 100644 src/Api/TaskApiWhere.php create mode 100644 src/Api/Value/TaskApiAttachment.php create mode 100644 src/Api/Value/TaskApiIdempotencyClaim.php create mode 100644 src/Api/Value/TaskApiIdempotencyOutcome.php create mode 100644 src/Api/Value/TaskApiNewTimeEntry.php create mode 100644 src/Api/Value/TaskApiRateLimitState.php create mode 100644 src/Api/Value/TaskApiSpace.php create mode 100644 src/Api/Value/TaskApiTeam.php create mode 100644 src/Api/Value/TaskApiTimeEntry.php create mode 100644 src/Api/Value/TaskApiTimePage.php create mode 100644 src/Api/Value/TaskApiTimeQuery.php create mode 100644 src/Api/Value/TaskApiUser.php create mode 100644 src/Event/TaskApiWriteEvent.php create mode 100644 src/Resources/config/services_api.yaml create mode 100644 tests/Functional/Api/ApiDefaultsDenyTest.php create mode 100644 tests/Functional/Api/ApiGuardTest.php create mode 100644 tests/Functional/Api/ApiOffHostTest.php create mode 100644 tests/Functional/Api/ApiPipelineTest.php create mode 100644 tests/Functional/Api/ApiTestCase.php create mode 100644 tests/Functional/Api/ApiWithoutFirewallTest.php create mode 100644 tests/Functional/Api/App/ApiFirewallAuthenticator.php create mode 100644 tests/Functional/Api/App/ApiKernel.php create mode 100644 tests/Functional/Api/App/ApiTestHost.php create mode 100644 tests/Unit/Api/TaskApiExtensionTest.php create mode 100644 tests/Unit/Api/TaskApiValuesTest.php diff --git a/docs/API.md b/docs/API.md new file mode 100644 index 00000000..cd8f90b5 --- /dev/null +++ b/docs/API.md @@ -0,0 +1,61 @@ +# Task API (since 2.2, in progress) + +A REST API shaped like ClickUp API v2, for scripts and agents with a personal token (spec 213). **Audit status: re-verification pending.** So far only the skeleton exists: `GET /v2/user` and `GET /v2/team`. The rest arrives before 2.2.0 is tagged, along with the OpenAPI document and the list of differences from ClickUp. + +## Off by default + +With `api.enabled: false` a host gets no route, service, listener, parameter or table. + +## What a host provides + +The bundle stores no tokens. `api.hooks.authenticator` is required with `enabled: true`; every other hook has a default, and every default denies. + +| `api.hooks.*` | Interface in `Nowo\TaskBoardBundle\Api\Hook` | Without it | +|---|---|---| +| `authenticator` | `TaskApiAuthenticatorInterface` | The container does not build | +| `rate_limiter` | `TaskApiRateLimiterInterface` | Every request 429 | +| `plan_gate` | `TaskApiPlanGateInterface` | 403 `PLAN_001` | +| `workspace` | `TaskApiWorkspaceInterface` | No teams or spaces, every board 404 | +| `users` | `TaskApiUsersInterface` | Nobody is known | +| `time_entries` | `TaskApiTimeEntriesInterface` (list and log; tracked time is never deleted through the API) | 403 `TIME_001` | +| `attachments` | `TaskApiAttachmentsInterface` | 403 `FILE_001` | +| `idempotency_store` | `TaskApiIdempotencyStoreInterface` | The bundle's store, when writes arrive | + +A listener on `TaskBoardEvents::API_WRITE` (`TaskApiWriteEvent`) audits each write. + +## Firewall + +The API must sit behind a **stateless firewall** whose security user is the token's user, as the access checkers read the security token. `security: false` answers 500 `SERVER_001` on every request. For a bad token the firewall lets the request through with no user, and adds no redirecting `access_control` on the path: the bundle answers, so all refusals share one shape. + +```yaml +nowo_task_board: + api: + enabled: true + prefix: /api/tasks + host_decides_access: true # only with security.resource_scoping: false, when your listeners decide access + hooks: + authenticator: App\Api\TaskApiAuthenticator +``` + +The build refuses `security.allow_unauthenticated: true`, and `security.resource_scoping: false` without `host_decides_access: true`: both would open every board to every token. + +## Access + +Each request: level ∩ the person's access ∩ where. In this order, and the first failure answers: + +| Check | Answer | +|---|---| +| Out of reach: missing, outside where, no space for its tenant, no view access, for My tasks not its own | 404 `ACCESS_001`, always the same | +| The tasks module is not in the plan | 403 `PLAN_001` | +| The level may not | 403 `TOKEN_001` | +| The person may not | 403 `ACCESS_002` | + +Levels are not a chain: *Read* changes nothing, while *My tasks* works on its own tasks. See `TaskApiCapability`. + +## Errors + +`{"err": "...", "ECODE": "..."}`, with `OAUTH_017` (401, no header), `OAUTH_019` (401, token not valid, one answer for every reason), `API_001` (404, the instance switch), `RATE_001` and `RATE_002` (429), and `SERVER_001` (500, no detail). Answers carry `X-RateLimit-Limit`, `-Remaining`, `-Reset` (Unix seconds) and `Cache-Control: no-store, private`. The credential is read only from the `Authorization` header, with or without `Bearer`, and never logged. + +## Known differences from the UI + +Task text is returned raw: an API reader sees the alt text and id of an inline image the UI hides. The file itself stays behind the host's download check. diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f99917f1..d47970a5 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -85,10 +85,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -Target: 2.2.0, spec 213 (Task API). Audit status: re-verification pending. No behaviour, schema or configuration change for a host. +Target: 2.2.0, spec 213 (Task API). Audit status: re-verification pending. Opt-in: with `api.enabled: false` (the default) no route, service, listener, parameter or table changes, and the UI's output stays as it was. See [API](API.md). ### Added +- **Task API, skeleton (`api.*`).** `GET /v2/user`, `GET /v2/team` under `api.prefix`; the request pipeline (failed-token limit, authenticator, security user, rate limit), `Api\Security\TaskApiGuard` (level ∩ access ∩ where), `Api\TaskApiCapability`, one `{err, ECODE}` shape. Hooks in `Api\Hook`, where every default denies; event `TaskBoardEvents::API_WRITE`. The build refuses `security.allow_unauthenticated` and, without `api.host_decides_access`, `security.resource_scoping: false`. - `Service\TaskManager::restore()` and `archive()`, with their history; `restore()` asks the host's limit and throws `TaskReopenRefusedException` when it refuses. - `Service\TaskBoardEraser::eraseTask()`: one task and its subtasks, leaf-first through Doctrine, tracked time kept and detached. Needs the board's manage decision; a task that follows a provider, or has a subtask that does, is refused with `TaskLinkedToProviderException`. - `Service\TaskBoardEditor::update()`: name and description of a board, the slug unchanged, `TaskBoardEvents::BOARD_UPDATED` (`BoardUpdatedEvent`). diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 606d378d..651b1d56 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -6,6 +6,7 @@ - [Database](#database) - [Routes](#routes) - [Why a route loader instead of `#[Route]` attributes (REQ-SF-004)](#why-a-route-loader-instead-of-route-attributes-req-sf-004) +- [Task API (since 2.2)](#task-api-since-22) - [Security](#security) - [Firewall](#firewall) - [Access checker](#access-checker) @@ -143,6 +144,10 @@ from this configuration, not declared with `#[Route]` attributes on `TaskBoardMa Route names, paths, methods and the prefix are unchanged by 1.21. +## Task API (since 2.2) + +Off by default. `api.enabled`, `api.prefix`, `api.host_decides_access`, `api.quota_listener_required`, `api.max_body_bytes` and `api.hooks.*`: see [API](API.md). + ## Security ### Firewall diff --git a/docs/SECURITY.md b/docs/SECURITY.md index 11f79599..46faeeee 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -28,6 +28,7 @@ | **Task forms** | Symfony forms for board/task creation and updates (title, description, links, members). | | **Configuration** | `nowo_task_board` YAML (routes, table prefix, access checker, team resolver). | | **Provider sync (since 1.23, opt-in)** | Server-side calls to a task provider's API with the host's token, file downloads from the provider's attachment hosts, remote text stored as Markdown. | +| **Task API (since 2.2, opt-in)** | Stateless routes under `api.prefix`, off by default. Token authentication is the host's; `Api\Security\TaskApiRequestListener` and `TaskApiGuard` fail closed. See [API](API.md). Audit status: re-verification pending. | | **TimeTrack bridge** | Optional. When TimeTrack is installed, task provider exposes trackable tasks; access gated by `TaskAccessGuard`. | ## Threats and mitigations diff --git a/docs/UPGRADING.md b/docs/UPGRADING.md index d228b9c6..8b02ce9e 100644 --- a/docs/UPGRADING.md +++ b/docs/UPGRADING.md @@ -154,7 +154,7 @@ Hosting FrankenPHP with **kernel not reset** between requests: see [FRANKENPHP-W ## Unreleased -Target: 2.2.0, spec 213. Audit status: re-verification pending. No schema or configuration change, and the UI's output stays as it was. +Target: 2.2.0, spec 213. Audit status: re-verification pending. No schema or configuration change is needed, and the UI's output stays as it was. The Task API is off unless you set `api.enabled: true`: see [API](API.md). A host that builds `TaskBoardManageController` by hand instead of through the container: the `$syncLimits` argument is gone, and the limit a restore asks is the one given to `TaskManager` (`$limits`). diff --git a/docs/USAGE.md b/docs/USAGE.md index 3108f6be..56c4cd0d 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -30,6 +30,7 @@ - [No inline styles](#no-inline-styles) - [Look and feel: ClickUp-like views, theming and icons (since 1.12)](#look-and-feel-clickup-like-views-theming-and-icons-since-112) - [Theming with design tokens](#theming-with-design-tokens) +- [Task API (since 2.2)](#task-api-since-22) - [Host hooks (since 2.0.0)](#host-hooks-since-200) - [Icons](#icons) - [Inline new task](#inline-new-task) @@ -236,6 +237,10 @@ Overrides and demos must not reintroduce raw `
` / `` / `