From 79236d84e95d3f46950131102091b18be0ac6964 Mon Sep 17 00:00:00 2001 From: Marijn Doeve Date: Sat, 7 Jun 2025 16:05:15 +0200 Subject: [PATCH] Add correction management to backoffice, refactor security voter logic, and enhance candidate scoring This commit introduces functionality to manage candidate corrections in the backoffice, with updated templates and a new route handler. The SeasonVoter is refactored to support additional entities, and scoring logic is updated to incorporate corrections consistently. Includes test coverage for voter logic and UI improvements for score tables. --- .github/workflows/ci.yml | 1 - phpstan.dist.neon | 1 + src/Controller/Backoffice/QuizController.php | 33 +++++++- src/Controller/QuizController.php | 4 +- src/Repository/CandidateRepository.php | 28 +++---- src/Repository/QuizCandidateRepository.php | 11 +++ src/Security/Voter/SeasonVoter.php | 34 +++++++- templates/backoffice/quiz.html.twig | 24 ++++-- .../quiz/elimination/candidate.html.twig | 2 +- tests/Security/Voter/SeasonVoterTest.php | 77 +++++++++++++++++++ 10 files changed, 182 insertions(+), 33 deletions(-) create mode 100644 tests/Security/Voter/SeasonVoterTest.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5eccf8b..8516037 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,6 @@ jobs: - name: Run migrations run: docker compose exec -T php bin/console -e test doctrine:migrations:migrate --no-interaction - name: Run PHPUnit - if: false # Remove this line when the tests are ready run: docker compose exec -T php vendor/bin/phpunit - name: Doctrine Schema Validator run: docker compose exec -T php bin/console -e test doctrine:schema:validate diff --git a/phpstan.dist.neon b/phpstan.dist.neon index 524626a..87d6918 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -7,3 +7,4 @@ parameters: - public/ - src/ - tests/ + treatPhpDocTypesAsCertain: false diff --git a/src/Controller/Backoffice/QuizController.php b/src/Controller/Backoffice/QuizController.php index a08ce25..d9db54f 100644 --- a/src/Controller/Backoffice/QuizController.php +++ b/src/Controller/Backoffice/QuizController.php @@ -5,13 +5,18 @@ declare(strict_types=1); namespace App\Controller\Backoffice; use App\Controller\AbstractController; +use App\Entity\Candidate; use App\Entity\Quiz; use App\Entity\Season; use App\Repository\CandidateRepository; +use App\Repository\QuizCandidateRepository; use App\Security\Voter\SeasonVoter; use Doctrine\ORM\EntityManagerInterface; +use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Attribute\AsController; +use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException; use Symfony\Component\Routing\Attribute\Route; use Symfony\Component\Security\Http\Attribute\IsGranted; @@ -23,7 +28,9 @@ class QuizController extends AbstractController private readonly CandidateRepository $candidateRepository, ) {} - #[Route('/backoffice/season/{seasonCode}/quiz/{quiz}', name: 'app_backoffice_quiz', + #[Route( + '/backoffice/season/{seasonCode}/quiz/{quiz}', + name: 'app_backoffice_quiz', requirements: ['seasonCode' => self::SEASON_CODE_REGEX], )] #[IsGranted(SeasonVoter::EDIT, subject: 'season')] @@ -36,11 +43,13 @@ class QuizController extends AbstractController ]); } - #[Route('/backoffice/season/{seasonCode}/quiz/{quiz}/enable', name: 'app_backoffice_enable', + #[Route( + '/backoffice/season/{seasonCode}/quiz/{quiz}/enable', + name: 'app_backoffice_enable', requirements: ['seasonCode' => self::SEASON_CODE_REGEX], )] #[IsGranted(SeasonVoter::EDIT, subject: 'season')] - public function enableQuiz(Season $season, ?Quiz $quiz, EntityManagerInterface $em): Response + public function enableQuiz(Season $season, ?Quiz $quiz, EntityManagerInterface $em): RedirectResponse { $season->setActiveQuiz($quiz); $em->flush(); @@ -51,4 +60,22 @@ class QuizController extends AbstractController return $this->redirectToRoute('app_backoffice_season', ['seasonCode' => $season->getSeasonCode()]); } + + #[Route( + '/backoffice/quiz/{quiz}/modify_correction/{candidate}', + name: 'app_backoffice_modify_correction', + )] + #[IsGranted(SeasonVoter::EDIT, subject: 'quiz')] + public function modifyCorrection(Quiz $quiz, Candidate $candidate, QuizCandidateRepository $quizCandidateRepository, Request $request): RedirectResponse + { + if (!$request->isMethod('POST')) { + throw new MethodNotAllowedHttpException(['POST']); + } + + $corrections = (float) $request->request->get('corrections'); + + $quizCandidateRepository->setCorrectionsForCandidate($quiz, $candidate, $corrections); + + return $this->redirectToRoute('app_backoffice_quiz', ['seasonCode' => $quiz->getSeason()->getSeasonCode(), 'quiz' => $quiz->getId()]); + } } diff --git a/src/Controller/QuizController.php b/src/Controller/QuizController.php index cd7460b..06d639c 100644 --- a/src/Controller/QuizController.php +++ b/src/Controller/QuizController.php @@ -21,10 +21,10 @@ use App\Repository\QuestionRepository; use App\Repository\QuizCandidateRepository; use App\Repository\SeasonRepository; use Symfony\Bridge\Doctrine\Attribute\MapEntity; -use Symfony\Component\HttpFoundation\Exception\BadRequestException; 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\Routing\Attribute\Route; use Symfony\Contracts\Translation\TranslatorInterface; @@ -109,7 +109,7 @@ final class QuizController extends AbstractController $answer = $answerRepository->findOneBy(['id' => $request->request->get('answer')]); if (!$answer instanceof Answer) { - throw new BadRequestException('Invalid Answer ID'); + throw new BadRequestHttpException('Invalid Answer ID'); } $givenAnswer = new GivenAnswer($candidate, $answer->getQuestion()->getQuiz(), $answer); diff --git a/src/Repository/CandidateRepository.php b/src/Repository/CandidateRepository.php index 3cdbaa4..01029a9 100644 --- a/src/Repository/CandidateRepository.php +++ b/src/Repository/CandidateRepository.php @@ -16,7 +16,7 @@ use Symfony\Component\Uid\Uuid; /** * @extends ServiceEntityRepository * - * @phpstan-type Result array{id: Uuid, name: string, correct: int, time: \DateInterval, corrections?: float, score: float} + * @phpstan-type Result array{id: Uuid, name: string, correct: int, time: \DateInterval, corrections: float, score: float} * @phpstan-type ResultList list */ class CandidateRepository extends ServiceEntityRepository @@ -54,40 +54,32 @@ class CandidateRepository extends ServiceEntityRepository /** @return ResultList */ public function getScores(Quiz $quiz): array { - $scoreQb = $this->createQueryBuilder('c', 'c.id') - ->select('c.id', 'c.name', 'sum(case when a.isRightAnswer = true then 1 else 0 end) as correct') + $qb = $this->createQueryBuilder('c', 'c.id') + ->select('c.id', 'c.name', 'sum(case when a.isRightAnswer = true then 1 else 0 end) as correct', 'qc.corrections', 'max(ga.created) - qc.created as time') ->join('c.givenAnswers', 'ga') ->join('ga.answer', 'a') - ->where('ga.quiz = :quiz') - ->groupBy('c.id') - ->setParameter('quiz', $quiz); - - $startTimeCorrectionQb = $this->createQueryBuilder('c', 'c.id') - ->select('c.id', 'qc.corrections', 'max(ga.created) - qc.created as time') ->join('c.quizData', 'qc') - ->join('c.givenAnswers', 'ga') ->where('qc.quiz = :quiz') ->groupBy('ga.quiz', 'c.id', 'qc.id') ->setParameter('quiz', $quiz); - $merged = array_merge_recursive( - $scoreQb->getQuery()->getArrayResult(), - $startTimeCorrectionQb->getQuery()->getArrayResult(), + return $this->sortResults( + $this->calculateScore( + $qb->getQuery()->getResult(), + ), ); - - return $this->sortResults($this->calculateScore($merged)); } /** - * @param array $in + * @param array $in * * @return array - * */ + */ private function calculateScore(array $in): array { return array_map(static fn ($candidate): array => [ ...$candidate, - 'score' => $candidate['correct'] + ($candidate['corrections'] ?? 0.0), + 'score' => $candidate['correct'] + $candidate['corrections'], ], $in); } diff --git a/src/Repository/QuizCandidateRepository.php b/src/Repository/QuizCandidateRepository.php index 42e217b..109f165 100644 --- a/src/Repository/QuizCandidateRepository.php +++ b/src/Repository/QuizCandidateRepository.php @@ -33,4 +33,15 @@ class QuizCandidateRepository extends ServiceEntityRepository return true; } + + public function setCorrectionsForCandidate(Quiz $quiz, Candidate $candidate, float $corrections): void + { + $quizCandidate = $this->findOneBy(['candidate' => $candidate, 'quiz' => $quiz]); + if (!$quizCandidate instanceof QuizCandidate) { + throw new \InvalidArgumentException('Quiz candidate not found'); + } + + $quizCandidate->setCorrections($corrections); + $this->getEntityManager()->flush(); + } } diff --git a/src/Security/Voter/SeasonVoter.php b/src/Security/Voter/SeasonVoter.php index 58a9cc2..a6d5bea 100644 --- a/src/Security/Voter/SeasonVoter.php +++ b/src/Security/Voter/SeasonVoter.php @@ -4,7 +4,11 @@ declare(strict_types=1); namespace App\Security\Voter; +use App\Entity\Answer; +use App\Entity\Candidate; use App\Entity\Elimination; +use App\Entity\Question; +use App\Entity\Quiz; use App\Entity\Season; use App\Entity\User; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -22,10 +26,17 @@ final class SeasonVoter extends Voter protected function supports(string $attribute, mixed $subject): bool { return \in_array($attribute, [self::EDIT, self::DELETE, self::ELIMINATION], true) - && ($subject instanceof Season || $subject instanceof Elimination); + && ( + $subject instanceof Season + || $subject instanceof Elimination + || $subject instanceof Quiz + || $subject instanceof Candidate + || $subject instanceof Answer + || $subject instanceof Question + ); } - /** @param Season|Elimination $subject */ + /** @param Season|Elimination|Quiz|Candidate|Answer|Question $subject */ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool { $user = $token->getUser(); @@ -37,7 +48,24 @@ final class SeasonVoter extends Voter return true; } - $season = $subject instanceof Season ? $subject : $subject->getQuiz()->getSeason(); + switch (true) { + case $subject instanceof Answer: + $season = $subject->getQuestion()->getQuiz()->getSeason(); + break; + case $subject instanceof Elimination: + case $subject instanceof Question: + $season = $subject->getQuiz()->getSeason(); + break; + case $subject instanceof Candidate: + case $subject instanceof Quiz: + $season = $subject->getSeason(); + break; + case $subject instanceof Season: + $season = $subject; + break; + default: + return false; + } return match ($attribute) { self::EDIT, self::DELETE, self::ELIMINATION => $season->isOwner($user), diff --git a/templates/backoffice/quiz.html.twig b/templates/backoffice/quiz.html.twig index 51acce3..34b8629 100644 --- a/templates/backoffice/quiz.html.twig +++ b/templates/backoffice/quiz.html.twig @@ -74,10 +74,10 @@ {{ 'Candidate'|trans }} - {{ 'Correct Answers'|trans }} - {{ 'Corrections'|trans }} - {{ 'Score'|trans }} - {{ 'Time'|trans }} + {{ 'Correct Answers'|trans }} + {{ 'Corrections'|trans }} + {{ 'Score'|trans }} + {{ 'Time'|trans }} @@ -85,7 +85,21 @@ {{ candidate.name }} {{ candidate.correct|default('0') }} - {{ candidate.corrections|default('0') }} + +
+
+
+ +
+
+ +
+
+
+ {{ candidate.score|default('x') }} {{ candidate.time }} diff --git a/templates/quiz/elimination/candidate.html.twig b/templates/quiz/elimination/candidate.html.twig index bb09028..35a7851 100644 --- a/templates/quiz/elimination/candidate.html.twig +++ b/templates/quiz/elimination/candidate.html.twig @@ -5,7 +5,7 @@ class="elimination-screen" id="{{ colour }}" alt="Screen with colour {{ colour }}" data-controller="elimination" - data-action="click->elimination#next" + data-action="click->elimination#next keydown@document->elimination#next" tabindex="0" > diff --git a/tests/Security/Voter/SeasonVoterTest.php b/tests/Security/Voter/SeasonVoterTest.php new file mode 100644 index 0000000..31afe01 --- /dev/null +++ b/tests/Security/Voter/SeasonVoterTest.php @@ -0,0 +1,77 @@ +seasonVoter = new SeasonVoter(); + $this->token = $this->createStub(TokenInterface::class); + + $this->user = $this->createStub(User::class); + $this->token->method('getUser')->willReturn($this->user); + } + + #[DataProvider('typesProvider')] + public function testWithTypes(mixed $subject): void + { + $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->seasonVoter->vote($this->token, $subject, ['SEASON_EDIT'])); + } + + public function testNotOwnerWillReturnDenied(): void + { + $season = self::createStub(Season::class); + $season->method('isOwner')->willReturn(false); + + $this->assertSame(VoterInterface::ACCESS_DENIED, $this->seasonVoter->vote($this->token, $season, ['SEASON_EDIT'])); + } + + public static function typesProvider(): \Generator + { + $season = self::createStub(Season::class); + $season->method('isOwner')->willReturn(true); + + $quiz = self::createStub(Quiz::class); + $quiz->method('getSeason')->willReturn($season); + + $elimination = self::createStub(Elimination::class); + $elimination->method('getQuiz')->willReturn($quiz); + + $candidate = self::createStub(Candidate::class); + $candidate->method('getSeason')->willReturn($season); + + $question = self::createStub(Question::class); + $question->method('getQuiz')->willReturn($quiz); + + $answer = self::createStub(Answer::class); + $answer->method('getQuestion')->willReturn($question); + + yield 'Season' => [$season]; + yield 'Elimination' => [$elimination]; + yield 'Quiz' => [$quiz]; + yield 'Candidate' => [$candidate]; + yield 'Question' => [$question]; + yield 'Answer' => [$answer]; + } +}