From 1a3640b15c90e5df14f561c452fe3fc791c4ad3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Fromme?= Date: Mon, 14 Sep 2026 09:40:17 +0200 Subject: [PATCH] feat: add admin views, a booking horizon and confirmation modals Co-Authored-By: Claude Opus 5 --- assets/icons/lucide/shield-check.svg | 1 + assets/icons/lucide/triangle-alert.svg | 1 + config/packages/security.yaml | 3 + src/Booking/Horizon.php | 31 ++ .../Admin/BookingAdminController.php | 88 ++++++ src/Controller/BookingController.php | 133 +++++++- src/Entity/BookingSeries.php | 3 - src/Form/Admin/BookingFilter.php | 46 +++ src/Form/Admin/BookingFilterType.php | 74 +++++ src/Form/BookingRequest.php | 34 ++- src/Form/BookingType.php | 11 + src/Repository/BookingRepository.php | 36 +++ src/Repository/BookingSeriesRepository.php | 36 ++- templates/admin/_modal_spot.html.twig | 52 ++++ templates/admin/bookings.html.twig | 104 +++++++ templates/board/_calendar.html.twig | 11 +- templates/board/index.html.twig | 11 + templates/board/plan/_spot.html.twig | 18 +- templates/booking/_modal_edit.html.twig | 16 +- templates/booking/_modal_end_series.html.twig | 17 ++ templates/booking/_modal_release.html.twig | 22 ++ templates/booking/index.html.twig | 28 +- templates/dashboard/index.html.twig | 17 ++ templates/htmx_confirmation_modal.html.twig | 66 ++++ tests/Booking/HorizonTest.php | 50 +++ .../Admin/BookingAdminControllerTest.php | 287 ++++++++++++++++++ tests/Controller/BookingControllerTest.php | 135 +++++++- 27 files changed, 1269 insertions(+), 62 deletions(-) create mode 100644 assets/icons/lucide/shield-check.svg create mode 100644 assets/icons/lucide/triangle-alert.svg create mode 100644 src/Booking/Horizon.php create mode 100644 src/Controller/Admin/BookingAdminController.php create mode 100644 src/Form/Admin/BookingFilter.php create mode 100644 src/Form/Admin/BookingFilterType.php create mode 100644 templates/admin/_modal_spot.html.twig create mode 100644 templates/admin/bookings.html.twig create mode 100644 templates/booking/_modal_end_series.html.twig create mode 100644 templates/booking/_modal_release.html.twig create mode 100644 templates/htmx_confirmation_modal.html.twig create mode 100644 tests/Booking/HorizonTest.php create mode 100644 tests/Controller/Admin/BookingAdminControllerTest.php diff --git a/assets/icons/lucide/shield-check.svg b/assets/icons/lucide/shield-check.svg new file mode 100644 index 0000000..93efae2 --- /dev/null +++ b/assets/icons/lucide/shield-check.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/assets/icons/lucide/triangle-alert.svg b/assets/icons/lucide/triangle-alert.svg new file mode 100644 index 0000000..ae83c2e --- /dev/null +++ b/assets/icons/lucide/triangle-alert.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/config/packages/security.yaml b/config/packages/security.yaml index f787a6b..80203e5 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -27,6 +27,9 @@ security: - { path: ^/(login|auth|logout)$, roles: PUBLIC_ACCESS } - { path: ^/auth/, roles: PUBLIC_ACCESS } - { path: ^/dev/, roles: PUBLIC_ACCESS } + # Before the catch-all, or ROLE_EMPLOYEE would match ^/ first and the admin area would be + # open to everybody. Order is the whole guard here. + - { path: ^/admin, roles: ROLE_ADMIN } # Either role opens the app -- access_control treats a list of roles as an OR. Not # ROLE_USER: myep authenticates far more people than work here, and someone who holds # neither of these is refused at login with an explanation rather than let in to an diff --git a/src/Booking/Horizon.php b/src/Booking/Horizon.php new file mode 100644 index 0000000..1c98940 --- /dev/null +++ b/src/Booking/Horizon.php @@ -0,0 +1,31 @@ +setTime(0, 0) + ->modify(\sprintf('+%d months', self::MONTHS)); + } +} diff --git a/src/Controller/Admin/BookingAdminController.php b/src/Controller/Admin/BookingAdminController.php new file mode 100644 index 0000000..b1b844d --- /dev/null +++ b/src/Controller/Admin/BookingAdminController.php @@ -0,0 +1,88 @@ +createForm(BookingFilterType::class, $filter); + // A GET form submits itself by navigation, so handleRequest() reads the query string. + // Unfiltered first visits simply keep the constructor's defaults. + $form->handleRequest($request); + + return $this->render('admin/bookings.html.twig', [ + 'form' => $form, + 'filter' => $filter, + 'bookings' => $this->bookings->findForAdmin($filter), + 'series' => $this->series->findActive($filter->fromDate(), $filter->user, $filter->type), + ]); + } + + /** + * The modal behind an occupied shape on the floor plan, for an administrator. + * + * Not app_booking_edit: a shape can hold several bookings on one day once part-day spans + * exist, so "the booking" has no single referent -- and editing reschedules, which an + * administrator deliberately may not do to somebody else's booking. + */ + #[Route('/admin/spot/{id}/{date}', name: 'app_admin_spot', requirements: ['date' => DateParam::PATTERN], methods: ['GET'])] + public function spot(Request $request, Resource $resource, string $date): Response + { + $selected = DateParam::parseOrToday($date); + + return $this->render('admin/_modal_spot.html.twig', [ + 'resource' => $resource, + 'date' => $selected, + // The board's span, so the modal lists exactly what the shape the user clicked shows. + // findForDate() rather than findOverlapping(): it eager-loads the holder, whose name + // is the point of this modal, and it is documented as the locked-only query's + // counterpart for reading. Filtering a dozen rows in PHP is cheaper than a second + // query shape to keep in sync. + 'bookings' => array_values(array_filter( + $this->bookings->findForDate($selected, BoardController::timeOf($request), $resource->getType()), + static fn ($booking): bool => $booking->getResource()->getId() === $resource->getId(), + )), + ]); + } +} diff --git a/src/Controller/BookingController.php b/src/Controller/BookingController.php index 73827d5..f590b07 100644 --- a/src/Controller/BookingController.php +++ b/src/Controller/BookingController.php @@ -8,6 +8,7 @@ use App\Booking\AlreadyHoldingResourceException; use App\Booking\BoardPresenter; use App\Booking\BookingManager; use App\Booking\DateParam; +use App\Booking\Horizon; use App\Booking\ResourceAlreadyBookedException; use App\Booking\Spot; use App\Booking\SeriesResult; @@ -21,6 +22,8 @@ use App\Form\BookingType; use App\Htmx\HxTrait; use App\Repository\BookingRepository; use App\Repository\BookingSeriesRepository; +use App\Security\Role; +use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormInterface; @@ -38,6 +41,7 @@ final class BookingController extends AbstractController private readonly BookingManager $bookings, private readonly BookingRepository $repository, private readonly BookingSeriesRepository $series, + private readonly LoggerInterface $logger, ) { } @@ -82,9 +86,13 @@ final class BookingController extends AbstractController $this->currentUser(), ); $data = new BookingRequest($selected, $spot->suggested() ?? $boardTime); + // An admin books on behalf of people and cleans up after the fact, so the three-month + // horizon that keeps desks from being blocked half a year ahead does not bind them. + $data->unlimitedHorizon = $this->isGranted(Role::ADMIN); $form = $this->createForm(BookingType::class, $data, [ 'action' => $this->generateUrl('app_booking_new', ['id' => $resource->getId(), 'date' => $date] + $this->timeParameters($boardTime)), + 'horizon' => $data->unlimitedHorizon ? null : Horizon::latest(), ]); $form->handleRequest($request); @@ -122,6 +130,8 @@ final class BookingController extends AbstractController #[Route('/bookings/{id}/edit', name: 'app_booking_edit', methods: ['GET', 'POST'])] public function edit(Request $request, Booking $booking): Response { + // Deliberately not widened for admins: rescheduling somebody else's booking is a + // different act from freeing a desk. Admins release; they do not move. $this->denyUnlessOwn($booking); $boardTime = BoardController::timeOf($request); @@ -154,44 +164,95 @@ final class BookingController extends AbstractController ]); } - #[Route('/bookings/{id}/release', name: 'app_booking_release', methods: ['POST'])] + /** + * GET renders the confirmation modal, POST does the release -- the sibling apps' shape for + * every destructive action (see templates/htmx_confirmation_modal.html.twig). The confirm + * button posts back to this same URI, so `?return=` survives the round trip on its own. + */ + #[Route('/bookings/{id}/release', name: 'app_booking_release', methods: ['GET', 'POST'])] public function release(Request $request, Booking $booking): Response { - $this->denyUnlessOwn($booking); + $this->denyUnlessMayManage($booking); + + if (!$request->isMethod(Request::METHOD_POST)) { + return $this->render('booking/_modal_release.html.twig', [ + 'booking' => $booking, + 'csrf_token_id' => 'release-'.$booking->getId(), + ]); + } + $this->denyUnlessCsrfValid($request, 'release-'.$booking->getId()); $date = $booking->getDate(); $type = $booking->getResource()->getType(); $label = $booking->getResource()->getLabel(); - $this->bookings->release($booking); + $holder = $booking->getUser(); + $onBehalf = !$booking->belongsTo($this->currentUser()); - $this->addFlash('success', \sprintf('%s freigegeben.', $label)); - - // The bookings list posts this as an ordinary form and has no board fragments to swap. - if (!$request->headers->has('HX-Request')) { - return $this->redirectToRoute('app_booking'); + if ($onBehalf) { + // The row is about to be deleted, so without this line nothing records that an + // administrator freed somebody else's desk. + $this->logger->info('Admin released a booking on behalf of another user.', [ + 'admin' => $this->currentUser()->getEmail(), + 'holder' => $holder->getEmail(), + 'resource' => $label, + 'date' => $date->format('Y-m-d'), + 'time' => (string) $booking->getTimeRange(), + ]); } - return $this->boardResponse($type, $date, BoardController::timeOf($request)); + $this->bookings->release($booking); + + $this->addFlash('success', $onBehalf + ? \sprintf('%s freigegeben (%s).', $label, $holder->getName()) + : \sprintf('%s freigegeben.', $label)); + + // The two lists have no board fragments to swap, so they say where they want to end up + // and get a genuine navigation. Everything else came from the board and is swapped in + // place -- including the modal the confirmation was opened over. + if (null !== $this->returnRoute($request)) { + return $this->htmxRedirect($request, $this->generateUrl((string) $this->returnRoute($request))); + } + + return $this->boardResponse($type, $date, BoardController::timeOf($request), dismissModal: true); } /** * Ends a series from today onwards. Past occurrences are history and stay put. */ - #[Route('/series/{id}/end', name: 'app_series_end', methods: ['POST'])] + #[Route('/series/{id}/end', name: 'app_series_end', methods: ['GET', 'POST'])] public function endSeries(Request $request, BookingSeries $series): Response { - if ($series->getUser()->getId() !== $this->currentUser()->getId()) { - throw $this->createAccessDeniedException('A series may only be ended by the person who created it.'); + $this->denyUnlessMayManageSeries($series); + + if (!$request->isMethod(Request::METHOD_POST)) { + return $this->render('booking/_modal_end_series.html.twig', [ + 'series' => $series, + 'csrf_token_id' => 'end-series-'.$series->getId(), + ]); } $this->denyUnlessCsrfValid($request, 'end-series-'.$series->getId()); + $onBehalf = $series->getUser()->getId() !== $this->currentUser()->getId(); + + if ($onBehalf) { + $this->logger->info('Admin ended a series on behalf of another user.', [ + 'admin' => $this->currentUser()->getEmail(), + 'holder' => $series->getUser()->getEmail(), + 'resource' => $series->getResource()->getLabel(), + 'series' => $series->getId(), + ]); + } + $removed = $this->bookings->endSeries($series, new \DateTimeImmutable('today')); - $this->addFlash('success', \sprintf('Serie beendet, %d Termine entfernt.', $removed)); + $this->addFlash('success', $onBehalf + ? \sprintf('Serie von %s beendet, %d Termine entfernt.', $series->getUser()->getName(), $removed) + : \sprintf('Serie beendet, %d Termine entfernt.', $removed)); - return $this->redirectToRoute('app_booking'); + // A series is only ever ended from a list, so this always navigates. + return $this->htmxRedirect($request, $this->generateUrl($this->returnRoute($request) ?? 'app_booking')); } private function createSeries(BookingRequest $data, Resource $resource): SeriesResult @@ -245,11 +306,18 @@ final class BookingController extends AbstractController * The whole response is out-of-band board blocks. Callers pair it with hx-swap="none" * (or an outerHTML target that should vanish, as the modal does). */ - private function boardResponse(ResourceType $type, \DateTimeImmutable $date, TimeRange $time): Response + private function boardResponse(ResourceType $type, \DateTimeImmutable $date, TimeRange $time, bool $dismissModal = false): Response { + // The confirmation modal swaps *itself* away, which cannot reach the modal it was opened + // over -- so that one is removed out of band instead. Only on this path: on every other + // one the modal is the swap target and deleting it would leave the swap with nowhere to go. + $blocks = $dismissModal + ? ['calendar', 'floorplan', 'flashes', 'modal_dismiss'] + : ['calendar', 'floorplan', 'flashes']; + return $this->htmxOobResponse( 'board/index.html.twig', - ['calendar', 'floorplan', 'flashes'], + $blocks, $this->board->context($type, $date, $time, $this->currentUser()), $this->generateUrl('app_board', BoardPresenter::urlParameters($type, $date, $time)), ); @@ -270,6 +338,39 @@ final class BookingController extends AbstractController } } + /** + * Releasing is the one thing an administrator may do to a booking that is not theirs -- a + * colleague calls in sick and their desk has to go back into the pool. + */ + private function denyUnlessMayManage(Booking $booking): void + { + if (!$booking->belongsTo($this->currentUser()) && !$this->isGranted(Role::ADMIN)) { + throw $this->createAccessDeniedException('A booking may only be released by the person holding it or by an administrator.'); + } + } + + private function denyUnlessMayManageSeries(BookingSeries $series): void + { + if ($series->getUser()->getId() !== $this->currentUser()->getId() && !$this->isGranted(Role::ADMIN)) { + throw $this->createAccessDeniedException('A series may only be ended by the person who created it or by an administrator.'); + } + } + + /** + * Where to navigate after a confirmed action, or null when the caller is the board and wants + * its fragments swapped instead. The opener says which -- the admin screen sends + * ?return=admin so freeing somebody else's desk does not drop the administrator on their own + * bookings. + */ + private function returnRoute(Request $request): ?string + { + return match ($request->query->get('return')) { + 'admin' => 'app_admin_booking', + 'bookings' => 'app_booking', + default => null, + }; + } + private function denyUnlessCsrfValid(Request $request, string $tokenId): void { if (!$this->isCsrfTokenValid($tokenId, $request->request->getString('_token'))) { diff --git a/src/Entity/BookingSeries.php b/src/Entity/BookingSeries.php index a6fb065..205bdcf 100644 --- a/src/Entity/BookingSeries.php +++ b/src/Entity/BookingSeries.php @@ -22,9 +22,6 @@ use Doctrine\ORM\Mapping as ORM; #[ORM\Entity(repositoryClass: BookingSeriesRepository::class)] class BookingSeries { - /** A series may not run further out than this; desks are not booked years ahead. */ - public const MAX_MONTHS = 12; - #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] diff --git a/src/Form/Admin/BookingFilter.php b/src/Form/Admin/BookingFilter.php new file mode 100644 index 0000000..3bf6554 --- /dev/null +++ b/src/Form/Admin/BookingFilter.php @@ -0,0 +1,46 @@ +from = new \DateTimeImmutable('today'); + $this->to = Horizon::latest(); + } + + /** + * Defaults applied here rather than in the query, so the template can show the window it is + * actually listing even when a field was cleared. + */ + public function fromDate(): \DateTimeImmutable + { + return ($this->from ?? new \DateTimeImmutable('today'))->setTime(0, 0); + } + + public function toDate(): \DateTimeImmutable + { + return ($this->to ?? $this->fromDate()->modify('+1 year'))->setTime(0, 0); + } +} diff --git a/src/Form/Admin/BookingFilterType.php b/src/Form/Admin/BookingFilterType.php new file mode 100644 index 0000000..6355db4 --- /dev/null +++ b/src/Form/Admin/BookingFilterType.php @@ -0,0 +1,74 @@ +add('from', DateType::class, [ + 'label' => 'Von', + 'widget' => 'single_text', + 'input' => 'datetime_immutable', + 'required' => false, + ]) + ->add('to', DateType::class, [ + 'label' => 'Bis', + 'widget' => 'single_text', + 'input' => 'datetime_immutable', + 'required' => false, + ]) + ->add('type', EnumType::class, [ + 'label' => 'Art', + 'class' => ResourceType::class, + 'required' => false, + 'placeholder' => 'Alle Arten', + 'choice_label' => static fn (ResourceType $type): string => $type->plural(), + ]) + ->add('user', EntityType::class, [ + 'label' => 'Person', + 'class' => User::class, + 'required' => false, + 'placeholder' => 'Alle Personen', + 'choice_label' => 'name', + 'query_builder' => static fn (EntityRepository $repository) => $repository + ->createQueryBuilder('u') + ->orderBy('u.name', 'ASC'), + ]); + } + + public function configureOptions(OptionsResolver $resolver): void + { + $resolver->setDefaults([ + 'data_class' => BookingFilter::class, + 'method' => 'GET', + 'csrf_protection' => false, + ]); + } + + /** + * No prefix, so the query string reads ?from=&to=&type=&user= rather than ?booking_filter[from]=. + */ + public function getBlockPrefix(): string + { + return ''; + } +} diff --git a/src/Form/BookingRequest.php b/src/Form/BookingRequest.php index 785d3b5..19268b7 100644 --- a/src/Form/BookingRequest.php +++ b/src/Form/BookingRequest.php @@ -4,9 +4,9 @@ declare(strict_types=1); namespace App\Form; +use App\Booking\Horizon; use App\Booking\RecurrenceRule; use App\Booking\TimeRange; -use App\Entity\BookingSeries; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Context\ExecutionContextInterface; @@ -20,6 +20,13 @@ final class BookingRequest #[Assert\GreaterThanOrEqual('today', message: 'Für vergangene Tage kann nicht gebucht werden.')] public ?\DateTimeImmutable $date = null; + /** + * Lifts the Horizon cap for this request. Set by the controller for ROLE_ADMIN -- a DTO + * cannot reach the security container, and giving it one would make these rules untestable + * in isolation. + */ + public bool $unlimitedHorizon = false; + public bool $allDay = true; public ?\DateTimeImmutable $startTime = null; @@ -85,6 +92,8 @@ final class BookingRequest } } + $this->validateHorizon($context); + if (!$this->repeats()) { return; } @@ -101,14 +110,31 @@ final class BookingRequest return; } - $latest = ($this->date ?? new \DateTimeImmutable('today'))->modify(\sprintf('+%d months', BookingSeries::MAX_MONTHS)); - if ($this->until > $latest) { - $context->buildViolation(\sprintf('Eine Serie darf höchstens %d Monate laufen.', BookingSeries::MAX_MONTHS)) + if (!$this->unlimitedHorizon && $this->until > Horizon::latest()) { + $context->buildViolation(\sprintf('Eine Serie darf höchstens %d Monate in die Zukunft laufen.', Horizon::MONTHS)) ->atPath('until') ->addViolation(); } } + /** + * The booking horizon, measured from today rather than from the chosen day -- see Horizon. + * Not an attribute constraint because it has to be liftable per role, which an attribute + * cannot express. + */ + private function validateHorizon(ExecutionContextInterface $context): void + { + if ($this->unlimitedHorizon || null === $this->date) { + return; + } + + if ($this->date > Horizon::latest()) { + $context->buildViolation(\sprintf('Es kann höchstens %d Monate im Voraus gebucht werden.', Horizon::MONTHS)) + ->atPath('date') + ->addViolation(); + } + } + private static function timeOf(int $minutes): \DateTimeImmutable { // 24:00 has no representation as a time-of-day, so an all-day end never reaches here. diff --git a/src/Form/BookingType.php b/src/Form/BookingType.php index 743dffc..c860d04 100644 --- a/src/Form/BookingType.php +++ b/src/Form/BookingType.php @@ -17,11 +17,19 @@ final class BookingType extends AbstractType { public function buildForm(FormBuilderInterface $builder, array $options): void { + // The native date picker refuses anything past the horizon by itself, so the server rule + // in BookingRequest is a backstop rather than the first thing a user meets. Null for an + // admin, who has no horizon. + $horizon = $options['horizon'] instanceof \DateTimeImmutable + ? ['max' => $options['horizon']->format('Y-m-d')] + : []; + $builder ->add('date', DateType::class, [ 'label' => 'Tag', 'widget' => 'single_text', 'input' => 'datetime_immutable', + 'attr' => $horizon, ]) ->add('allDay', CheckboxType::class, [ 'label' => 'Ganzer Tag', @@ -53,6 +61,7 @@ final class BookingType extends AbstractType 'widget' => 'single_text', 'input' => 'datetime_immutable', 'required' => false, + 'attr' => $horizon, ]); } @@ -60,6 +69,8 @@ final class BookingType extends AbstractType { $resolver->setDefaults([ 'data_class' => BookingRequest::class, + 'horizon' => null, ]); + $resolver->setAllowedTypes('horizon', ['null', \DateTimeImmutable::class]); } } diff --git a/src/Repository/BookingRepository.php b/src/Repository/BookingRepository.php index d5adf0f..5aca816 100644 --- a/src/Repository/BookingRepository.php +++ b/src/Repository/BookingRepository.php @@ -10,6 +10,7 @@ use App\Entity\BookingSeries; use App\Entity\Resource; use App\Entity\ResourceType; use App\Entity\User; +use App\Form\Admin\BookingFilter; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; @@ -169,6 +170,41 @@ class BookingRepository extends ServiceEntityRepository return $counts; } + /** + * Every booking in a window, across everybody -- what the admin list renders. + * + * findForUserInRange() is the same query with a mandatory user; this one keeps it optional + * and eager-selects the holder as well, because the admin list names a different person on + * every row and would otherwise fire a query per booking. + * + * @return list + */ + public function findForAdmin(BookingFilter $filter): array + { + $qb = $this->createQueryBuilder('b') + ->addSelect('u', 'r', 'g', 's') + ->join('b.user', 'u') + ->join('b.resource', 'r') + ->leftJoin('r.group', 'g') + ->leftJoin('b.series', 's') + ->andWhere('b.date BETWEEN :from AND :to') + ->setParameter('from', $filter->fromDate()) + ->setParameter('to', $filter->toDate()) + ->orderBy('b.date', 'ASC') + ->addOrderBy('b.startMinute', 'ASC') + ->addOrderBy('r.label', 'ASC'); + + if (null !== $filter->user) { + $qb->andWhere('b.user = :user')->setParameter('user', $filter->user); + } + + if (null !== $filter->type) { + $qb->andWhere('r.type = :type')->setParameter('type', $filter->type); + } + + return $qb->getQuery()->getResult(); + } + /** * The occurrences an "end this series" would remove. * diff --git a/src/Repository/BookingSeriesRepository.php b/src/Repository/BookingSeriesRepository.php index e21b8fd..4e361c6 100644 --- a/src/Repository/BookingSeriesRepository.php +++ b/src/Repository/BookingSeriesRepository.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Repository; use App\Entity\BookingSeries; +use App\Entity\ResourceType; use App\Entity\User; use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository; use Doctrine\Persistence\ManagerRegistry; @@ -24,15 +25,38 @@ class BookingSeriesRepository extends ServiceEntityRepository */ public function findActiveForUser(User $user, \DateTimeImmutable $on): array { - return $this->createQueryBuilder('s') - ->addSelect('r') + return $this->findActive($on, $user); + } + + /** + * Every series still running on the given day, optionally narrowed to one person or one kind + * of resource. The admin list is the caller that needs it across everybody. + * + * The user is eager-selected because the admin list names the holder of every row -- without + * it the page fires a query per series. + * + * @return list + */ + public function findActive(\DateTimeImmutable $on, ?User $user = null, ?ResourceType $type = null): array + { + $qb = $this->createQueryBuilder('s') + ->addSelect('u', 'r', 'g') + ->join('s.user', 'u') ->join('s.resource', 'r') - ->andWhere('s.user = :user') + ->leftJoin('r.group', 'g') ->andWhere('s.endDate >= :on') - ->setParameter('user', $user) ->setParameter('on', $on->setTime(0, 0)) ->orderBy('s.startDate', 'ASC') - ->getQuery() - ->getResult(); + ->addOrderBy('u.name', 'ASC'); + + if (null !== $user) { + $qb->andWhere('s.user = :user')->setParameter('user', $user); + } + + if (null !== $type) { + $qb->andWhere('r.type = :type')->setParameter('type', $type); + } + + return $qb->getQuery()->getResult(); } } diff --git a/templates/admin/_modal_spot.html.twig b/templates/admin/_modal_spot.html.twig new file mode 100644 index 0000000..305ca71 --- /dev/null +++ b/templates/admin/_modal_spot.html.twig @@ -0,0 +1,52 @@ +{# + What an administrator sees when clicking an occupied shape on the floor plan. + + Not the edit modal: a shape can hold several bookings on one day once part-day spans exist, + so "the booking" has no single referent -- and an administrator releases rather than + reschedules. Each row posts to the ordinary app_booking_release route, whose ownership check + admits an admin, so there is still one code path that removes a booking. +#} +{% extends 'htmx_modal.html.twig' %} + +{% block title %}{{ resource.label }} verwalten{% endblock %} + +{% block content %} +

+ {{ resource.type.label }} · {{ date|format_datetime(pattern: 'EEEE, d. MMMM y') }} + {%- if resource.group %} · {{ resource.group.label }}{% endif %} +

+ + {% if bookings is empty %} +

In diesem Zeitraum liegt hier keine Buchung.

+ {% else %} +
    + {% for booking in bookings %} +
  • +
    +
    {{ booking.user.name }}
    +
    + {{ booking.timeRange }} + {% if booking.partOfSeries %} + Serie + {% endif %} +
    +
    + {# Same as the edit modal: opens the shared confirmation modal over this one. + Confirming repaints the board out of band and removes both. #} + +
    • + {% endfor %} +
+ {% endif %} + +
+ +
+{% endblock %} diff --git a/templates/admin/bookings.html.twig b/templates/admin/bookings.html.twig new file mode 100644 index 0000000..27c2da2 --- /dev/null +++ b/templates/admin/bookings.html.twig @@ -0,0 +1,104 @@ +{# + Every booking, not just yours. The point of the screen is the "Freigeben" button on somebody + else's row: when a colleague calls in sick, their desk has to go back into the pool. + + Filtering is a plain GET form -- the house rule hands navigation to the browser. The two + destructive actions open the shared confirmation modal via htmx and then navigate back here, + which is what ?return=admin says; without it a confirmed release swaps board fragments this + page does not have. +#} +{% extends 'layout.html.twig' %} + +{% block title %}Verwaltung · ephub{% endblock %} + +{% block content %} + ← Übersicht + +

Verwaltung

+

Buchungen aller Mitarbeitenden.

+ + {{ form_start(form, { attr: { class: 'mt-6 rounded-md border border-gray-200 p-4' } }) }} +
+ {{ form_row(form.from) }} + {{ form_row(form.to) }} + {{ form_row(form.type) }} + {{ form_row(form.user) }} +
+
+ Zurücksetzen + +
+ {{ form_end(form) }} + + {% if series is not empty %} +

Serien

+
    + {% for entry in series %} +
  • +
    +
    + {{ entry.user.name }} + · {{ entry.resource.label }} + {%- if entry.resource.group %} · {{ entry.resource.group.label }}{% endif %} +
    +
    + {{ entry.describe }} · bis {{ entry.endDate|format_datetime(pattern: 'd. MMMM y') }} +
    +
    + +
    • + {% endfor %} +
+ {% endif %} + +

Termine

+

+ {{ filter.fromDate|format_datetime(pattern: 'd. MMMM y') }} bis + {{ filter.toDate|format_datetime(pattern: 'd. MMMM y') }} +

+ + {% if bookings is empty %} +

Keine Buchungen in diesem Zeitraum.

+ {% else %} +
    + {% for booking in bookings %} +
  • +
    +
    + {{ booking.user.name }} + + · {{ booking.date|format_datetime(pattern: 'EEE, d. MMM y') }} + · {{ booking.timeRange }} + +
    +
    + {{ booking.resource.type.label }} · {{ booking.resource.label }} + {%- if booking.resource.group %} · {{ booking.resource.group.label }}{% endif %} + {% if booking.partOfSeries %} + Serie + {% endif %} + {% if booking.overridden %} + abweichend + {% endif %} +
    +
    +
    + Im Plan + +
    +
    • + {% endfor %} +
+ {% endif %} +{% endblock %} diff --git a/templates/board/_calendar.html.twig b/templates/board/_calendar.html.twig index d4fa6d0..94c926e 100644 --- a/templates/board/_calendar.html.twig +++ b/templates/board/_calendar.html.twig @@ -37,6 +37,11 @@ {% set classes = classes ~ ' border-primary bg-primary text-white font-bold' %} {% elseif not day.in_month %} {% set classes = classes ~ ' border-transparent text-gray-300' %} + {% elseif day.is_past %} + {# Dashed rather than the weekend's grey fill: the two states must not be + confusable, and the day stays clickable -- looking back at a past plan is + legitimate, only booking it is not. #} + {% set classes = classes ~ ' border-dashed border-gray-200 text-gray-400' %} {% elseif day.is_weekend %} {% set classes = classes ~ ' border-transparent bg-gray-50 text-gray-400' %} {% else %} @@ -49,15 +54,16 @@ {% endfor %} @@ -68,6 +74,7 @@
eigene Buchung
+
gestrichelt = vergangen, nicht mehr buchbar
Zahl = belegte Plätze{{ selected_time.allDay ? ' an dem Tag' : ' zwischen ' ~ selected_time.start ~ ' und ' ~ selected_time.end }}
diff --git a/templates/board/index.html.twig b/templates/board/index.html.twig index 9551e2e..67d131f 100644 --- a/templates/board/index.html.twig +++ b/templates/board/index.html.twig @@ -32,3 +32,14 @@ {% endblock %} + +{# + Removes whatever modal is open, out of band. Only BookingController::boardResponse() renders + it, and only for a confirmation that was opened over another modal -- the confirmation swaps + itself away by targeting its own element, which leaves the one underneath behind. + + Never rendered as part of the page: nothing in the layout calls this block. +#} +{% block modal_dismiss %} +
+{% endblock %} diff --git a/templates/board/plan/_spot.html.twig b/templates/board/plan/_spot.html.twig index bb2ee3f..7ff00c4 100644 --- a/templates/board/plan/_spot.html.twig +++ b/templates/board/plan/_spot.html.twig @@ -53,6 +53,14 @@ {% endif %} {% set interactive = not is_past and spot.actionable %} + {# An administrator may free somebody else's desk -- a colleague calls in sick and the spot + should go back into the pool. Colours are untouched: MINE still means mine, and only + clickability widens. Past days stay inert for admins too; there is no past-write path. + + Only a shape that is not already actionable: a partly booked one still opens the create + modal, because taking the free window is the more useful act there and an admin can reach + that booking from /admin/bookings anyway. #} + {% set manageable = not is_past and not interactive and is_granted('ROLE_ADMIN') and spot.bookings is not empty %} {# One line per booking, then the window still free if there is one -- that last line is the reason a partly booked shape is worth clicking, so it earns its place. #} @@ -77,8 +85,8 @@ {% set lines = lines|slice(0, MAX_LINES - 1)|merge(['+' ~ (lines|length - MAX_LINES + 1) ~ ' weitere']) %} {% endif %} - diff --git a/templates/booking/_modal_edit.html.twig b/templates/booking/_modal_edit.html.twig index 7e840e7..0ec79d4 100644 --- a/templates/booking/_modal_edit.html.twig +++ b/templates/booking/_modal_edit.html.twig @@ -4,7 +4,8 @@ differing from it. Releasing lives here too rather than on the floor plan, so the SVG needs no confirm dialogs - or inline CSRF tokens -- clicking your own desk always just opens this. + or inline CSRF tokens -- clicking your own desk always just opens this. The release button + opens the shared confirmation modal over this one. #} {% extends 'htmx_modal.html.twig' %} @@ -32,14 +33,13 @@ {% include 'booking/_fields.html.twig' %}
- {# type="button" with its own hx-post: htmx issues the release request directly, so - this never submits the surrounding edit form and no nested
is needed. #} + {# type="button" with its own hx-get: htmx opens the confirmation modal over this + one, so nothing submits the surrounding edit form and no nested is needed. + Confirming removes both modals -- see BookingController::boardResponse(). #} diff --git a/templates/booking/_modal_end_series.html.twig b/templates/booking/_modal_end_series.html.twig new file mode 100644 index 0000000..8f5898f --- /dev/null +++ b/templates/booking/_modal_end_series.html.twig @@ -0,0 +1,17 @@ +{% extends 'htmx_confirmation_modal.html.twig' %} + +{% block title %}Serie beenden?{% endblock %} + +{% block content %} +

+ {{ series.resource.label }} · {{ series.describe }} + {% if series.user.id != app.user.id %} + — Serie von {{ series.user.name }} + {% endif %} +

+

+ Alle Termine ab heute werden entfernt. Vergangene Termine bleiben als Historie stehen. +

+{% endblock %} + +{% block button_confirm %}Serie beenden{% endblock %} diff --git a/templates/booking/_modal_release.html.twig b/templates/booking/_modal_release.html.twig new file mode 100644 index 0000000..e69722e --- /dev/null +++ b/templates/booking/_modal_release.html.twig @@ -0,0 +1,22 @@ +{% extends 'htmx_confirmation_modal.html.twig' %} + +{% block title %}Buchung freigeben?{% endblock %} + +{% block content %} +

+ {{ booking.resource.type.label }} {{ booking.resource.label }} + {%- if booking.resource.group %} ({{ booking.resource.group.label }}){% endif %} + am {{ booking.date|format_datetime(pattern: 'EEEE, d. MMMM y') }}, {{ booking.timeRange }} + {% if not booking.belongsTo(app.user) %} + — gebucht von {{ booking.user.name }} + {% endif %} +

+ + {% if booking.partOfSeries %} +

+ Teil einer Serie: {{ booking.series.describe }}. Es wird nur dieser Tag freigegeben. +

+ {% endif %} +{% endblock %} + +{% block button_confirm %}Freigeben{% endblock %} diff --git a/templates/booking/index.html.twig b/templates/booking/index.html.twig index 591e56c..b30bdf8 100644 --- a/templates/booking/index.html.twig +++ b/templates/booking/index.html.twig @@ -22,12 +22,14 @@ {{ entry.describe }} · bis {{ entry.endDate|format_datetime(pattern: 'd. MMMM y') }}
- {# Ends the series from today on; past occurrences are history and stay put. #} - - - - + {# Opens the confirmation modal; the CSRF token and the actual POST live in + there, which is why this row carries neither a form nor a token. #} + {% endfor %} @@ -62,12 +64,14 @@
Im Plan - {# Ordinary form post: this page has no board fragments to swap, so it - reloads afterwards rather than pretending to be htmx. #} -
- - -
+ {# ?return=bookings: this page has no board fragments to swap, so the + confirmed release navigates back here instead. #} +
{% endfor %} diff --git a/templates/dashboard/index.html.twig b/templates/dashboard/index.html.twig index f62d7ba..745cfe3 100644 --- a/templates/dashboard/index.html.twig +++ b/templates/dashboard/index.html.twig @@ -47,6 +47,23 @@

Alle Termine und Serien verwalten.

+ + {# + The header is deliberately not navigation, so the dashboard is the only place an + admin screen can be reached from. + #} + {% if is_granted('ROLE_ADMIN') %} + +
+ {{ ux_icon('lucide:shield-check', { class: 'h-6 w-6 shrink-0 text-primary' }) }} + Verwaltung +
+

+ Buchungen aller Mitarbeitenden einsehen und ggf. freigeben. +

+ + {% endif %}

Als Nächstes

diff --git a/templates/htmx_confirmation_modal.html.twig b/templates/htmx_confirmation_modal.html.twig new file mode 100644 index 0000000..f736f84 --- /dev/null +++ b/templates/htmx_confirmation_modal.html.twig @@ -0,0 +1,66 @@ +{# + The shared "are you sure?" modal, ported from myep/myep-team's htmx_confirmation_modal.html.twig. + + It replaces both native confirmation mechanisms this app used before: htmx's `hx-confirm` + (a window.confirm, unstyleable and invisible to the page) and `onsubmit="return confirm(...)"` + on the list forms. Both are gone; neither may come back. + + The flow is the sibling one: a route serves GET (render this) and POST (do the thing), the + opener is a plain `hx-get` button appending to , and the confirm button posts back to + the *same* URI -- so the query string that brought the user here, `?return=` included, is + carried over for free. + + The CSRF token lives here rather than on the opener, which is why the lists no longer carry + hidden token fields at all. + + Blocks: title, content, button_attribute (replace the hx-* defaults), button_confirm, + button_cancel. Callers must pass `csrf_token_id`. +#} +
+
+
+
+
+
+ {{ ux_icon('lucide:triangle-alert', { class: 'w-7 h-7 shrink-0 text-amber-500' }) }} +
+ {% block title %}Bist du sicher?{% endblock %} +
+
+ +
+
+ {% block content %}{% endblock %} + +
+ + {# + Targets itself with outerHTML, so a response consisting only of + out-of-band blocks leaves nothing to swap in and this modal disappears -- + the same mechanism the booking modal uses. A response that instead sets + HX-Redirect navigates away and the swap never happens. + #} + +
+
+
+
+
diff --git a/tests/Booking/HorizonTest.php b/tests/Booking/HorizonTest.php new file mode 100644 index 0000000..ea06445 --- /dev/null +++ b/tests/Booking/HorizonTest.php @@ -0,0 +1,50 @@ +format('Y-m-d'), + ); + } + + /** + * PHP's "+3 months" from a 31st overflows into the next month (31 November does not exist, + * so it becomes 1 December). Pinned rather than clamped: this is a cap, and one extra day of + * horizon four times a year is not worth the arithmetic it would take to remove. The + * recurrence code deliberately avoids months for exactly this reason -- see + * RecurrenceRule::intervalDays() -- but a single boundary can live with it. + */ + public function testAMonthEndOverflowsByADayAndThatIsAccepted(): void + { + self::assertSame( + '2026-12-01', + Horizon::latest(new \DateTimeImmutable('2026-08-31'))->format('Y-m-d'), + ); + } + + public function testTheTimePartIsDroppedSoTheComparisonIsPerDay(): void + { + self::assertSame( + '00:00:00', + Horizon::latest(new \DateTimeImmutable('2026-09-14 17:42:13'))->format('H:i:s'), + ); + } + + public function testItDefaultsToToday(): void + { + self::assertEquals( + Horizon::latest(new \DateTimeImmutable('today')), + Horizon::latest(), + ); + } +} diff --git a/tests/Controller/Admin/BookingAdminControllerTest.php b/tests/Controller/Admin/BookingAdminControllerTest.php new file mode 100644 index 0000000..271ca30 --- /dev/null +++ b/tests/Controller/Admin/BookingAdminControllerTest.php @@ -0,0 +1,287 @@ +client = self::createClient(); + $this->client->disableReboot(); + $this->em = self::getContainer()->get(EntityManagerInterface::class); + $this->em->getConnection()->beginTransaction(); + } + + protected function tearDown(): void + { + $this->em->getConnection()->rollBack(); + parent::tearDown(); + } + + public function testAnEmployeeCannotReachTheAdminList(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + + $this->client->request('GET', '/admin/bookings'); + + self::assertSame(Response::HTTP_FORBIDDEN, $this->client->getResponse()->getStatusCode()); + } + + public function testTheAdminListShowsOtherPeoplesBookings(): void + { + $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $this->resource('D1')); + $this->client->loginUser($this->admin()); + + $crawler = $this->client->request('GET', '/admin/bookings'); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('Bea Berger', $crawler->filter('body')->text()); + } + + public function testAnAdminReleasesABookingOnBehalfOfSomebodyElse(): void + { + $booking = $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $this->resource('D1')); + $id = $booking->getId(); + $this->client->loginUser($this->admin()); + + // The two-step flow a browser walks: the list opens the confirmation modal, the modal + // carries the token and posts back to the same URI. + $crawler = $this->client->request('GET', '/admin/bookings'); + self::assertCount(1, $crawler->filter(\sprintf('[hx-get="/bookings/%d/release?return=admin"]', $id))); + + $uri = \sprintf('/bookings/%d/release?return=admin', $id); + $this->client->request('POST', $uri, ['_token' => $this->confirmationToken($uri)], server: ['HTTP_HX-Request' => 'true']); + + // ?return= means a genuine navigation rather than a board swap: this page has no board + // fragments, so HX-Redirect is the honest answer. + self::assertSame('/admin/bookings', $this->client->getResponse()->headers->get('HX-Redirect')); + $this->em->clear(); + self::assertNull($this->em->getRepository(Booking::class)->find($id)); + } + + /** + * The regression that matters: widening the check for admins must not have widened it for + * everybody. + */ + public function testAnEmployeeStillCannotReleaseSomebodyElsesBooking(): void + { + $booking = $this->booking($this->user('bea@ep-reisen.de'), $this->resource('D1')); + $id = $booking->getId(); + $this->client->loginUser($this->user('anna@ep-reisen.de')); + + $this->client->request('POST', \sprintf('/bookings/%d/release', $id), ['_token' => 'whatever']); + + self::assertSame(Response::HTTP_FORBIDDEN, $this->client->getResponse()->getStatusCode()); + // The ownership guard runs before the CSRF one, so asserting on its message is what + // distinguishes "refused because it is not yours" from "refused because of the token". + self::assertStringContainsString( + 'released by the person holding it or by an administrator', + (string) $this->client->getResponse()->getContent(), + ); + self::assertNotNull($this->em->getRepository(Booking::class)->find($id)); + } + + /** + * Admins release; they do not reschedule. Moving somebody else's booking is a different act + * and was deliberately left alone. + */ + public function testAnAdminStillCannotEditSomebodyElsesBooking(): void + { + $booking = $this->booking($this->user('bea@ep-reisen.de'), $this->resource('D1')); + $this->client->loginUser($this->admin()); + + $this->client->request('GET', \sprintf('/bookings/%d/edit', $booking->getId())); + + self::assertSame(Response::HTTP_FORBIDDEN, $this->client->getResponse()->getStatusCode()); + } + + public function testTheListNarrowsToOnePerson(): void + { + $bea = $this->user('bea@ep-reisen.de', 'Bea Berger'); + $this->booking($bea, $this->resource('D1')); + $this->booking($this->user('carl@ep-reisen.de', 'Carl Conrad'), $this->resource('D2')); + $this->client->loginUser($this->admin()); + + $crawler = $this->client->request('GET', '/admin/bookings?user='.$bea->getId()); + + self::assertResponseIsSuccessful(); + $text = $crawler->filter('ul')->text(); + self::assertStringContainsString('Bea Berger', $text); + self::assertStringNotContainsString('Carl Conrad', $text); + } + + public function testTheListNarrowsToOneResourceType(): void + { + $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $this->resource('D1')); + $this->booking($this->user('carl@ep-reisen.de', 'Carl Conrad'), $this->resource('P1', ResourceType::PARKING)); + $this->client->loginUser($this->admin()); + + $crawler = $this->client->request('GET', '/admin/bookings?type=parking'); + + self::assertResponseIsSuccessful(); + $text = $crawler->filter('ul')->text(); + self::assertStringContainsString('Carl Conrad', $text); + self::assertStringNotContainsString('Bea Berger', $text); + } + + /** + * The way into the admin modal from the board: a desk held by a colleague is inert for an + * employee and clickable for an admin. + */ + public function testAnOccupiedDeskIsClickableForAnAdminOnly(): void + { + $desk = $this->resource('D1', ResourceType::DESK, 'desk-01'); + $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $desk); + $date = (new \DateTimeImmutable('today'))->format('Y-m-d'); + + $this->client->loginUser($this->user('anna@ep-reisen.de')); + $crawler = $this->client->request('GET', '/book/desk?date='.$date); + self::assertCount(0, $crawler->filter('g[hx-get*="/admin/spot/"]')); + + $this->client->loginUser($this->admin()); + $crawler = $this->client->request('GET', '/book/desk?date='.$date); + self::assertCount(1, $crawler->filter(\sprintf('g[hx-get="/admin/spot/%d/%s"]', $desk->getId(), $date))); + } + + public function testTheSpotModalListsTheHolderAndReleasesTheBooking(): void + { + $desk = $this->resource('D1', ResourceType::DESK, 'desk-01'); + $booking = $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $desk); + $this->client->loginUser($this->admin()); + + $crawler = $this->client->request( + 'GET', + \sprintf('/admin/spot/%d/%s', $desk->getId(), $booking->getDate()->format('Y-m-d')), + server: ['HTTP_HX-Request' => 'true'], + ); + + self::assertResponseIsSuccessful(); + self::assertStringContainsString('Bea Berger', $crawler->filter('#htmx-modal')->text()); + self::assertCount(1, $crawler->filter(\sprintf('button[hx-get="/bookings/%d/release"]', $booking->getId()))); + } + + /** + * The modal is scoped to the shape that was clicked, not to the whole day. + */ + public function testTheSpotModalShowsOnlyThatResourcesBookings(): void + { + $desk = $this->resource('D1', ResourceType::DESK, 'desk-01'); + $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $desk); + $this->booking($this->user('carl@ep-reisen.de', 'Carl Conrad'), $this->resource('D2', ResourceType::DESK, 'desk-02')); + $this->client->loginUser($this->admin()); + + $crawler = $this->client->request( + 'GET', + \sprintf('/admin/spot/%d/%s', $desk->getId(), (new \DateTimeImmutable('today'))->format('Y-m-d')), + server: ['HTTP_HX-Request' => 'true'], + ); + + $text = $crawler->filter('#htmx-modal')->text(); + self::assertStringContainsString('Bea Berger', $text); + self::assertStringNotContainsString('Carl Conrad', $text); + } + + public function testTheDashboardTileIsAdminOnly(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + self::assertCount(0, $this->client->request('GET', '/')->filter('a[href="/admin/bookings"]')); + + $this->client->loginUser($this->admin()); + self::assertCount(1, $this->client->request('GET', '/')->filter('a[href="/admin/bookings"]')); + } + + /** + * The confirmation modal names the holder, so an administrator cannot free the wrong desk + * without seeing whose it is. This is the whole reason it exists rather than a + * window.confirm. + */ + public function testTheConfirmationModalNamesTheHolder(): void + { + $booking = $this->booking($this->user('bea@ep-reisen.de', 'Bea Berger'), $this->resource('D1')); + $this->client->loginUser($this->admin()); + + $crawler = $this->client->request( + 'GET', + \sprintf('/bookings/%d/release?return=admin', $booking->getId()), + server: ['HTTP_HX-Request' => 'true'], + ); + + self::assertResponseIsSuccessful(); + $modal = $crawler->filter('#htmx-confirmation-modal'); + self::assertStringContainsString('Bea Berger', $modal->text()); + // Posts back to the same URI, which is what carries ?return=admin through the round trip. + self::assertSame( + \sprintf('http://localhost/bookings/%d/release?return=admin', $booking->getId()), + $modal->filter('[hx-post]')->attr('hx-post'), + ); + } + + private function confirmationToken(string $uri): string + { + $crawler = $this->client->request('GET', $uri, server: ['HTTP_HX-Request' => 'true']); + self::assertResponseIsSuccessful(); + + $vals = $crawler->filter('#htmx-confirmation-modal [hx-post]')->attr('hx-vals'); + + return json_decode((string) $vals, true, flags: \JSON_THROW_ON_ERROR)['_token']; + } + + private function booking(User $user, Resource $resource): Booking + { + $booking = new Booking($user, $resource, new \DateTimeImmutable('today')); + $this->em->persist($booking); + $this->em->flush(); + + return $booking; + } + + private function admin(string $email = 'chef@ep-reisen.de', string $name = 'Chef Chefin'): User + { + return $this->user($email, $name, ['ROLE_ADMIN']); + } + + /** + * @param list $roles + */ + private function user(string $email, string $name = 'Test', array $roles = ['ROLE_EMPLOYEE']): User + { + $user = new User($email, $name); + $user->setRoles($roles); + $this->em->persist($user); + $this->em->flush(); + + return $user; + } + + /** + * The floor plan only draws resources whose svgElementId appears in the geometry map. + */ + private function resource(string $label, ResourceType $type = ResourceType::DESK, ?string $svgElementId = null): Resource + { + $resource = new Resource($type, $label, $svgElementId); + $this->em->persist($resource); + $this->em->flush(); + + return $resource; + } +} diff --git a/tests/Controller/BookingControllerTest.php b/tests/Controller/BookingControllerTest.php index 6060b1a..3a30e19 100644 --- a/tests/Controller/BookingControllerTest.php +++ b/tests/Controller/BookingControllerTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Tests\Controller; +use App\Booking\Horizon; use App\Booking\TimeRange; use App\Entity\Booking; use App\Entity\Resource; @@ -184,16 +185,20 @@ class BookingControllerTest extends WebTestCase $this->client->loginUser($anna); - // Releasing lives in the edit modal now, not on the floor plan -- clicking your own desk - // opens that modal, and the token is rendered there. + // The whole two-step flow: the edit modal opens the confirmation modal, and the + // confirmation modal is where the token and the POST live. $crawler = $this->client->request('GET', \sprintf('/bookings/%d/edit', $id), server: ['HTTP_HX-Request' => 'true']); - $vals = $crawler->filter(\sprintf('[hx-post="/bookings/%d/release"]', $id))->attr('hx-vals'); - $token = json_decode((string) $vals, true, flags: \JSON_THROW_ON_ERROR)['_token']; + self::assertCount(1, $crawler->filter(\sprintf('[hx-get="/bookings/%d/release"]', $id))); + + $token = $this->confirmationToken(\sprintf('/bookings/%d/release', $id)); $this->client->request('POST', \sprintf('/bookings/%d/release', $id), ['_token' => $token], server: ['HTTP_HX-Request' => 'true']); + $content = (string) $this->client->getResponse()->getContent(); self::assertResponseIsSuccessful(); - self::assertSame(3, substr_count((string) $this->client->getResponse()->getContent(), 'hx-swap-oob="true"')); + self::assertSame(3, substr_count($content, 'hx-swap-oob="true"')); + // The confirmation swaps itself away; the edit modal underneath it has to go too. + self::assertStringContainsString('id="htmx-modal" hx-swap-oob="delete"', $content); self::assertNull($this->em->getRepository(Booking::class)->find($id)); } @@ -345,14 +350,23 @@ class BookingControllerTest extends WebTestCase $this->client->loginUser($this->user('anna@ep-reisen.de')); $desk = $this->resource('D1'); - $response = $this->submitBookingModal($desk, '2026-10-05', [ + // Relative to today, unlike the rest of this file: the three-month horizon moves with the + // clock, so a fixed end date here would start failing the day it fell outside it. + $start = (new \DateTimeImmutable('today'))->modify('next monday'); + $until = $start->modify('+56 days'); + + $response = $this->submitBookingModal($desk, $start->format('Y-m-d'), [ 'allDay' => '1', 'repeat' => 'four_weekly', - 'until' => '2026-12-31', + 'until' => $until->format('Y-m-d'), ]); self::assertSame(Response::HTTP_OK, $response->getStatusCode()); - self::assertCount(4, $this->em->getRepository(Booking::class)->findBy(['resource' => $desk])); + $occurrences = $this->em->getRepository(Booking::class)->findBy(['resource' => $desk]); + self::assertCount(3, $occurrences); + foreach ($occurrences as $booking) { + self::assertSame(1, (int) $booking->getDate()->format('N')); + } } public function testASeriesWithoutAnEndDateIsRejected(): void @@ -379,7 +393,86 @@ class BookingControllerTest extends WebTestCase ]); self::assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode()); - self::assertStringContainsString('höchstens 12 Monate', (string) $response->getContent()); + self::assertStringContainsString('höchstens 3 Monate', (string) $response->getContent()); + } + + public function testABookingBeyondTheHorizonIsRejected(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + $beyond = Horizon::latest()->modify('+1 day'); + + $response = $this->submitBookingModal($this->resource('D1'), $beyond->format('Y-m-d'), ['allDay' => '1']); + + self::assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode()); + self::assertStringContainsString('höchstens 3 Monate im Voraus', (string) $response->getContent()); + } + + public function testTheLastDayInsideTheHorizonIsStillBookable(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + + $response = $this->submitBookingModal($this->resource('D1'), Horizon::latest()->format('Y-m-d'), ['allDay' => '1']); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + /** + * The hole in the old rule, which measured the cap from the series' own start date: starting + * later simply pushed the horizon out with it. This is the property that would regress + * silently if the measurement ever moved back onto $this->date. + */ + public function testASeriesCannotPushTheHorizonOutByStartingLater(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + $start = (new \DateTimeImmutable('today'))->modify('+2 months'); + + $response = $this->submitBookingModal($this->resource('D1'), $start->format('Y-m-d'), [ + 'allDay' => '1', + 'repeat' => 'weekly', + 'until' => $start->modify('+2 months')->format('Y-m-d'), + ]); + + self::assertSame(Response::HTTP_UNPROCESSABLE_ENTITY, $response->getStatusCode()); + self::assertStringContainsString('höchstens 3 Monate in die Zukunft', (string) $response->getContent()); + } + + public function testAnAdminIsNotBoundByTheHorizon(): void + { + $this->client->loginUser($this->admin('chef@ep-reisen.de')); + $beyond = Horizon::latest()->modify('+2 months'); + + $response = $this->submitBookingModal($this->resource('D1'), $beyond->format('Y-m-d'), ['allDay' => '1']); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + } + + /** + * Past days stay clickable -- looking back at a plan is legitimate -- but they have to say + * that nothing can be booked there, which they did not before. + */ + public function testPastDaysInTheCalendarAreMarkedButStillNavigable(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + $yesterday = (new \DateTimeImmutable('today'))->modify('-1 day')->format('Y-m-d'); + + $crawler = $this->client->request('GET', '/book/desk'); + $cell = $crawler->filter(\sprintf('button[hx-get*="/book/desk/day/%s"]', $yesterday)); + + self::assertCount(1, $cell); + self::assertSame('true', $cell->attr('aria-disabled')); + self::assertStringContainsString('border-dashed', (string) $cell->attr('class')); + } + + public function testFutureDaysInTheCalendarAreNotMarkedAsPast(): void + { + $this->client->loginUser($this->user('anna@ep-reisen.de')); + $tomorrow = (new \DateTimeImmutable('today'))->modify('+1 day')->format('Y-m-d'); + + $crawler = $this->client->request('GET', '/book/desk'); + $cell = $crawler->filter(\sprintf('button[hx-get*="/book/desk/day/%s"]', $tomorrow)); + + self::assertCount(1, $cell); + self::assertNull($cell->attr('aria-disabled')); } /** @@ -479,6 +572,30 @@ class BookingControllerTest extends WebTestCase return $this->client->getResponse(); } + /** + * Opens a confirmation modal and reads its CSRF token off the confirm button's hx-vals -- + * the same round trip a browser makes, rather than reaching into the token manager (which + * has no session to read outside a request). + */ + private function confirmationToken(string $uri): string + { + $crawler = $this->client->request('GET', $uri, server: ['HTTP_HX-Request' => 'true']); + self::assertResponseIsSuccessful(); + + $vals = $crawler->filter('#htmx-confirmation-modal [hx-post]')->attr('hx-vals'); + + return json_decode((string) $vals, true, flags: \JSON_THROW_ON_ERROR)['_token']; + } + + private function admin(string $email, string $name = 'Chef'): User + { + $user = $this->user($email, $name); + $user->setRoles(['ROLE_ADMIN']); + $this->em->flush(); + + return $user; + } + private function user(string $email, string $name = 'Test'): User { $user = new User($email, $name);