diff --git a/assets/css/components/modal-window.scss b/assets/css/components/modal-window.scss index 690375d9b..a1424eb4f 100644 --- a/assets/css/components/modal-window.scss +++ b/assets/css/components/modal-window.scss @@ -62,6 +62,10 @@ display: flex; justify-content: space-between; margin-top: calculateRem(45px); + + &--single { + justify-content: flex-end; + } } .modalWindowButton { diff --git a/assets/js/modalWindow.js b/assets/js/modalWindow.js index d285e958d..96878b6c5 100644 --- a/assets/js/modalWindow.js +++ b/assets/js/modalWindow.js @@ -16,6 +16,29 @@ window.addEventListener('load', function () { } } + // Modals rendered with `autoOpen: true` (see helpers/modal.html.twig) should show + // themselves immediately, e.g. to give feedback after a redirect back from another + // service. The query parameter that signalled the result is stripped from the URL + // first so a page refresh does not reopen the modal. This has to happen before the + // trigger is clicked: Firefox resolves the trigger's fragment link against the URL + // as it was at click time, and would otherwise reload the page with the original + // query string, over and over. + const autoOpenButtons = document.querySelectorAll('.modalWindowButton[data-auto-open="true"]'); + if (!!autoOpenButtons) { + for (let i = 0; i < autoOpenButtons.length; i++) { + const trigger = autoOpenButtons[i]; + + const cleanParam = trigger.getAttribute('data-clean-param'); + if (cleanParam) { + const url = new URL(window.location.href); + url.searchParams.delete(cleanParam); + window.history.replaceState(null, '', url.pathname + url.search + url.hash); + } + + trigger.click(); + } + } + const cancelButtons = document.querySelectorAll('.modalWindow__cancel'); if (!!cancelButtons) { for (let i = 0; i < cancelButtons.length; i++) { diff --git a/src/OpenConext/ProfileBundle/Controller/MyProfileController.php b/src/OpenConext/ProfileBundle/Controller/MyProfileController.php index 2ff1b3935..49ad9c5e4 100644 --- a/src/OpenConext/ProfileBundle/Controller/MyProfileController.php +++ b/src/OpenConext/ProfileBundle/Controller/MyProfileController.php @@ -22,6 +22,7 @@ use OpenConext\ProfileBundle\Service\UserService; use OpenConext\ProfileBundle\Service\WayfResetLinkBuilder; +use OpenConext\ProfileBundle\Service\WayfResetResultResolver; use Psr\Log\LoggerInterface; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -40,6 +41,7 @@ public function __construct( private readonly UserService $userService, private readonly LoggerInterface $logger, private readonly WayfResetLinkBuilder $wayfResetLinkBuilder, + private readonly WayfResetResultResolver $wayfResetResultResolver, private readonly UrlGeneratorInterface $urlGenerator, private readonly string $wayfResetUrl, private readonly CsrfTokenManagerInterface $csrfTokenManager, @@ -52,8 +54,9 @@ public function __construct( methods: ["GET"], schemes: "https", )] - public function overview(): Response - { + public function overview( + Request $request, + ): Response { $this->logger->info('Showing My Profile page'); $user = $this->userService->getUser(); @@ -62,6 +65,7 @@ public function overview(): Response '@OpenConextProfile/MyProfile/overview.html.twig', [ 'user' => $user, + 'wayfResetResult' => $this->wayfResetResultResolver->resolve($request), ], ); } diff --git a/src/OpenConext/ProfileBundle/Service/WayfResetResultResolver.php b/src/OpenConext/ProfileBundle/Service/WayfResetResultResolver.php new file mode 100644 index 000000000..c56398b4a --- /dev/null +++ b/src/OpenConext/ProfileBundle/Service/WayfResetResultResolver.php @@ -0,0 +1,46 @@ +query->get('wayfReset'); + + return in_array($result, self::KNOWN_RESULTS, true) ? $result : null; + } +} diff --git a/src/OpenConext/ProfileBundle/Tests/Controller/MyProfileControllerTest.php b/src/OpenConext/ProfileBundle/Tests/Controller/MyProfileControllerTest.php index 776a92bdb..c99ffaa84 100644 --- a/src/OpenConext/ProfileBundle/Tests/Controller/MyProfileControllerTest.php +++ b/src/OpenConext/ProfileBundle/Tests/Controller/MyProfileControllerTest.php @@ -32,6 +32,7 @@ use OpenConext\ProfileBundle\Service\SupportContactEmailService; use OpenConext\ProfileBundle\Service\UserService; use OpenConext\ProfileBundle\Service\WayfResetLinkBuilder; +use OpenConext\ProfileBundle\Service\WayfResetResultResolver; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; @@ -64,6 +65,7 @@ public function it_redirects_to_the_wayf_reset_url_when_the_csrf_token_is_valid( $this->createUserService(), $this->createMock(LoggerInterface::class), new WayfResetLinkBuilder(), + new WayfResetResultResolver(), $urlGenerator, 'https://engine.example.org/reset-remember-wayf', $csrfTokenManager, @@ -96,6 +98,7 @@ public function it_denies_access_when_the_csrf_token_is_invalid(): void $this->createUserService(), $this->createMock(LoggerInterface::class), new WayfResetLinkBuilder(), + new WayfResetResultResolver(), $urlGenerator, 'https://engine.example.org/reset-remember-wayf', $csrfTokenManager, diff --git a/src/OpenConext/ProfileBundle/Tests/Service/WayfResetResultResolverTest.php b/src/OpenConext/ProfileBundle/Tests/Service/WayfResetResultResolverTest.php new file mode 100644 index 000000000..dfa714f68 --- /dev/null +++ b/src/OpenConext/ProfileBundle/Tests/Service/WayfResetResultResolverTest.php @@ -0,0 +1,80 @@ +resolve(Request::create('/my-profile?wayfReset=removed')); + + $this->assertSame(WayfResetResultResolver::RESULT_REMOVED, $result); + } + + /** + * @test + * @group Service + */ + public function it_resolves_the_none_result(): void + { + $resolver = new WayfResetResultResolver(); + + $result = $resolver->resolve(Request::create('/my-profile?wayfReset=none')); + + $this->assertSame(WayfResetResultResolver::RESULT_NONE, $result); + } + + /** + * @test + * @group Service + */ + public function it_returns_null_when_the_query_parameter_is_missing(): void + { + $resolver = new WayfResetResultResolver(); + + $result = $resolver->resolve(Request::create('/my-profile')); + + $this->assertNull($result); + } + + /** + * @test + * @group Service + */ + public function it_returns_null_for_an_unrecognised_value(): void + { + $resolver = new WayfResetResultResolver(); + + $result = $resolver->resolve(Request::create('/my-profile?wayfReset=')); + + $this->assertNull($result); + } +} diff --git a/templates/MyProfile/overview.html.twig b/templates/MyProfile/overview.html.twig index 0bee4dff8..97b4ea59b 100644 --- a/templates/MyProfile/overview.html.twig +++ b/templates/MyProfile/overview.html.twig @@ -66,4 +66,34 @@ + {% if wayfResetResult is not empty %} + {% set resultModalContent %} +

+ {% if wayfResetResult == 'removed' %} + {{ 'profile.my_profile.wayf_reset.result_removed_message'|trans }} + {% else %} + {{ 'profile.my_profile.wayf_reset.result_none_message'|trans }} + {% endif %} +

+ {% endset %} + + {# Explicitly clear buttonRowContent: `include` without `only` inherits the + surrounding scope, so without this the form set for the modal above would + leak into this single-button modal. #} + {% set buttonRowContent = '' %} + {% include "@OpenConextProfile/helpers/modal.html.twig" with { + buttonText: 'profile.my_profile.wayf_reset.link_text'|trans, + linkId: 'wayf_reset_result', + buttonClass: 'modalWindowButton visually-hidden', + describedBy: 'wayf-reset-result-description', + modalHeader: wayfResetResult == 'removed' + ? 'profile.my_profile.wayf_reset.result_removed_title'|trans + : 'profile.my_profile.wayf_reset.result_none_title'|trans, + modalContent: resultModalContent, + cancelButtonText: 'general.ok'|trans, + autoOpen: true, + cleanParam: 'wayfReset', + } %} + {% endif %} + {% endblock %} diff --git a/templates/helpers/modal.html.twig b/templates/helpers/modal.html.twig index f4abb7749..fab0f985d 100644 --- a/templates/helpers/modal.html.twig +++ b/templates/helpers/modal.html.twig @@ -2,7 +2,10 @@ {% if linkId is not defined %} {% set linkId = linkIdPrefix ~ loop.index %} {% endif %} -{% set headerId = 'dialog-header-' ~ loop.index %} +{# Derive headerId from linkId (always unique) rather than loop.index, so that + multiple modal instances rendered outside of a real iteration (e.g. two + independent modals on the same page) don't end up with duplicate ids. #} +{% set headerId = 'dialog-header-' ~ linkId %} {{ buttonText }} @@ -29,8 +34,8 @@ >{{ modalHeader }}