Feature/login page fails on first login

.env

@@ -63,5 +63,5 @@ LOCK_DSN=flock
 # See https://github.com/itk-dev/tidy-feedback#all-configuration-options
 TIDY_FEEDBACK_DATABASE_URL=pdo-sqlite://localhost//app/var/tidy-feedback.sqlite
 TIDY_FEEDBACK_DISABLE=false
-TIDY_FEEDBACK_DISABLE_PATTERN=@^/tidy-feedback/@
+TIDY_FEEDBACK_DISABLE_PATTERN='@^/(login|tidy-feedback)@'
 ###< tidy-feedback ###

CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.4.0] - XXXX-XX-XX
+
+* [PR-48](https://github.com/itk-dev/itk-projects/pull/48)
+  Remove feedback from login page.
+* [PR-47](https://github.com/itk-dev/itk-projects/pull/47)
+  Keep the feedback widget working on pages reached through Turbo Drive
+  visits, not only on full page loads.
 * [PR-46](https://github.com/itk-dev/itk-projects/pull/46)
   * Remove the "Summary" field.
   * Ask "Er projektet godkendt" instead of "Er projektet vedtaget".
@@ -128,5 +135,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 [Unreleased]: https://github.com/itk-dev/itk-projects/compare/0.3.0...HEAD
 [0.3.0]: https://github.com/itk-dev/itk-projects/compare/0.2.0...0.3.0
-[0.2.0]: https://github.com/itk-dev/itk-projects/compare/0.1.09i876rbhn%3E%20gvfcdevgbhjgnybtfr%20v8decsxz...0.2.0
-[0.1.0]: <https://github.com/itk-dev/itk-projects/releases/tag/0.1.09i876rbhn> gvfcdevgbhjgnybtfr v8decsxz
+[0.2.0]: https://github.com/itk-dev/itk-projects/compare/0.1.0...0.2.0
+[0.1.0]: https://github.com/itk-dev/itk-projects/releases/tag/0.1.0

assets/app.js

@@ -279,3 +279,21 @@ document.addEventListener("turbo:before-cache", () => {
     });
     teardownPoolSelect();
 });
+
+// The Tidy feedback widget is injected server side as a declarative shadow root
+// with its script inside the template. Turbo parses the next page with
+// DOMParser, which builds neither, so the widget would only work on full page
+// loads. Marking its two elements permanent on both sides just before a render
+// makes Turbo carry the live, initialised nodes over instead of the inert copy.
+const TIDY_FEEDBACK_IDS = ["tidy-feedback", "tidy-feedback-region"];
+
+document.addEventListener("turbo:before-render", (event) => {
+    for (const id of TIDY_FEEDBACK_IDS) {
+        for (const root of [document, event.detail.newBody]) {
+            root.querySelector(`#${id}`)?.setAttribute(
+                "data-turbo-permanent",
+                "",
+            );
+        }
+    }
+});

config/packages/security.yaml

@@ -34,6 +34,10 @@ security:
     # (contacts, departments) is open to any signed-in user.
     - { path: ^/admin/users, roles: ROLE_ADMIN }
     - { path: ^/admin, roles: ROLE_USER }
+    # The feedback widget's stylesheet and script are static files. Behind the
+    # firewall they would be answered with a redirect to the login page, and a
+    # redirected GET saves itself as the post-login target.
+    - { path: ^/tidy-feedback/asset/, roles: PUBLIC_ACCESS }
     - { path: ^/, roles: ROLE_USER }
 
 when@test:

templates/dashboard/index.html.twig

@@ -8,7 +8,9 @@
             <a href="{{ path('app_project_new') }}" class="btn btn--primary">{{ 'action.new_project'|trans }}</a>
         </twig:Page:Header>
 
-        {{ turbo_stream_from('activities') }}
+        {# Kept out of the feedback widget's screenshot: snapdom clones the page,
+           and a cloned stream source connects without its src and throws. #}
+        <div data-capture="exclude" hidden>{{ turbo_stream_from('activities') }}</div>
         <script type="application/json" id="dashboard-viz-data" data-dashboard-viz-target="source">{{ include('dashboard/_viz_json.html.twig') }}</script>
 
         <div id="dashboard-content">

templates/security/login.html.twig

@@ -17,7 +17,9 @@
                 </div>
             {% endif %}
 
-            <form method="post">
+            {# A full page load after login: the feedback widget only initialises on
+               one, and Turbo carries it over to later visits from there. #}
+            <form method="post" data-turbo="false">
                 <div class="form-row">
                     <label for="username">{{ 'login.email'|trans }}</label>
                     <input type="email" id="username" name="_username" value="{{ last_username }}" autocomplete="email" required autofocus>

tests/Controller/TidyFeedbackTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller;
+
+use App\Tests\FunctionalTestCase;
+
+final class TidyFeedbackTest extends FunctionalTestCase
+{
+    public function testTheWidgetIsInjectedUnderTheIdsTheTurboHookReliesOn(): void
+    {
+        // assets/app.js marks these two elements permanent on each Turbo visit, so
+        // a renamed id in the bundle would silently bring the problem back.
+        $this->loginAsAdmin();
+        $crawler = $this->client->request('GET', '/');
+
+        $this->assertResponseIsSuccessful();
+        self::assertCount(1, $crawler->filter('#tidy-feedback'));
+        self::assertCount(1, $crawler->filter('#tidy-feedback-region'));
+    }
+
+    public function testTheMercureStreamSourceIsKeptOutOfTheScreenshot(): void
+    {
+        // The widget screenshots the page with snapdom, which clones the DOM; a
+        // cloned <turbo-mercure-stream-source> connects without its src and
+        // throws, so the dashboard keeps it inside an excluded wrapper.
+        $this->loginAsAdmin();
+        $crawler = $this->client->request('GET', '/');
+
+        self::assertCount(1, $crawler->filter('[data-capture="exclude"] turbo-mercure-stream-source[src]'));
+    }
+
+    public function testTheLoginPageCarriesNoWidget(): void
+    {
+        // TIDY_FEEDBACK_DISABLE_PATTERN keeps the widget off /login: its check
+        // endpoint and feedback form need a signed-in user anyway, and a redirected
+        // request from it would hijack the post-login redirect.
+        $crawler = $this->client->request('GET', '/login');
+
+        $this->assertResponseIsSuccessful();
+        self::assertCount(0, $crawler->filter('#tidy-feedback'));
+        // The widget only initialises on a full page load, so the first page after
+        // login must be one.
+        self::assertSame('false', $crawler->filter('form')->attr('data-turbo'));
+    }
+
+    public function testTheWidgetAssetsNeedNoSignIn(): void
+    {
+        $this->client->request('GET', '/tidy-feedback/asset/widget.css');
+
+        $this->assertResponseIsSuccessful();
+        self::assertStringStartsWith('text/css', (string) $this->client->getResponse()->headers->get('Content-Type'));
+    }
+
+    public function testAnAssetRequestBeforeLoginDoesNotHijackTheRedirect(): void
+    {
+        // Before the asset path was public, this request was answered with a
+        // redirect that stored itself as the target path, so the login below
+        // ended on the stylesheet instead of the dashboard.
+        $this->client->request('GET', '/tidy-feedback/asset/widget.css');
+
+        $crawler = $this->client->request('GET', '/login');
+        $this->client->request('POST', '/login', [
+            '_username' => 'admin@example.com',
+            '_password' => 'password',
+            '_csrf_token' => (string) $crawler->filter('input[name="_csrf_token"]')->attr('value'),
+        ]);
+
+        $this->assertResponseRedirects('/');
+    }
+}