diff --git a/assets/app.js b/assets/app.js index 3cb7ffc..96924db 100644 --- a/assets/app.js +++ b/assets/app.js @@ -23,3 +23,11 @@ htmx.config.responseHandling = [ { code: '422', swap: true, error: false }, { code: '[45]..', swap: false, error: true }, ]; + +// Anything else that fails -- a 4xx other than 422, a 5xx, or no response at all -- reloads the +// page. htmx only opens modals and refreshes fragments, so there is always a whole page underneath, +// and every page is a GET: reloading is safe, closes the modal and shows current data. The server +// keeps honest status codes and leaves a flash explaining the reload (HtmxErrorFlashListener). +const reloadPage = () => window.location.reload(); +document.addEventListener('htmx:responseError', reloadPage); +document.addEventListener('htmx:sendError', reloadPage); diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 80203e5..a81bf21 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -10,7 +10,8 @@ security: firewalls: dev: - pattern: ^/(_profiler|_wdt|assets)/ + # _error is the dev-only preview of the error pages (/_error/404, /_error/500). + pattern: ^/(_profiler|_wdt|_error|assets)/ security: false main: lazy: true diff --git a/src/EventListener/HtmxErrorFlashListener.php b/src/EventListener/HtmxErrorFlashListener.php new file mode 100644 index 0000000..1dc1c41 --- /dev/null +++ b/src/EventListener/HtmxErrorFlashListener.php @@ -0,0 +1,41 @@ +getRequest(); + + if (!$request->headers->has('HX-Request') || !$request->hasSession()) { + return; + } + + $session = $request->getSession(); + + if ($session instanceof FlashBagAwareSessionInterface) { + $session->getFlashBag()->add('error', 'Da ist etwas schiefgelaufen. Bitte versuche es erneut.'); + } + } +} diff --git a/src/EventListener/HtmxExceptionListener.php b/src/EventListener/HtmxExceptionListener.php deleted file mode 100644 index d42a03e..0000000 --- a/src/EventListener/HtmxExceptionListener.php +++ /dev/null @@ -1,48 +0,0 @@ - so it appends - * regardless of what the original hx-target was. - * - * allowCustomResponseCode() is what keeps the 200; without it the kernel rewrites it to 500 and - * htmx discards the body again. - */ -#[AsEventListener(event: KernelEvents::EXCEPTION, priority: -10)] -final readonly class HtmxExceptionListener -{ - public function __construct(private Environment $twig) - { - } - - public function __invoke(ExceptionEvent $event): void - { - if (!$event->getRequest()->headers->has('HX-Request')) { - return; - } - - if ($event->hasResponse()) { - return; - } - - $response = new Response($this->twig->render('htmx_error_modal.html.twig'), Response::HTTP_OK); - $response->headers->set('HX-Retarget', 'body'); - $response->headers->set('HX-Reswap', 'beforeend'); - - $event->setResponse($response); - $event->allowCustomResponseCode(); - } -} diff --git a/src/Security/LoginEntryPoint.php b/src/Security/LoginEntryPoint.php index 050e9e5..b7f34c6 100644 --- a/src/Security/LoginEntryPoint.php +++ b/src/Security/LoginEntryPoint.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Security; +use App\Htmx\HxRedirectResponse; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -16,6 +17,10 @@ use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface * * Deliberately the login *page* rather than straight into /auth/init: the page is where a failed * login's flash message is shown, and bouncing there instead would swallow it. + * + * An htmx request -- typically a click after the session expired -- gets HX-Redirect instead: + * a plain 302 is followed invisibly by the browser, and htmx would swap the whole login page into + * whatever the click targeted, a modal included. */ final readonly class LoginEntryPoint implements AuthenticationEntryPointInterface { @@ -25,6 +30,12 @@ final readonly class LoginEntryPoint implements AuthenticationEntryPointInterfac public function start(Request $request, ?AuthenticationException $authException = null): Response { - return new RedirectResponse($this->urlGenerator->generate('app_login')); + $url = $this->urlGenerator->generate('app_login'); + + if ($request->headers->has('HX-Request')) { + return new HxRedirectResponse($url); + } + + return new RedirectResponse($url); } } diff --git a/templates/bundles/TwigBundle/Exception/error.html.twig b/templates/bundles/TwigBundle/Exception/error.html.twig new file mode 100644 index 0000000..49219f3 --- /dev/null +++ b/templates/bundles/TwigBundle/Exception/error.html.twig @@ -0,0 +1,21 @@ +{# + Symfony's error page for anything but a 404, adopted from myep-team. Only used with debug off; + dev keeps showing the exception page (preview at /_error/500). + + Never reached by an htmx request: a failed one reloads its page instead (see assets/app.js). +#} +{% extends 'layout.html.twig' %} + +{% block title %}Fehler · ephub{% endblock %} + +{% block content %} +

+ Oops... Ein Fehler ist aufgetreten +

+

+ Wir haben uns ein paar Notizen dazu gemacht und werden das Problem so schnell wie möglich beseitigen. +

+ + nochmal von vorne + +{% endblock %} diff --git a/templates/bundles/TwigBundle/Exception/error404.html.twig b/templates/bundles/TwigBundle/Exception/error404.html.twig new file mode 100644 index 0000000..ac262f4 --- /dev/null +++ b/templates/bundles/TwigBundle/Exception/error404.html.twig @@ -0,0 +1,16 @@ +{# Symfony's 404 page, adopted from myep-team. Only used with debug off. #} +{% extends 'layout.html.twig' %} + +{% block title %}Nicht gefunden · ephub{% endblock %} + +{% block content %} +

+ Oops... Ein Fehler ist aufgetreten +

+

+ Die von dir aufgerufene Seite können wir leider nicht (mehr) finden. +

+ + nochmal von vorne + +{% endblock %} diff --git a/templates/htmx_error_modal.html.twig b/templates/htmx_error_modal.html.twig deleted file mode 100644 index c5c959d..0000000 --- a/templates/htmx_error_modal.html.twig +++ /dev/null @@ -1,9 +0,0 @@ -{% embed 'htmx_modal.html.twig' %} - {% block title %}Oops…{% endblock %} - {% block content %} -
- Da ist etwas schiefgelaufen. Wir haben uns Notizen gemacht und kümmern uns - schnellstmöglich darum. -
- {% endblock %} -{% endembed %} diff --git a/tests/Controller/Admin/ResourceAdminControllerTest.php b/tests/Controller/Admin/ResourceAdminControllerTest.php index c23e6c8..30f6cc8 100644 --- a/tests/Controller/Admin/ResourceAdminControllerTest.php +++ b/tests/Controller/Admin/ResourceAdminControllerTest.php @@ -43,8 +43,7 @@ class ResourceAdminControllerTest extends WebTestCase $this->client->request('GET', '/admin/resources'); self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); - // Not as htmx: an htmx request gets the error modal instead of the bare status. - $this->client->request('GET', '/admin/resources/new/desk'); + $this->client->request('GET', '/admin/resources/new/desk', server: ['HTTP_HX-Request' => 'true']); self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); } @@ -123,7 +122,7 @@ class ResourceAdminControllerTest extends WebTestCase { $this->client->loginUser($this->admin()); - $this->client->request('GET', '/admin/resources/new/boat'); + $this->client->request('GET', '/admin/resources/new/boat', server: ['HTTP_HX-Request' => 'true']); self::assertResponseStatusCodeSame(Response::HTTP_NOT_FOUND); } @@ -221,7 +220,7 @@ class ResourceAdminControllerTest extends WebTestCase $crawler = $this->client->request('GET', '/admin/resources'); self::assertCount(0, $crawler->filter(\sprintf('[hx-get="/admin/resource-groups/%d/delete"]', $room->getId()))); - $this->client->request('POST', \sprintf('/admin/resource-groups/%d/delete', $room->getId()), ['_token' => 'whatever']); + $this->client->request('POST', \sprintf('/admin/resource-groups/%d/delete', $room->getId()), ['_token' => 'whatever'], server: ['HTTP_HX-Request' => 'true']); self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); } diff --git a/tests/Controller/BookingControllerTest.php b/tests/Controller/BookingControllerTest.php index e042cb4..289cb1e 100644 --- a/tests/Controller/BookingControllerTest.php +++ b/tests/Controller/BookingControllerTest.php @@ -155,12 +155,6 @@ class BookingControllerTest extends WebTestCase self::assertNull($this->em->getRepository(Booking::class)->findOneBy(['resource' => $desk])); } - /** - * Note the status is 200, not 403: HtmxExceptionListener deliberately converts every - * exception on an htmx request -- access denied included -- into a visible error modal, - * because htmx discards a 4xx body and the click would otherwise do nothing at all. - * What matters here is that the booking survives. - */ public function testReleasingAnotherPersonsBookingIsRefused(): void { $booking = new Booking($this->user('bea@ep-reisen.de'), $this->resource('D1'), new \DateTimeImmutable('2026-10-01')); @@ -170,8 +164,7 @@ class BookingControllerTest extends WebTestCase $this->client->loginUser($this->user('anna@ep-reisen.de')); $this->client->request('POST', \sprintf('/bookings/%d/release', $booking->getId()), server: ['HTTP_HX-Request' => 'true']); - $response = $this->client->getResponse(); - self::assertSame('body', $response->headers->get('HX-Retarget')); + self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); self::assertNotNull($this->em->getRepository(Booking::class)->find($booking->getId())); } diff --git a/tests/EventListener/HtmxErrorFlashListenerTest.php b/tests/EventListener/HtmxErrorFlashListenerTest.php new file mode 100644 index 0000000..6c89e19 --- /dev/null +++ b/tests/EventListener/HtmxErrorFlashListenerTest.php @@ -0,0 +1,66 @@ +client = self::createClient(); + $this->client->disableReboot(); + $this->em = self::getContainer()->get(EntityManagerInterface::class); + $this->em->getConnection()->beginTransaction(); + + $user = new User('anna@ep-reisen.de', 'Anna'); + $user->setRoles(['ROLE_EMPLOYEE']); + $this->em->persist($user); + $this->em->flush(); + $this->client->loginUser($user); + } + + protected function tearDown(): void + { + $this->em->getConnection()->rollBack(); + parent::tearDown(); + } + + public function testAFailedHtmxRequestKeepsItsStatusAndLeavesAFlash(): void + { + $this->client->request('GET', '/admin/resources', server: ['HTTP_HX-Request' => 'true']); + self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); + self::assertFalse($this->client->getResponse()->headers->has('HX-Retarget')); + + $this->client->request('GET', '/'); + self::assertSelectorTextContains('#flashes', self::MESSAGE); + } + + /** + * An ordinary page request shows Symfony's error page instead, so a flash would only turn up + * later, out of context. + */ + public function testAnOrdinaryRequestLeavesNoFlash(): void + { + $this->client->request('GET', '/admin/resources'); + self::assertResponseStatusCodeSame(Response::HTTP_FORBIDDEN); + + $this->client->request('GET', '/'); + self::assertSelectorTextNotContains('#flashes', self::MESSAGE); + } +} diff --git a/tests/EventListener/HtmxExceptionListenerFunctionalTest.php b/tests/EventListener/HtmxExceptionListenerFunctionalTest.php deleted file mode 100644 index 40ade03..0000000 --- a/tests/EventListener/HtmxExceptionListenerFunctionalTest.php +++ /dev/null @@ -1,45 +0,0 @@ -catchExceptions(true); - $client->request('GET', '/does-not-exist', server: ['HTTP_HX-Request' => 'true']); - - $response = $client->getResponse(); - self::assertSame(Response::HTTP_OK, $response->getStatusCode()); - self::assertSame('body', $response->headers->get('HX-Retarget')); - self::assertSame('beforeend', $response->headers->get('HX-Reswap')); - self::assertStringContainsString('id="htmx-modal"', (string) $response->getContent()); - } - - public function testTheSameExceptionOnAnOrdinaryRequestStillGivesAnHonestErrorStatus(): void - { - $client = self::createClient(); - $client->catchExceptions(true); - $client->request('GET', '/does-not-exist'); - - // Asserting on headers rather than body: in debug mode Symfony's exception page renders - // the source of every stack frame, this file included, so any marker string searched for - // here would match its own source code. - self::assertSame(Response::HTTP_NOT_FOUND, $client->getResponse()->getStatusCode()); - self::assertFalse($client->getResponse()->headers->has('HX-Retarget')); - self::assertFalse($client->getResponse()->headers->has('HX-Reswap')); - } -} diff --git a/tests/EventListener/HtmxExceptionListenerTest.php b/tests/EventListener/HtmxExceptionListenerTest.php deleted file mode 100644 index 6e135cd..0000000 --- a/tests/EventListener/HtmxExceptionListenerTest.php +++ /dev/null @@ -1,94 +0,0 @@ -dispatch(['HTTP_HX-Request' => 'true']); - - $response = $event->getResponse(); - self::assertInstanceOf(Response::class, $response); - // 200, not 500: htmx does not swap 5xx, so an error response would be discarded unseen. - self::assertSame(Response::HTTP_OK, $response->getStatusCode()); - // Without this the kernel turns the 200 back into a 500. - self::assertTrue($event->isAllowingCustomResponseCode()); - } - - public function testTheModalIsRetargetedAtTheBodySoItAppearsWhateverTheTriggerWas(): void - { - $response = $this->dispatch(['HTTP_HX-Request' => 'true'])->getResponse(); - - self::assertNotNull($response); - self::assertSame('body', $response->headers->get('HX-Retarget')); - self::assertSame('beforeend', $response->headers->get('HX-Reswap')); - } - - public function testTheRenderedModalIsTheStandaloneModalFragment(): void - { - $content = (string) $this->dispatch(['HTTP_HX-Request' => 'true'])->getResponse()?->getContent(); - - self::assertStringContainsString('id="htmx-modal"', $content); - self::assertStringContainsString('data-controller="modal"', $content); - self::assertStringNotContainsString('dispatch()->getResponse()); - } - - public function testAResponseSetByAnEarlierListenerIsNotOverwritten(): void - { - $event = $this->event(['HTTP_HX-Request' => 'true']); - $event->setResponse($alreadySet = new Response('handled elsewhere')); - - ($this->listener())($event); - - self::assertSame($alreadySet, $event->getResponse()); - } - - /** - * @param array $server - */ - private function dispatch(array $server = []): ExceptionEvent - { - $event = $this->event($server); - ($this->listener())($event); - - return $event; - } - - /** - * @param array $server - */ - private function event(array $server): ExceptionEvent - { - return new ExceptionEvent( - self::bootKernel(), - Request::create('/bookings', server: $server), - HttpKernelInterface::MAIN_REQUEST, - new \RuntimeException('boom'), - ); - } - - private function listener(): HtmxExceptionListener - { - return new HtmxExceptionListener(self::getContainer()->get(Environment::class)); - } -} diff --git a/tests/Security/LoginEntryPointTest.php b/tests/Security/LoginEntryPointTest.php new file mode 100644 index 0000000..18e63ac --- /dev/null +++ b/tests/Security/LoginEntryPointTest.php @@ -0,0 +1,35 @@ +request('GET', '/book/desk', server: ['HTTP_HX-Request' => 'true']); + + self::assertSame('/login', $client->getResponse()->headers->get('HX-Redirect')); + } + + public function testAnExpiredSessionOnAnOrdinaryRequestStillRedirects(): void + { + $client = self::createClient(); + + $client->request('GET', '/book/desk'); + + self::assertResponseRedirects('/login'); + } +}