diff --git a/.githooks/pre-commit b/.githooks/pre-commit index bc6ea31..7e5a4de 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -4,7 +4,7 @@ setopt ERR_EXIT PIPE_FAIL NOUNSET # Collect staged PHP and Twig files STAGED_PHP=() while IFS= read -r file; do - [[ -n "$file" ]] && STAGED_PHP+=("$file") + [[ -n "$file" && "$file" != "config/reference.php" ]] && STAGED_PHP+=("$file") done < <(git diff --cached --name-only --diff-filter=ACMR | grep -E '\.php$' || true) STAGED_TWIG=() @@ -32,7 +32,7 @@ if [[ ${#STAGED_PHP[@]} -gt 0 ]]; then git add "${STAGED_PHP[@]}" echo " → PHP-CS-Fixer" - "${DOCKER_CMD[@]}" vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php "${STAGED_PHP[@]}" + "${DOCKER_CMD[@]}" vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php --path-mode=intersection "${STAGED_PHP[@]}" git add "${STAGED_PHP[@]}" echo " → PHPStan" diff --git a/CLAUDE.md b/CLAUDE.md index 4827ccf..8897dc6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -148,6 +148,11 @@ tests/ - Coverage excluded from: `src/DataFixtures/` - Test environment: `APP_ENV=test` (set in phpunit.dist.xml) +### Testing Conventions (TDD) +- **Write the failing test first.** When fixing any PHP-reachable bug, write a PHPUnit test that reproduces the failure before touching the production code. Fix the code until the test passes. +- Only skip a test if the bug is purely in JavaScript/frontend where PHPUnit cannot reach it. +- Follow the pattern in `tests/Controller/Backoffice/` for controller/integration tests: log in, GET for CSRF token, POST form data, assert redirect, clear entity manager, assert DB state. + ### Code Style & Standards - **PHP-CS-Fixer**: Symfony ruleset + risky rules enabled - Strict types declaration required diff --git a/Dockerfile b/Dockerfile index 75b90d6..967e614 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,7 +32,6 @@ RUN set -eux; \ opcache \ zip \ gd \ - excimer \ ; # https://getcomposer.org/doc/03-cli.md#composer-allow-superuser diff --git a/assets/backoffice.js b/assets/backoffice.js index de391c0..af1dcdf 100644 --- a/assets/backoffice.js +++ b/assets/backoffice.js @@ -1,6 +1,7 @@ import 'bootstrap/dist/css/bootstrap.min.css'; import 'bootstrap-icons/font/bootstrap-icons.min.css'; import './styles/backoffice.scss'; +import '@hotwired/turbo'; import './stimulus.js'; import './bootstrap.js'; import * as Sentry from '@sentry/browser'; @@ -16,11 +17,13 @@ const effectiveDsn = dsn || 'https://0@o0.ingest.sentry.io/0'; const feedbackIntegration = Sentry.feedbackIntegration({ colorScheme: 'system', - showName: true, + showName: false, showEmail: true, - isNameRequired: false, isEmailRequired: false, autoInject: false, + triggerLabel: 'Report feedback', + formTitle: 'Report Feedback', + submitButtonLabel: 'Send Feedback', }); Sentry.init({ diff --git a/assets/controllers/bo/form_collection_controller.js b/assets/controllers/bo/form_collection_controller.js index e7a7d4c..e4e973a 100644 --- a/assets/controllers/bo/form_collection_controller.js +++ b/assets/controllers/bo/form_collection_controller.js @@ -6,8 +6,31 @@ export default class extends Controller { connect() { this.index = this.collectionTarget.children.length; - this._setupDrag(); this._syncOrdering(); + + if (this.index === 0) { + this.addItem(); + } + + // `submit` fires on the ancestor
, which is outside this controller's + // subtree. Stimulus data-action only works within the controller element, so + // addEventListener on the form is the only option here. + this._form = this.element.closest('form'); + if (this._form) { + this._submitHandler = () => { + [...this.collectionTarget.children].forEach(item => { + const input = item.querySelector('input[type="text"]'); + if (input && input.value.trim() === '') item.remove(); + }); + }; + this._form.addEventListener('submit', this._submitHandler); + } + } + + disconnect() { + if (this._form && this._submitHandler) { + this._form.removeEventListener('submit', this._submitHandler); + } } addItem() { @@ -15,13 +38,13 @@ export default class extends Controller { item.innerHTML = this.prototypeValue.replace(/__name__/g, this.index); const el = item.firstElementChild; this.collectionTarget.appendChild(el); - this._makeDraggable(el); this.index++; this._syncOrdering(); } removeItem(event) { event.target.closest('[data-collection-item]').remove(); + this._notifyChange(); } sortAlphabetically() { @@ -33,6 +56,7 @@ export default class extends Controller { }); items.forEach(item => this.collectionTarget.appendChild(item)); this._syncOrdering(); + this._notifyChange(); } randomize() { @@ -43,56 +67,64 @@ export default class extends Controller { } items.forEach(item => this.collectionTarget.appendChild(item)); this._syncOrdering(); + this._notifyChange(); + } + + autoExpand(event) { + if (event.target.type !== 'text') return; + const item = event.target.closest('[data-collection-item]'); + const last = [...this.collectionTarget.children].at(-1); + if (item && item === last && event.target.value.trim() !== '') { + this.addItem(); + } } // — drag-and-drop — - _setupDrag() { - [...this.collectionTarget.children].forEach(el => this._makeDraggable(el)); + dragStart(event) { + this._dragging = event.currentTarget.closest('[data-collection-item]'); + this._dragging.classList.add('opacity-50'); + event.dataTransfer.effectAllowed = 'move'; } - _makeDraggable(el) { - const handle = el.querySelector('[data-drag-handle]'); - if (!handle) return; + dragEnd(event) { + event.currentTarget.closest('[data-collection-item]').classList.remove('opacity-50'); + this._dragging = null; + this.collectionTarget.querySelectorAll('[data-collection-item]').forEach(i => + i.classList.remove('border-top', 'border-bottom', 'border-primary'), + ); + } - handle.setAttribute('draggable', 'true'); + dragOver(event) { + event.preventDefault(); + const el = event.currentTarget; + if (!this._dragging || this._dragging === el) return; + event.dataTransfer.dropEffect = 'move'; + const rect = el.getBoundingClientRect(); + const isBottom = event.clientY > rect.top + rect.height / 2; + el.classList.toggle('border-top', !isBottom); + el.classList.toggle('border-bottom', isBottom); + el.classList.add('border-primary'); + } - handle.addEventListener('dragstart', (e) => { - this._dragging = el; - el.classList.add('opacity-50'); - e.dataTransfer.effectAllowed = 'move'; - }); + dragLeave(event) { + event.currentTarget.classList.remove('border-top', 'border-bottom', 'border-primary'); + } - handle.addEventListener('dragend', () => { - this._dragging = null; - el.classList.remove('opacity-50'); - this.collectionTarget.querySelectorAll('[data-collection-item]').forEach(i => i.classList.remove('border-top', 'border-bottom', 'border-primary')); - }); + drop(event) { + event.preventDefault(); + const el = event.currentTarget; + el.classList.remove('border-top', 'border-bottom', 'border-primary'); + if (!this._dragging || this._dragging === el) return; + const rect = el.getBoundingClientRect(); + const isBottom = event.clientY > rect.top + rect.height / 2; + this.collectionTarget.insertBefore(this._dragging, isBottom ? el.nextSibling : el); + this._syncOrdering(); + this._notifyChange(); + } - el.addEventListener('dragover', (e) => { - e.preventDefault(); - if (!this._dragging || this._dragging === el) return; - e.dataTransfer.dropEffect = 'move'; - const rect = el.getBoundingClientRect(); - const isBottom = e.clientY > rect.top + rect.height / 2; - el.classList.toggle('border-top', !isBottom); - el.classList.toggle('border-bottom', isBottom); - el.classList.add('border-primary'); - }); - - el.addEventListener('dragleave', () => { - el.classList.remove('border-top', 'border-bottom', 'border-primary'); - }); - - el.addEventListener('drop', (e) => { - e.preventDefault(); - el.classList.remove('border-top', 'border-bottom', 'border-primary'); - if (!this._dragging || this._dragging === el) return; - const rect = el.getBoundingClientRect(); - const isBottom = e.clientY > rect.top + rect.height / 2; - this.collectionTarget.insertBefore(this._dragging, isBottom ? el.nextSibling : el); - this._syncOrdering(); - }); + _notifyChange() { + this.element.dispatchEvent(new Event('change', {bubbles: true})); } _syncOrdering() { diff --git a/assets/controllers/bo/modal_controller.js b/assets/controllers/bo/modal_controller.js new file mode 100644 index 0000000..543d5df --- /dev/null +++ b/assets/controllers/bo/modal_controller.js @@ -0,0 +1,49 @@ +import {Controller} from '@hotwired/stimulus'; +import {Modal} from 'bootstrap'; +import {visit} from '@hotwired/turbo'; + +export default class extends Controller { + static targets = ['modal', 'frame']; + + open(event) { + event.preventDefault(); + const {src, modalTitle} = event.currentTarget.dataset; + if (modalTitle) { + const titleEl = this.modalTarget.querySelector('.modal-title'); + if (titleEl) titleEl.textContent = modalTitle; + } + this.resetDirty(); + this.frameTarget.innerHTML = '
'; + this.frameTarget.removeAttribute('src'); + this.frameTarget.setAttribute('src', src); + Modal.getOrCreateInstance(this.modalTarget).show(); + } + + frameSubmitEnd(event) { + if (event.detail.success) { + Modal.getOrCreateInstance(this.modalTarget).hide(); + visit(window.location.href); + } + } + + markDirty() { + if (this._dirty) return; + this._dirty = true; + // Using _config instead of preventDefault on hide.bs.modal because we need + // to block only user-triggered dismissal (backdrop click, Escape key) while + // keeping programmatic hide() working — frameSubmitEnd() calls hide() after + // a successful save and must not be blocked. _config.backdrop/keyboard is + // the correct primitive for that distinction and has been stable across all + // Bootstrap 5.x releases. + const modal = Modal.getOrCreateInstance(this.modalTarget); + modal._config.backdrop = 'static'; + modal._config.keyboard = false; + } + + resetDirty() { + this._dirty = false; + const modal = Modal.getOrCreateInstance(this.modalTarget); + modal._config.backdrop = true; + modal._config.keyboard = true; + } +} diff --git a/assets/controllers/bo/question_list_controller.js b/assets/controllers/bo/question_list_controller.js new file mode 100644 index 0000000..294cf18 --- /dev/null +++ b/assets/controllers/bo/question_list_controller.js @@ -0,0 +1,123 @@ +import {Controller} from '@hotwired/stimulus'; + +export default class extends Controller { + static targets = ['list', 'item', 'status']; + static values = { + reorderUrl: String, + csrf: String, + savedLabel: String, + errorLabel: String, + errorHint: String, + }; + + connect() { + this._locked = false; + } + + dragStart(event) { + const item = event.currentTarget.closest('[data-bo--question-list-target="item"]'); + this._dragging = item; + event.dataTransfer.effectAllowed = 'move'; + setTimeout(() => item.classList.add('opacity-50'), 0); + } + + dragEnd(event) { + const item = event.currentTarget.closest('[data-bo--question-list-target="item"]'); + item.classList.remove('opacity-50'); + this._dragging = null; + this._removePlaceholder(); + } + + dragOver(event) { + event.preventDefault(); + if (!this._dragging) return; + event.dataTransfer.dropEffect = 'move'; + + const target = event.target.closest('[data-bo--question-list-target="item"]'); + if (!target || target === this._dragging) return; + + const rect = target.getBoundingClientRect(); + const insertBefore = event.clientY > rect.top + rect.height / 2 ? target.nextSibling : target; + + if (!this._placeholder) { + this._placeholder = document.createElement('div'); + this._placeholder.className = 'bg-primary rounded mb-2'; + this._placeholder.style.height = '3px'; + } + + if (this._placeholder.nextSibling !== insertBefore) { + this.listTarget.insertBefore(this._placeholder, insertBefore); + } + } + + dragLeave(event) { + if (!event.relatedTarget || !this.listTarget.contains(event.relatedTarget)) { + this._removePlaceholder(); + } + } + + async drop(event) { + event.preventDefault(); + if (!this._dragging || !this._placeholder || this._locked) return; + this.listTarget.insertBefore(this._dragging, this._placeholder); + this._removePlaceholder(); + await this._persistOrder(); + } + + _removePlaceholder() { + if (this._placeholder) { + this._placeholder.remove(); + this._placeholder = null; + } + } + + _setStatus(state) { + if (!this.hasStatusTarget) return; + const el = this.statusTarget; + el.classList.remove('d-none', 'text-bg-success', 'text-bg-danger', 'text-bg-warning'); + if (state === 'saving') { + el.classList.add('text-bg-warning'); + el.textContent = '…'; + } else if (state === 'saved') { + el.classList.add('text-bg-success'); + el.textContent = this.savedLabelValue || 'Saved'; + } else if (state === 'error') { + el.classList.add('text-bg-danger'); + el.textContent = this.errorLabelValue || 'Error'; + } + } + + async _persistOrder() { + this._setStatus('saving'); + + const params = new URLSearchParams(); + params.append('_token', this.csrfValue); + this.itemTargets.forEach((el, i) => { + params.append('ordering[]', el.dataset.questionId); + const numberEl = el.querySelector('[data-question-number]'); + if (numberEl) numberEl.textContent = String(i + 1); + }); + + for (let attempt = 0; attempt < 2; attempt++) { + try { + const res = await fetch(this.reorderUrlValue, {method: 'POST', body: params}); + if (res.ok) { + this._setStatus('saved'); + return; + } + } catch { + // network error — retry on first attempt + } + } + + this._locked = true; + this._setStatus('error'); + + const alert = document.createElement('div'); + alert.className = 'alert alert-danger alert-dismissible mt-3'; + alert.setAttribute('role', 'alert'); + const hint = this.errorHintValue || 'Refresh the page to try again.'; + alert.innerHTML = `${this.errorLabelValue || 'Error saving order'} — ${hint} `; + this.listTarget.after(alert); + } +} diff --git a/assets/styles/backoffice.scss b/assets/styles/backoffice.scss index 4ab8c9f..cb69351 100644 --- a/assets/styles/backoffice.scss +++ b/assets/styles/backoffice.scss @@ -1,3 +1,5 @@ .col-result-xs { width: 10%; } .col-result-sm { width: 15%; } .col-result-md { width: 20%; } + +.modal-content > turbo-frame { display: contents; } diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 6cc4cc2..b58f752 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -30,7 +30,7 @@ security: access_control: - { path: ^/admin, roles: ROLE_ADMIN } - - { path: ^/backoffice, roles: ROLE_USER } + - { path: ^/backoffice, roles: IS_AUTHENTICATED } when@test: security: diff --git a/config/reference.php b/config/reference.php index 27b09bf..dad68a9 100644 --- a/config/reference.php +++ b/config/reference.php @@ -1,7 +1,5 @@ createForm(BankQuestionFormType::class, $bankQuestion, ['season' => $season]); + $isTurboFrame = $request->headers->has('Turbo-Frame'); + + $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, [ + 'season' => $season, + 'action' => $this->generateUrl('tvdt_backoffice_question_bank_new', ['seasonCode' => $season->seasonCode]), + ]); $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { - $this->applyAnswerOrdering($bankQuestion); $season->addBankQuestion($bankQuestion); $this->em->persist($bankQuestion); $this->em->flush(); $this->addFlash(FlashType::Success, $this->translator->trans('Question added to the question bank')); + if ($isTurboFrame) { + return new Response(''); + } + return $this->redirectToRoute('tvdt_backoffice_question_bank', ['seasonCode' => $season->seasonCode]); } - return $this->render('backoffice/question_bank/form.html.twig', [ + $template = $isTurboFrame + ? 'backoffice/question_bank/_frame.html.twig' + : 'backoffice/question_bank/form.html.twig'; + + $response = $this->render($template, [ 'season' => $season, 'form' => $form, 'bankQuestion' => null, ]); + + if ($form->isSubmitted()) { + $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY); + } + + return $response; } #[IsGranted(SeasonVoter::EDIT, subject: 'season')] @@ -120,7 +138,15 @@ class QuestionBankController extends AbstractController { $this->assertSameSeason($season, $bankQuestion->season); - $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, ['season' => $season]); + $isTurboFrame = $request->headers->has('Turbo-Frame'); + + $form = $this->createForm(BankQuestionFormType::class, $bankQuestion, [ + 'season' => $season, + 'action' => $this->generateUrl('tvdt_backoffice_question_bank_edit', [ + 'seasonCode' => $season->seasonCode, + 'bankQuestion' => $bankQuestion->id, + ]), + ]); $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { @@ -130,14 +156,28 @@ class QuestionBankController extends AbstractController $this->addFlash(FlashType::Success, $this->translator->trans('Question updated')); + if ($isTurboFrame) { + return new Response(''); + } + return $this->redirectToRoute('tvdt_backoffice_question_bank', ['seasonCode' => $season->seasonCode]); } - return $this->render('backoffice/question_bank/form.html.twig', [ + $template = $isTurboFrame + ? 'backoffice/question_bank/_frame.html.twig' + : 'backoffice/question_bank/form.html.twig'; + + $response = $this->render($template, [ 'season' => $season, 'form' => $form, 'bankQuestion' => $bankQuestion, ]); + + if ($form->isSubmitted()) { + $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY); + } + + return $response; } #[IsCsrfTokenValid('delete_bank_question')] @@ -349,14 +389,6 @@ class QuestionBankController extends AbstractController } } - private function applyAnswerOrdering(BankQuestion $bankQuestion): void - { - $ordering = 1; - foreach ($bankQuestion->answers as $answer) { - $answer->ordering = $ordering++; - } - } - private function syncUsagesAfterEdit(BankQuestion $bankQuestion): void { $pendingNames = []; diff --git a/src/Controller/Backoffice/QuizController.php b/src/Controller/Backoffice/QuizController.php index 275e1d5..7ee7460 100644 --- a/src/Controller/Backoffice/QuizController.php +++ b/src/Controller/Backoffice/QuizController.php @@ -30,7 +30,7 @@ use Tvdt\Repository\QuizRepository; use Tvdt\Security\Voter\SeasonVoter; #[AsController] -#[IsGranted('ROLE_USER')] +#[IsGranted('IS_AUTHENTICATED')] class QuizController extends AbstractController { public function __construct( diff --git a/src/Controller/Backoffice/QuizQuestionController.php b/src/Controller/Backoffice/QuizQuestionController.php index 140e174..3f8f0b8 100644 --- a/src/Controller/Backoffice/QuizQuestionController.php +++ b/src/Controller/Backoffice/QuizQuestionController.php @@ -8,9 +8,11 @@ use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Attribute\AsController; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Routing\Requirement\Requirement; +use Symfony\Component\Security\Http\Attribute\IsCsrfTokenValid; use Symfony\Component\Security\Http\Attribute\IsGranted; use Symfony\Contracts\Translation\TranslatorInterface; use Tvdt\Controller\AbstractController; @@ -22,7 +24,7 @@ use Tvdt\Form\QuestionFormType; use Tvdt\Security\Voter\SeasonVoter; #[AsController] -#[IsGranted('ROLE_USER')] +#[IsGranted('IS_AUTHENTICATED')] class QuizQuestionController extends AbstractController { public function __construct( @@ -42,34 +44,106 @@ class QuizQuestionController extends AbstractController throw new NotFoundHttpException(); } - $form = $this->createForm(QuestionFormType::class, $question); + $isTurboFrame = $request->headers->has('Turbo-Frame'); + + $form = $this->createForm(QuestionFormType::class, $question, [ + 'action' => $this->generateUrl('tvdt_backoffice_quiz_question_edit', [ + 'seasonCode' => $season->seasonCode, + 'quiz' => $quiz->id, + 'question' => $question->id, + ]), + ]); $form->handleRequest($request); if ($form->isSubmitted() && $form->isValid()) { - $this->applyAnswerOrdering($question); $this->em->flush(); $this->addFlash(FlashType::Success, $this->translator->trans('Question updated')); + if ($isTurboFrame) { + return new Response(''); + } + return $this->redirectToRoute('tvdt_backoffice_quiz_overview', [ 'seasonCode' => $season->seasonCode, 'quiz' => $quiz->id, ]); } - return $this->render('backoffice/quiz/question_form.html.twig', [ + $template = $isTurboFrame + ? 'backoffice/quiz/_question_frame.html.twig' + : 'backoffice/quiz/question_form.html.twig'; + + $response = $this->render($template, [ 'season' => $season, 'quiz' => $quiz, 'question' => $question, 'form' => $form, ]); + + if ($form->isSubmitted()) { + $response->setStatusCode(Response::HTTP_UNPROCESSABLE_ENTITY); + } + + return $response; } - private function applyAnswerOrdering(Question $question): void + #[IsGranted(SeasonVoter::EDIT, subject: 'season')] + #[Route( + '/backoffice/season/{seasonCode:season}/quiz/{quiz}/question/{question}/view', + name: 'tvdt_backoffice_quiz_question_view', + requirements: ['seasonCode' => self::SEASON_CODE_REGEX, 'quiz' => Requirement::UUID, 'question' => Requirement::UUID], + )] + public function view(Season $season, Quiz $quiz, Question $question): Response { - $ordering = 1; - foreach ($question->answers as $answer) { - $answer->ordering = $ordering++; + if ($question->quiz !== $quiz || $quiz->season !== $season) { + throw new NotFoundHttpException(); } + + return $this->render('backoffice/quiz/_question_detail_frame.html.twig', [ + 'question' => $question, + ]); + } + + #[IsCsrfTokenValid('question_reorder')] + #[IsGranted(SeasonVoter::MODIFY_QUIZ_CONTENT, subject: 'quiz')] + #[Route( + '/backoffice/season/{seasonCode:season}/quiz/{quiz}/questions/reorder', + name: 'tvdt_backoffice_quiz_questions_reorder', + requirements: ['seasonCode' => self::SEASON_CODE_REGEX, 'quiz' => Requirement::UUID], + methods: ['POST'], + )] + public function reorder(Season $season, Quiz $quiz, Request $request): Response + { + if ($quiz->season !== $season) { + throw new NotFoundHttpException(); + } + + /** @var list $ordering */ + $ordering = $request->request->all('ordering'); + + $questionsById = []; + foreach ($quiz->questions as $question) { + $questionsById[$question->id->toString()] = $question; + } + + foreach ($ordering as $questionId) { + if (!isset($questionsById[$questionId])) { + throw new BadRequestHttpException(\sprintf('Unknown question id: %s', $questionId)); + } + } + + if (\count(array_unique($ordering)) !== \count($questionsById)) { + throw new BadRequestHttpException('Ordering must include every question exactly once'); + } + + $position = 1; + foreach ($ordering as $questionId) { + $questionsById[$questionId]->ordering = $position++; + } + + $this->em->flush(); + + return new Response('', Response::HTTP_NO_CONTENT); } } diff --git a/src/Controller/Backoffice/SeasonController.php b/src/Controller/Backoffice/SeasonController.php index 3082050..fd1dbbc 100644 --- a/src/Controller/Backoffice/SeasonController.php +++ b/src/Controller/Backoffice/SeasonController.php @@ -30,7 +30,7 @@ use Tvdt\Security\Voter\SeasonVoter; use Tvdt\Service\QuizSpreadsheetService; #[AsController] -#[IsGranted('ROLE_USER')] +#[IsGranted('IS_AUTHENTICATED')] class SeasonController extends AbstractController { public function __construct( diff --git a/src/Controller/EliminationController.php b/src/Controller/EliminationController.php index bb3f467..3514447 100644 --- a/src/Controller/EliminationController.php +++ b/src/Controller/EliminationController.php @@ -23,7 +23,7 @@ use Tvdt\Security\Voter\SeasonVoter; use function Symfony\Component\Translation\t; #[AsController] -#[IsGranted('ROLE_USER')] +#[IsGranted('IS_AUTHENTICATED')] final class EliminationController extends AbstractController { public function __construct(private readonly TranslatorInterface $translator, private readonly CandidateRepository $candidateRepository) {} diff --git a/src/Entity/Question.php b/src/Entity/Question.php index 8816fed..eeac841 100644 --- a/src/Entity/Question.php +++ b/src/Entity/Question.php @@ -54,6 +54,13 @@ class Question implements \Stringable return $this; } + public function removeAnswer(Answer $answer): static + { + $this->answers->removeElement($answer); + + return $this; + } + public function __toString(): string { return $this->question ?? ''; diff --git a/src/Form/BankQuestionFormType.php b/src/Form/BankQuestionFormType.php index 02d38eb..4fa19f9 100644 --- a/src/Form/BankQuestionFormType.php +++ b/src/Form/BankQuestionFormType.php @@ -43,6 +43,7 @@ class BankQuestionFormType extends AbstractType 'multiple' => true, 'expanded' => true, 'required' => false, + 'choice_attr' => static fn (QuestionLabel $label): array => ['data-colour' => $label->colour->value], 'query_builder' => static fn (QuestionLabelRepository $repository): QueryBuilder => $repository ->createQueryBuilder('l') ->where('l.season = :season') diff --git a/templates/backoffice/partials/answer_row.html.twig b/templates/backoffice/partials/answer_row.html.twig index a3b633e..6cc87b0 100644 --- a/templates/backoffice/partials/answer_row.html.twig +++ b/templates/backoffice/partials/answer_row.html.twig @@ -1,7 +1,10 @@ {% macro answer_row(answerForm) %} -
+
{{ form_widget(answerForm.ordering) }} - +
{{ form_widget(answerForm.text) }}
{{ form_widget(answerForm.isRightAnswer) }}
-
{% endmacro %} diff --git a/templates/backoffice/question_bank/_form_body.html.twig b/templates/backoffice/question_bank/_form_body.html.twig new file mode 100644 index 0000000..fcd1674 --- /dev/null +++ b/templates/backoffice/question_bank/_form_body.html.twig @@ -0,0 +1,54 @@ +{% import 'backoffice/partials/answer_row.html.twig' as macros %} + +{{ form_start(form, {attr: {novalidate: 'novalidate'}}) }} +{{ form_row(form.question) }} +{{ form_row(form.reusable) }} +
+ {{ form_label(form.labels) }} + {{ form_errors(form.labels) }} + {% for labelChoice in form.labels %} +
+ + +
+ {% endfor %} + {% do form.labels.setRendered %} +
+ +
+ {{ form_label(form.answers) }} + {{ form_errors(form.answers) }} +
+ {% for answerForm in form.answers %} + {{ macros.answer_row(answerForm) }} + {% endfor %} +
+ {% do form.answers.setRendered %} +
+ + + +
+
+ +{% if isModal ?? false %} + {% do form.save.setRendered %} +{% endif %} +{{ form_end(form) }} +{% if isModal ?? false %} + +{% endif %} diff --git a/templates/backoffice/question_bank/_frame.html.twig b/templates/backoffice/question_bank/_frame.html.twig new file mode 100644 index 0000000..7aa5de0 --- /dev/null +++ b/templates/backoffice/question_bank/_frame.html.twig @@ -0,0 +1,51 @@ +{% import 'backoffice/partials/answer_row.html.twig' as macros %} + + + {{ form_start(form, {attr: {novalidate: 'novalidate'}}) }} +
+ {{ form_row(form.question) }} + {{ form_row(form.reusable) }} +
+ {{ form_label(form.labels) }} + {{ form_errors(form.labels) }} + {% for labelChoice in form.labels %} +
+ + +
+ {% endfor %} + {% do form.labels.setRendered %} +
+
+ {{ form_label(form.answers) }} + {{ form_errors(form.answers) }} +
+ {% for answerForm in form.answers %} + {{ macros.answer_row(answerForm) }} + {% endfor %} +
+ {% do form.answers.setRendered %} +
+ + + +
+
+
+
+ + {{ form_widget(form.save, {attr: {class: 'btn btn-primary'}}) }} +
+ {{ form_end(form) }} + diff --git a/templates/backoffice/question_bank/form.html.twig b/templates/backoffice/question_bank/form.html.twig index 2ace58f..3f6c67b 100644 --- a/templates/backoffice/question_bank/form.html.twig +++ b/templates/backoffice/question_bank/form.html.twig @@ -18,33 +18,7 @@

{{ bankQuestion is null ? 'Add question'|trans : 'Edit question'|trans }}

- - {{ form_start(form) }} - {{ form_row(form.question) }} - {{ form_row(form.reusable) }} - {{ form_row(form.labels) }} - -
- {{ form_label(form.answers) }} - {{ form_errors(form.answers) }} -
- {% for answerForm in form.answers %} - {{ macros.answer_row(answerForm) }} - {% endfor %} -
- {% do form.answers.setRendered %} -
- - - -
-
- - {{ form_end(form) }} + {{ include('backoffice/question_bank/_form_body.html.twig') }}
{{ include('backoffice/help/quiz_question_bank_form.html.twig') }} diff --git a/templates/backoffice/quiz/_question_detail_body.html.twig b/templates/backoffice/quiz/_question_detail_body.html.twig new file mode 100644 index 0000000..3c15eb2 --- /dev/null +++ b/templates/backoffice/quiz/_question_detail_body.html.twig @@ -0,0 +1,21 @@ +

{{ question.question }}

+ +
    + {% for answer in question.answers %} +
  • + + {% if answer.isRightAnswer %}{% endif %} + {{ answer.text }} + + {% if answer.candidates|length > 0 %} + {{ answer.candidates|map(c => c.name)|join(', ') }} + {% endif %} +
    • + {% else %} +
  • {{ 'There are no answers for this question'|trans }}
    • + {% endfor %} +
+ + diff --git a/templates/backoffice/quiz/_question_detail_frame.html.twig b/templates/backoffice/quiz/_question_detail_frame.html.twig new file mode 100644 index 0000000..d941e10 --- /dev/null +++ b/templates/backoffice/quiz/_question_detail_frame.html.twig @@ -0,0 +1,23 @@ + +
+

{{ question.question }}

+
    + {% for answer in question.answers %} +
  • + + {% if answer.isRightAnswer %}{% endif %} + {{ answer.text }} + + {% if answer.candidates|length > 0 %} + {{ answer.candidates|map(c => c.name)|join(', ') }} + {% endif %} +
    • + {% else %} +
  • {{ 'There are no answers for this question'|trans }}
    • + {% endfor %} +
+
+
+ +
+ diff --git a/templates/backoffice/quiz/_question_form_body.html.twig b/templates/backoffice/quiz/_question_form_body.html.twig new file mode 100644 index 0000000..b373466 --- /dev/null +++ b/templates/backoffice/quiz/_question_form_body.html.twig @@ -0,0 +1,36 @@ +{% import 'backoffice/partials/answer_row.html.twig' as macros %} + +{{ form_start(form, {attr: {novalidate: 'novalidate'}}) }} +{{ form_row(form.question) }} + +
+ {{ form_label(form.answers) }} + {{ form_errors(form.answers) }} +
+ {% for answerForm in form.answers %} + {{ macros.answer_row(answerForm) }} + {% endfor %} +
+ {% do form.answers.setRendered %} +
+ + + +
+
+ +{% if isModal ?? false %} + {% do form.save.setRendered %} +{% endif %} +{{ form_end(form) }} +{% if isModal ?? false %} + +{% endif %} diff --git a/templates/backoffice/quiz/_question_frame.html.twig b/templates/backoffice/quiz/_question_frame.html.twig new file mode 100644 index 0000000..4af4a7e --- /dev/null +++ b/templates/backoffice/quiz/_question_frame.html.twig @@ -0,0 +1,33 @@ +{% import 'backoffice/partials/answer_row.html.twig' as macros %} + + + {{ form_start(form, {attr: {novalidate: 'novalidate'}}) }} +
+ {{ form_row(form.question) }} +
+ {{ form_label(form.answers) }} + {{ form_errors(form.answers) }} +
+ {% for answerForm in form.answers %} + {{ macros.answer_row(answerForm) }} + {% endfor %} +
+ {% do form.answers.setRendered %} +
+ + + +
+
+
+
+ + {{ form_widget(form.save, {attr: {class: 'btn btn-primary'}}) }} +
+ {{ form_end(form) }} + diff --git a/templates/backoffice/quiz/question_form.html.twig b/templates/backoffice/quiz/question_form.html.twig index 81ddfac..af98253 100644 --- a/templates/backoffice/quiz/question_form.html.twig +++ b/templates/backoffice/quiz/question_form.html.twig @@ -18,31 +18,7 @@

{{ 'Edit question'|trans }}

- - {{ form_start(form) }} - {{ form_row(form.question) }} - -
- {{ form_label(form.answers) }} - {{ form_errors(form.answers) }} -
- {% for answerForm in form.answers %} - {{ macros.answer_row(answerForm) }} - {% endfor %} -
- {% do form.answers.setRendered %} -
- - - -
-
- - {{ form_end(form) }} + {{ include('backoffice/quiz/_question_form_body.html.twig') }}
{{ include('backoffice/help/quiz_question_bank_form.html.twig') }} diff --git a/templates/backoffice/quiz/tab_overview.html.twig b/templates/backoffice/quiz/tab_overview.html.twig index 99d1423..4210b2a 100644 --- a/templates/backoffice/quiz/tab_overview.html.twig +++ b/templates/backoffice/quiz/tab_overview.html.twig @@ -73,10 +73,10 @@ {% endif %} - -
-

{{ 'Questions'|trans }}

-
- {%~ for question in quiz.questions ~%} -
-

- -

-
-
- {% if is_granted('QUIZ_MODIFY_CONTENT', question) %} - - {{ 'Edit'|trans }} - - {% endif %} -
    - {%~ for answer in question.answers %} - - {{ answer.text }} - {% if answer.candidates|length > 0 %} - - ({{ answer.candidates|map(c => c.name)|join(', ') }}) - - {% endif %} - - {%~ else %} - {{ 'There are no answers for this question'|trans -}} - {%~ endfor %} -
+
+ +

+ {{ 'Questions'|trans }} + +

+ +
+ {%~ for question in quiz.questions ~%} +
+
+
+ {% if is_granted('QUIZ_MODIFY_CONTENT', question) %} + + + + {% endif %} +
+ {% set questionError = questionErrors[question.id.toString] ?? null %} + ! + {{ loop.index }}. {{ question.question }} +
+ {% if is_granted('QUIZ_MODIFY_CONTENT', question) %} + + {% elseif quiz.isLocked %} + + {% endif %} +
+ {% else %} + {{ 'No questions have been added to this quiz yet.'|trans }} + {% endfor %} +
+ +
{{ _self.confirm_modal( diff --git a/templates/backoffice/season/tab_question_bank.html.twig b/templates/backoffice/season/tab_question_bank.html.twig index 75f9467..093da75 100644 --- a/templates/backoffice/season/tab_question_bank.html.twig +++ b/templates/backoffice/season/tab_question_bank.html.twig @@ -1,7 +1,13 @@
-
+
- {{ 'Add question'|trans }} +
@@ -120,9 +126,11 @@ {% endif %}
- + @@ -163,6 +171,22 @@ {% endfor %} +
{{ include('backoffice/help/season_question_bank.html.twig') }} diff --git a/templates/backoffice/season/tab_tests.html.twig b/templates/backoffice/season/tab_tests.html.twig index 49d650b..5f070d4 100644 --- a/templates/backoffice/season/tab_tests.html.twig +++ b/templates/backoffice/season/tab_tests.html.twig @@ -2,9 +2,9 @@
{{ 'Import Quiz from Excel'|trans }} - {{ 'Add Empty Quiz'|trans }} + {{ 'Import Quiz from Excel'|trans }}
{% for quiz in season.quizzes %} diff --git a/tests/Controller/Backoffice/QuestionBankControllerTest.php b/tests/Controller/Backoffice/QuestionBankControllerTest.php index dfd3bfa..56dce89 100644 --- a/tests/Controller/Backoffice/QuestionBankControllerTest.php +++ b/tests/Controller/Backoffice/QuestionBankControllerTest.php @@ -10,6 +10,7 @@ use Symfony\Bundle\FrameworkBundle\KernelBrowser; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; use Symfony\Component\HttpFoundation\Request; use Tvdt\Controller\Backoffice\QuestionBankController; +use Tvdt\Entity\BankAnswer; use Tvdt\Entity\BankQuestion; use Tvdt\Entity\Question; use Tvdt\Entity\QuestionLabel; @@ -298,6 +299,79 @@ final class QuestionBankControllerTest extends WebTestCase $this->assertResponseStatusCodeSame(403); } + public function testCreateBankQuestionPreservesAnswerOrdering(): void + { + $this->loginAsOwner(); + $crawler = $this->client->request(Request::METHOD_GET, '/backoffice/season/krtek/question-bank/new'); + $this->assertResponseIsSuccessful(); + $token = (string) $crawler->filter('input[name="bank_question_form[_token]"]')->attr('value'); + + // Submit 3 answers with non-sequential ordering values. + // The stored ordering field (not the submission index) must dictate retrieval order. + $this->client->request(Request::METHOD_POST, '/backoffice/season/krtek/question-bank/new', [ + 'bank_question_form' => [ + 'question' => 'Volgorderingstest nieuwe vraag', + 'answers' => [ + 0 => ['text' => 'Antwoord C', 'isRightAnswer' => '1', 'ordering' => '5'], + 1 => ['text' => 'Antwoord A', 'ordering' => '1'], + 2 => ['text' => 'Antwoord B', 'ordering' => '3'], + ], + '_token' => $token, + ], + ]); + + $this->assertResponseRedirects('/backoffice/season/krtek/question-bank'); + + $this->entityManager->clear(); + $bankQuestion = $this->getBankQuestion('Volgorderingstest nieuwe vraag'); + $answers = $bankQuestion->answers->toArray(); + $this->assertCount(3, $answers); + // @OrderBy(['ordering' => 'ASC']): ordering 1 → 3 → 5 + $this->assertSame('Antwoord A', $answers[0]->text); + $this->assertSame('Antwoord B', $answers[1]->text); + $this->assertSame('Antwoord C', $answers[2]->text); + } + + public function testEditBankQuestionPreservesAnswerOrdering(): void + { + $this->loginAsOwner(); + $bankQuestion = $this->getBankQuestion('Wat at de Krtek als ontbijt?'); + // Fixture answers in insertion order (all have ordering=0): Brood (correct), Yoghurt, Niks + + $url = \sprintf('/backoffice/season/krtek/question-bank/%s/edit', $bankQuestion->id); + $crawler = $this->client->request(Request::METHOD_GET, $url); + $this->assertResponseIsSuccessful(); + $token = (string) $crawler->filter('input[name="bank_question_form[_token]"]')->attr('value'); + + $answers = $bankQuestion->answers->toArray(); + $this->assertCount(3, $answers); + $texts = array_map(static fn (BankAnswer $a): string => $a->text, $answers); + + // Assign ordering values: first answer gets 4, second gets 0, third gets 2. + // Expected retrieval order after @OrderBy ASC: index 1 (0) → index 2 (2) → index 0 (4). + $this->client->request(Request::METHOD_POST, $url, [ + 'bank_question_form' => [ + 'question' => $bankQuestion->question, + 'answers' => [ + 0 => ['text' => $texts[0], 'isRightAnswer' => '1', 'ordering' => '4'], + 1 => ['text' => $texts[1], 'ordering' => '0'], + 2 => ['text' => $texts[2], 'ordering' => '2'], + ], + '_token' => $token, + ], + ]); + + $this->assertResponseRedirects('/backoffice/season/krtek/question-bank'); + + $this->entityManager->clear(); + $bankQuestion = $this->getBankQuestion('Wat at de Krtek als ontbijt?'); + $reloadedAnswers = $bankQuestion->answers->toArray(); + $this->assertCount(3, $reloadedAnswers); + $this->assertSame($texts[1], $reloadedAnswers[0]->text); // ordering=0 → first + $this->assertSame($texts[2], $reloadedAnswers[1]->text); // ordering=2 → second + $this->assertSame($texts[0], $reloadedAnswers[2]->text); // ordering=4 → third + } + public function testAddAndDeleteLabel(): void { $this->loginAsOwner(); diff --git a/tests/Controller/Backoffice/QuizQuestionControllerTest.php b/tests/Controller/Backoffice/QuizQuestionControllerTest.php new file mode 100644 index 0000000..4cb7f03 --- /dev/null +++ b/tests/Controller/Backoffice/QuizQuestionControllerTest.php @@ -0,0 +1,147 @@ +client = self::createClient(); + $this->entityManager = self::getContainer()->get(EntityManagerInterface::class); + } + + private function loginAsOwner(): void + { + $user = $this->entityManager->getRepository(User::class)->findOneBy(['email' => 'krtek-admin@example.org']); + $this->assertInstanceOf(User::class, $user); + $this->client->loginUser($user); + } + + private function getQuizByName(string $name): Quiz + { + $quiz = $this->entityManager->getRepository(Quiz::class)->findOneBy(['name' => $name]); + $this->assertInstanceOf(Quiz::class, $quiz); + + return $quiz; + } + + public function testEditPreservesAnswerOrdering(): void + { + $this->loginAsOwner(); + + $quiz = $this->getQuizByName('Quiz 2'); + $question = null; + foreach ($quiz->questions as $q) { + if ('Is de Krtek een man of een vrouw?' === $q->question) { + $question = $q; + break; + } + } + + $this->assertInstanceOf(Question::class, $question); + + $answers = $question->answers->toArray(); + $this->assertCount(2, $answers); + $firstText = $answers[0]->text; + $secondText = $answers[1]->text; + + $url = \sprintf( + '/backoffice/season/krtek/quiz/%s/question/%s/edit', + $quiz->id, + $question->id, + ); + + $crawler = $this->client->request(Request::METHOD_GET, $url); + $this->assertResponseIsSuccessful(); + $token = (string) $crawler->filter('input[name="question_form[_token]"]')->attr('value'); + + // Submit with ordering values that invert which answer appears first on reload. + // The answer currently at index 0 ($firstText) gets ordering=7, + // the one at index 1 ($secondText) gets ordering=3. + // @OrderBy(['ordering' => 'ASC']) on Question::$answers will return + // $secondText (3) before $firstText (7) after flush+clear. + $this->client->request(Request::METHOD_POST, $url, [ + 'question_form' => [ + 'question' => $question->question, + 'answers' => [ + 0 => ['text' => $firstText, 'ordering' => '7'], + 1 => ['text' => $secondText, 'ordering' => '3'], + ], + '_token' => $token, + ], + ]); + + $this->assertResponseRedirects(); + + $this->entityManager->clear(); + $quiz = $this->getQuizByName('Quiz 2'); + $reloadedQuestion = null; + foreach ($quiz->questions as $q) { + if ('Is de Krtek een man of een vrouw?' === $q->question) { + $reloadedQuestion = $q; + break; + } + } + + $this->assertInstanceOf(Question::class, $reloadedQuestion); + + $reloadedAnswers = $reloadedQuestion->answers->toArray(); + $this->assertSame(3, $reloadedAnswers[0]->ordering); + $this->assertSame($secondText, $reloadedAnswers[0]->text); + $this->assertSame(7, $reloadedAnswers[1]->ordering); + $this->assertSame($firstText, $reloadedAnswers[1]->text); + } + + public function testReorderQuestionsWithinQuiz(): void + { + $this->loginAsOwner(); + + $quiz = $this->getQuizByName('Quiz 2'); + $originalQuestions = $quiz->questions->toArray(); + $this->assertGreaterThanOrEqual(3, \count($originalQuestions)); + + $originalFirstId = (string) $originalQuestions[0]->id; + $originalLastId = (string) $originalQuestions[\count($originalQuestions) - 1]->id; + + $overviewUrl = \sprintf('/backoffice/season/krtek/quiz/%s/overview', $quiz->id); + $crawler = $this->client->request(Request::METHOD_GET, $overviewUrl); + $this->assertResponseIsSuccessful(); + + $csrfToken = $crawler->filter('[data-bo--question-list-csrf-value]')->attr('data-bo--question-list-csrf-value'); + $this->assertNotEmpty($csrfToken); + + $reversedIds = array_reverse(array_map(static fn (Question $q): string => (string) $q->id, $originalQuestions)); + + $reorderUrl = \sprintf('/backoffice/season/krtek/quiz/%s/questions/reorder', $quiz->id); + $this->client->request(Request::METHOD_POST, $reorderUrl, [ + '_token' => $csrfToken, + 'ordering' => $reversedIds, + ]); + + $this->assertResponseStatusCodeSame(204); + + $this->entityManager->clear(); + $quiz = $this->getQuizByName('Quiz 2'); + $reorderedQuestions = $quiz->questions->toArray(); + + $this->assertSame($originalLastId, (string) $reorderedQuestions[0]->id); + $this->assertSame($originalFirstId, (string) $reorderedQuestions[\count($reorderedQuestions) - 1]->id); + } +} diff --git a/translations/messages+intl-icu.nl.xliff b/translations/messages+intl-icu.nl.xliff index c860042..e786c53 100644 --- a/translations/messages+intl-icu.nl.xliff +++ b/translations/messages+intl-icu.nl.xliff @@ -257,6 +257,10 @@ Error clearing quiz Fout bij het leegmaken van de test + + Error saving order + Fout bij het opslaan van de volgorde + Export to XLSX Exporteren naar XLSX @@ -385,6 +389,10 @@ No candidates Geen kandidaten + + No questions have been added to this quiz yet. + Er zijn nog geen vragen aan deze test toegevoegd. + No questions in the question bank yet Nog geen vragen in de vragenbank @@ -409,13 +417,17 @@ Open Openen + + Order saved + Volgorde opgeslagen + Overview Overzicht Owner(s) - Eigenaar(s) + Eigenaar/Eigenaren Password @@ -473,6 +485,10 @@ Question bank Vragenbank + + Question details + Vraagdetails + Question removed from quiz %quiz% Vraag verwijderd uit quiz %quiz% @@ -557,6 +573,10 @@ Red Rood + + Refresh the page to try again. + Ververs de pagina om het opnieuw te proberen. + Register Registreren @@ -703,7 +723,7 @@ Toggle correct answer - + Goed antwoord aan/uitzetten Unassign @@ -717,6 +737,10 @@ Used in Gebruikt in + + View + Bekijken + White Wit