fix: properly forward return urls where missing

src/Controller/Admin/Application/DeleteController.php

@@ -39,6 +39,7 @@ class DeleteController extends AbstractController
 
             return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $application->getAssignment()->getUuid(),
+                'r' => $request->query->get('r'),
             ]));
         }
 

src/Controller/Admin/Application/DisposeController.php

@@ -46,6 +46,7 @@ class DisposeController extends AbstractController
 
             $redirectUrl = $this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $application->getAssignment()->getUuid(),
+                'r' => $request->query->get('r'),
             ]);
 
             return new HxRedirectResponse($redirectUrl);

src/Controller/Admin/Application/StatusController.php

@@ -57,6 +57,7 @@ class StatusController extends AbstractController
 
             return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $application->getAssignment()->getUuid(),
+                'r' => $request->query->get('r'),
             ]));
         }
 

src/Controller/Admin/Disposition/DeleteController.php

@@ -53,6 +53,7 @@ class DeleteController extends AbstractController
 
             return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $disposition->getAssignment()->getUuid(),
+                'r' => $request->query->get('r'),
             ]));
         }
 

src/Controller/Admin/Teamer/SkillsController.php

@@ -44,6 +44,7 @@ class SkillsController extends AbstractController
 
             return $this->redirectToRoute('app_admin_teamer_skills', [
                 'uuid' => $teamer->getUuid(),
+                'r' => $request->query->get('r'),
             ]);
         }
 

src/Controller/Administrative/Assignment/DuplicateController.php

@@ -55,6 +55,7 @@ class DuplicateController extends AbstractController
 
             return $this->redirectToRoute('app_administrative_assignment_edit', [
                 'uuid' => $copy->getUuid(),
+                'r' => $request->query->get('r'),
             ]);
         }
 

src/Controller/Administrative/Disposition/BusProNetSyncToggleController.php

@@ -38,6 +38,7 @@ class BusProNetSyncToggleController extends AbstractController
 
         return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
             'uuid' => $disposition->getAssignment()->getUuid(),
+            'r' => $request->query->get('r'),
         ]));
     }
 }

src/Controller/Administrative/Disposition/CallOffController.php

@@ -55,6 +55,7 @@ class CallOffController extends AbstractController
 
             return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $assignment->getUuid(),
+                'r' => $request->query->get('r'),
             ]));
         }
 

src/Controller/Administrative/Disposition/DocumentUploadController.php

@@ -61,6 +61,7 @@ class DocumentUploadController extends AbstractController
 
             return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $disposition->getAssignment()->getUuid(),
+                'r' => $request->query->get('r'),
             ]));
         }
 

src/Controller/Administrative/Disposition/SpecialAgreementsController.php

@@ -38,6 +38,7 @@ class SpecialAgreementsController extends AbstractController
 
             return new HxRedirectResponse($this->generateUrl('app_administrative_assignment_detail', [
                 'uuid' => $disposition->getAssignment()->getUuid(),
+                'r' => $request->query->get('r'),
             ]));
         }
 

src/Controller/Administrative/Teamer/InfoController.php

@@ -24,7 +24,13 @@ class InfoController extends AbstractController
     #[IsGranted(new Expression('is_granted("ROLE_ADMINISTRATIVE") or is_granted("ROLE_HOUSE_MANAGER")'))]
     public function index(Teamer $teamer, Request $request): Response
     {
-        $returnUrl = $this->getReturnUrl($request, 'app_administrative_assignment_index');
+        // House managers reach this modal but not the administrative overviews,
+        // so their fallback has to stay inside their own area.
+        $defaultReturnUrlRoute = $this->isGranted('ROLE_ADMINISTRATIVE')
+            ? 'app_administrative_assignment_index'
+            : 'app_house_manager_index';
+
+        $returnUrl = $this->getReturnUrl($request, $defaultReturnUrlRoute);
 
         $recentDispositions = $this->dispositionRepository->findRecentDispositionsByTeamer($teamer);
 

src/Controller/Teamer/Disposition/DetailController.php

@@ -66,6 +66,7 @@ class DetailController extends AbstractController
 
             return $this->redirectToRoute('app_teamer_disposition_detail', [
                 'uuid' => $disposition->getUuid(),
+                'r' => $request->query->get('r'),
             ]);
         }
 

src/Controller/Traits/ReturnUrlTrait.php

@@ -8,9 +8,15 @@ trait ReturnUrlTrait
 {
     public function getReturnUrl(Request $request, string $defaultRoute, array $parameters = []): string
     {
-        $defaultUrl = $this->generateUrl($defaultRoute, $parameters);
+        $returnUrl = $request->query->get('r');
 
-        return rawurldecode($request->query->get('r', $defaultUrl));
+        // An empty "r" reaches us whenever a link forwards a return url it never got
+        // itself, it must not be mistaken for a return url pointing at the root.
+        if (false === is_string($returnUrl) || '' === $returnUrl) {
+            return $this->generateUrl($defaultRoute, $parameters);
+        }
+
+        return rawurldecode($returnUrl);
     }
 
     /**

src/Twig/AppExtension.php

@@ -32,6 +32,7 @@ class AppExtension extends AbstractExtension
             new TwigFunction('icon', [AppRuntime::class, 'renderIcon'], ['needs_environment' => true, 'is_safe' => ['html']]),
             new TwigFunction('is_current_route', [AppRuntime::class, 'isCurrentRoute']),
             new TwigFunction('return_url', [AppRuntime::class, 'getEncodedReturnUrl']),
+            new TwigFunction('forwarded_return_url', [AppRuntime::class, 'getForwardedReturnUrl']),
             new TwigFunction('qa_attribute', [AppRuntime::class, 'renderQaAttribute'], ['is_safe' => ['html']]),
             new TwigFunction('has_pickup', [AppRuntime::class, 'hasPickup']),
         ];

src/Twig/AppRuntime.php

@@ -192,6 +192,21 @@ class AppRuntime implements RuntimeExtensionInterface
         return rawurlencode($masterRequest->getRequestUri());
     }
 
+    /**
+     * Passes the return url of the current page on to the next one.
+     *
+     * Used by links that lead deeper into a detail view or trigger an action on it:
+     * they have to keep pointing back at where the user originally came from instead
+     * of at the page they sit on. Null when there is nothing to pass on, so that the
+     * url generator drops the parameter rather than emitting an empty one.
+     */
+    public function getForwardedReturnUrl(): ?string
+    {
+        $returnUrl = $this->requestStack->getMainRequest()?->query->get('r');
+
+        return is_string($returnUrl) && '' !== $returnUrl ? $returnUrl : null;
+    }
+
     public function renderQaAttribute(string $label, ?string $value = null): string
     {
         if ('test' !== $this->environment) {

templates/admin/application/index.html.twig

@@ -107,7 +107,7 @@
                                          role="button"
                                          title="Bewerber:innen-Info anzeigen"
                                          aria-label="Bewerber:innen-Info anzeigen"
-                                         hx-get="{{ path('app_administrative_teamer_info', { 'uuid': application.teamer.uuid }) }}"
+                                         hx-get="{{ path('app_administrative_teamer_info', { 'uuid': application.teamer.uuid, 'r': return_url() }) }}"
                                          hx-target="body"
                                          hx-swap="beforeend">
                                         {% set requirementsMet = true %}

templates/admin/index.html.twig

@@ -41,7 +41,7 @@
                 {% for disposition in newDispositions %}
                     {% set assignment = disposition.assignment %}
                     <li class="py-2 first:pt-0 last:pb-0 overflow-x-hidden">
-                        <a href="{{ path('app_administrative_assignment_detail', { 'uuid': assignment.uuid }) }}" class="flex items-center space-x-1">
+                        <a href="{{ path('app_administrative_assignment_detail', { 'uuid': assignment.uuid, 'r': return_url() }) }}" class="flex items-center space-x-1">
                             {{ icon('hand', 'w-4 h-4 shrink-0') }}
                             {% include '_partials/_disposition_data.html.twig' with { 'disposition': disposition } %}
                         </a>

templates/administrative/assignment/detail.html.twig

@@ -73,7 +73,8 @@
                                                     class="text-green-500"
                                                     title="einteilen"
                                                     hx-get="{{ path('app_admin_application_dispose', {
-                                                        'uuid': application.uuid
+                                                        'uuid': application.uuid,
+                                                        'r': forwarded_return_url()
                                                     }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
@@ -85,7 +86,8 @@
                                                     class="text-primary"
                                                     title="Bewerbungsstatus bearbeiten"
                                                     hx-get="{{ path('app_admin_application_status', {
-                                                        'uuid': application.uuid
+                                                        'uuid': application.uuid,
+                                                        'r': forwarded_return_url()
                                                     }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
@@ -97,7 +99,8 @@
                                                     class="text-red-500"
                                                     title="Bewerbung löschen"
                                                     hx-get="{{ path('app_admin_application_delete', {
-                                                        'uuid': application.uuid
+                                                        'uuid': application.uuid,
+                                                        'r': forwarded_return_url()
                                                     }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
@@ -152,7 +155,7 @@
                                 <button type="button"
                                         class="flex items-center space-x-1"
                                         title="Teamer:inneninfo {{ disposition.teamer }}"
-                                        hx-get="{{ path('app_administrative_teamer_info', { 'uuid': disposition.teamer.uuid }) }}"
+                                        hx-get="{{ path('app_administrative_teamer_info', { 'uuid': disposition.teamer.uuid, 'r': return_url() }) }}"
                                         hx-target="body"
                                         hx-swap="beforeend">
                                     <span>{{ disposition.teamer }}</span>
@@ -183,7 +186,7 @@
                             </td>
                             <td>
                                 <div class="flex items-center justify-end space-x-2">
-                                    <button type="button" hx-post="{{ path('app_administrative_disposition_bpn_sync_toggle', { 'uuid': disposition.uuid }) }}" hx-swap="none">
+                                    <button type="button" hx-post="{{ path('app_administrative_disposition_bpn_sync_toggle', { 'uuid': disposition.uuid, 'r': forwarded_return_url() }) }}" hx-swap="none">
                                         {{ icon('chair', html_classes('w-5 h-5', {'text-green-500': disposition.syncedWithBusProNet })) }}
                                     </button>
                                     <twig:DropDown>
@@ -191,7 +194,7 @@
                                                 class="text-gray-700 hover:text-primary block px-4 py-2 text-sm w-full text-left"
                                                 role="menuitem"
                                                 tabindex="-1"
-                                                hx-get="{{ path('app_administrative_disposition_special_agreements', { 'uuid': disposition.uuid }) }}"
+                                                hx-get="{{ path('app_administrative_disposition_special_agreements', { 'uuid': disposition.uuid, 'r': forwarded_return_url() }) }}"
                                                 hx-target="body"
                                                 hx-swap="beforeend">
                                             zusätzliche Absprachen
@@ -201,7 +204,7 @@
                                                     class="text-gray-700 hover:text-primary block px-4 py-2 text-sm w-full text-left"
                                                     role="menuitem"
                                                     tabindex="-1"
-                                                    hx-get="{{ path('app_administrative_disposition_document_upload', { 'uuid': disposition.uuid, 'type': 'contract' }) }}"
+                                                    hx-get="{{ path('app_administrative_disposition_document_upload', { 'uuid': disposition.uuid, 'type': 'contract', 'r': forwarded_return_url() }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
                                                 Honorarvertrag hochladen
@@ -210,7 +213,7 @@
                                                     class="text-gray-700 hover:text-primary block px-4 py-2 text-sm w-full text-left"
                                                     role="menuitem"
                                                     tabindex="-1"
-                                                    hx-get="{{ path('app_administrative_disposition_document_upload', { 'uuid': disposition.uuid, 'type': 'invoice' }) }}"
+                                                    hx-get="{{ path('app_administrative_disposition_document_upload', { 'uuid': disposition.uuid, 'type': 'invoice', 'r': forwarded_return_url() }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
                                                 Honorarnote hochladen
@@ -221,7 +224,7 @@
                                                     class="text-red-500 hover:text-primary block px-4 py-2 text-sm w-full text-left"
                                                     role="menuitem"
                                                     tabindex="-1"
-                                                    hx-get="{{ path('app_administrative_disposition_call_off', { 'uuid': disposition.uuid }) }}"
+                                                    hx-get="{{ path('app_administrative_disposition_call_off', { 'uuid': disposition.uuid, 'r': forwarded_return_url() }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
                                                 absagen
@@ -232,7 +235,7 @@
                                                     class="text-red-500 hover:text-primary block px-4 py-2 text-sm w-full text-left"
                                                     role="menuitem"
                                                     tabindex="-1"
-                                                    hx-get="{{ path('app_admin_disposition_delete', { 'uuid': disposition.uuid }) }}"
+                                                    hx-get="{{ path('app_admin_disposition_delete', { 'uuid': disposition.uuid, 'r': forwarded_return_url() }) }}"
                                                     hx-target="body"
                                                     hx-swap="beforeend">
                                                 löschen

templates/administrative/assignment/index.html.twig

@@ -146,7 +146,7 @@
                                     <div role="button"
                                          title="Teamer:innen-Info anzeigen"
                                          aria-label="Teamer:innen-Info anzeigen"
-                                         hx-get="{{ path('app_administrative_teamer_info', { 'uuid': disposition.teamer.uuid }) }}"
+                                         hx-get="{{ path('app_administrative_teamer_info', { 'uuid': disposition.teamer.uuid, 'r': return_url() }) }}"
                                          hx-target="body"
                                          hx-swap="beforeend">
                                         <span class="whitespace-nowrap">{{ disposition.teamer }}</span>
@@ -197,7 +197,7 @@
                                             class="block px-4 py-2 text-sm w-full text-left"
                                             role="menuitem"
                                             tabindex="-1"
-                                            hx-post="{{ path('app_administrative_assignment_publish', { 'uuid': assignment.uuid }) }}">
+                                            hx-post="{{ path('app_administrative_assignment_publish', { 'uuid': assignment.uuid, 'r': return_url() }) }}">
                                         veröffentlichen
                                     </button>
                                 {% endif %}
@@ -248,7 +248,7 @@
         </div>
     </div>
 
-    <a href="{{ path('app_administrative_assignment_create') }}" class="btn">
+    <a href="{{ path('app_administrative_assignment_create', { 'r': return_url() }) }}" class="btn">
         Neu
     </a>
 {% endblock %}

templates/administrative/assignment/timeline.html.twig

@@ -23,7 +23,7 @@
                 <button type="button"
                         class="flex items-center space-x-2"
                         title="Teamer:inneninfo {{ item.teamerName }}"
-                        hx-get="{{ path('app_administrative_teamer_info', { 'uuid': item.teamerUuid }) }}"
+                        hx-get="{{ path('app_administrative_teamer_info', { 'uuid': item.teamerUuid, 'r': return_url() }) }}"
                         hx-target="body"
                         hx-swap="beforeend">
                     <span class="text-xs font-semibold">{{ item.teamerName | raw }}</span>

templates/administrative/document/index.html.twig

@@ -101,7 +101,7 @@
                                     {% endif %}
                                     {% if is_granted('CHECK', upload) %}
                                         <button type="button"
-                                                hx-get="{{ path('app_administrative_document_check', { 'uuid': upload.uuid }) }}"
+                                                hx-get="{{ path('app_administrative_document_check', { 'uuid': upload.uuid, 'r': return_url() }) }}"
                                                 hx-target="body"
                                                 hx-swap="beforeend">
                                             {{ icon('edit') }}

templates/administrative/teamer/modal_info.html.twig

@@ -1,7 +1,13 @@
 {% extends 'htmx_modal.html.twig' %}
 
 {% block title %}
-    Teamer:inneninfo <a href="{{ path('app_administrative_teamer_profile', { 'uuid': teamer.uuid, 'r': returnUrl }) }}">{{ teamer }}</a>
+    {# The profile is administrative only, house managers reach this modal but not it. #}
+    Teamer:inneninfo
+    {% if is_granted('ROLE_ADMINISTRATIVE') %}
+        <a href="{{ path('app_administrative_teamer_profile', { 'uuid': teamer.uuid, 'r': returnUrl }) }}">{{ teamer }}</a>
+    {% else %}
+        {{ teamer }}
+    {% endif %}
 {% endblock %}
 
 {% block content %}

templates/house_manager/index.html.twig

@@ -6,7 +6,7 @@
     <button type="button"
             class="flex items-center space-x-1"
             title="Teamer:inneninfo {{ disposition.teamer }}"
-            hx-get="{{ path('app_administrative_teamer_info', { 'uuid': disposition.teamer.uuid }) }}"
+            hx-get="{{ path('app_administrative_teamer_info', { 'uuid': disposition.teamer.uuid, 'r': return_url() }) }}"
             hx-target="body"
             hx-swap="beforeend">
         {{ icon('info', 'w-4 h-4') }}

templates/manager/index.html.twig

@@ -75,7 +75,7 @@
                 {% for disposition in dispositions %}
                     {% set assignment = disposition.assignment %}
                     <li class="py-2 first:pt-0 last:pb-0 overflow-x-hidden">
-                        <a href="{{ path('app_administrative_assignment_detail', { 'uuid': assignment.uuid }) }}"
+                        <a href="{{ path('app_administrative_assignment_detail', { 'uuid': assignment.uuid, 'r': return_url() }) }}"
                            class="flex items-center space-x-1">
                             {{ icon('hand', 'w-4 h-4 shrink-0') }}
                             {% include '_partials/_disposition_data.html.twig' with { 'disposition': disposition } %}

templates/teamer/assignment/detail.html.twig

@@ -34,7 +34,7 @@
                         <button type="button"
                                 class="btn btn--secondary"
                                 title="Bewerbung zurückziehen"
-                                hx-get="{{ path('app_teamer_application_withdraw', { 'uuid': application.uuid }) }}"
+                                hx-get="{{ path('app_teamer_application_withdraw', { 'uuid': application.uuid, 'r': forwarded_return_url() }) }}"
                                 hx-target="body"
                                 hx-swap="beforeend">
                             Bewerbung zurückziehen

tests/Controller/Traits/ReturnUrlTraitTest.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller\Traits;
+
+use App\Controller\Traits\ReturnUrlTrait;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+
+class ReturnUrlTraitTest extends TestCase
+{
+    public const DEFAULT_ROUTE = 'app_administrative_teamer_index';
+    public const DEFAULT_URL = '/administrative/teamer';
+
+    private object $controller;
+
+    protected function setUp(): void
+    {
+        $this->controller = new class {
+            use ReturnUrlTrait;
+
+            /**
+             * @param array<string, mixed> $parameters
+             */
+            public function generateUrl(string $route, array $parameters = []): string
+            {
+                return ReturnUrlTraitTest::DEFAULT_URL.([] === $parameters ? '' : '?'.http_build_query($parameters));
+            }
+        };
+    }
+
+    public function testTheReturnUrlIsDecoded(): void
+    {
+        $request = Request::create('/somewhere?r='.rawurlencode('/administrative/teamer?page=3&sort=name'));
+
+        self::assertSame(
+            '/administrative/teamer?page=3&sort=name',
+            $this->controller->getReturnUrl($request, self::DEFAULT_ROUTE)
+        );
+    }
+
+    public function testAMissingParameterFallsBackToTheDefaultRoute(): void
+    {
+        $request = Request::create('/somewhere');
+
+        self::assertSame(self::DEFAULT_URL, $this->controller->getReturnUrl($request, self::DEFAULT_ROUTE));
+    }
+
+    /**
+     * A link forwarding a return url it never received itself emits "r=", which must
+     * not be read as a return url pointing at the root.
+     */
+    public function testAnEmptyParameterFallsBackToTheDefaultRoute(): void
+    {
+        $request = Request::create('/somewhere?r=');
+
+        self::assertSame(self::DEFAULT_URL, $this->controller->getReturnUrl($request, self::DEFAULT_ROUTE));
+    }
+
+    public function testTheDefaultRouteKeepsItsParameters(): void
+    {
+        $request = Request::create('/somewhere');
+
+        self::assertSame(
+            self::DEFAULT_URL.'?uuid=abc',
+            $this->controller->getReturnUrl($request, self::DEFAULT_ROUTE, ['uuid' => 'abc'])
+        );
+    }
+}

tests/Twig/ForwardedReturnUrlTest.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Twig;
+
+use App\Twig\AppRuntime;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;
+
+class ForwardedReturnUrlTest extends KernelTestCase
+{
+    private RequestStack $requestStack;
+    private AppRuntime $appRuntime;
+
+    protected function setUp(): void
+    {
+        self::bootKernel();
+
+        $this->requestStack = self::getContainer()->get(RequestStack::class);
+        $this->appRuntime = self::getContainer()->get(AppRuntime::class);
+    }
+
+    public function testTheReturnUrlIsPassedOnUntouched(): void
+    {
+        $encoded = rawurlencode('/administrative/assignment?page=3');
+        $this->requestStack->push(Request::create('/administrative/assignment/detail/abc?r='.$encoded));
+
+        // The raw query value, handed straight back to the url generator, which
+        // re-encodes it. No decoding of its own, that stays the consumer's job.
+        self::assertSame('/administrative/assignment?page=3', $this->appRuntime->getForwardedReturnUrl());
+    }
+
+    /**
+     * Null rather than an empty string, so the url generator drops the parameter
+     * instead of emitting "r=".
+     */
+    public function testNothingIsPassedOnWithoutAReturnUrl(): void
+    {
+        $this->requestStack->push(Request::create('/administrative/assignment/detail/abc'));
+
+        self::assertNull($this->appRuntime->getForwardedReturnUrl());
+    }
+
+    public function testNothingIsPassedOnForAnEmptyReturnUrl(): void
+    {
+        $this->requestStack->push(Request::create('/administrative/assignment/detail/abc?r='));
+
+        self::assertNull($this->appRuntime->getForwardedReturnUrl());
+    }
+}