Skip to content

Improvements#62

Open
jonasdekeukelaere wants to merge 8 commits intomainfrom
improvements
Open

Improvements#62
jonasdekeukelaere wants to merge 8 commits intomainfrom
improvements

Conversation

@jonasdekeukelaere
Copy link
Member

@jonasdekeukelaere jonasdekeukelaere commented Mar 4, 2026

Summary by Sourcery

Refactor user management and authentication flows to use modal-based form submissions, simplify controllers, and tighten type safety and dependencies across the user module.

New Features:

  • Add modal confirmation dialogs for disabling, enabling, and resending confirmation emails for users in the admin edit screen.

Bug Fixes:

  • Prevent admins from disabling their own account from the user edit page.
  • Redirect users back to their profile after successfully changing their password instead of staying on the password form.

Enhancements:

  • Replace link-based admin actions for enable/disable/resend confirmation with POST-backed Symfony forms handled in the user edit controller.
  • Unify password reset email sending in the admin edit screen via a named form instead of a generic form type with bound DTO.
  • Mark controllers and message handlers as final/readonly where appropriate and align constructor dependencies with Symfony best practices.
  • Improve consistency of controller render calls, breadcrumb labels, and attribute usage for users and 2FA flows.
  • Clarify and extend Dutch translations for user management confirmation prompts and button labels.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 4, 2026

Reviewer's Guide

Refactors several user-related controllers and message handlers for consistency and immutability while centralizing admin user actions (enable/disable/resend confirmation/password reset) into the edit-user page using modal-backed Symfony forms instead of separate controllers and links.

Sequence diagram for admin disabling a user via edit page modal

sequenceDiagram
    actor Admin
    participant Browser
    participant EditUserController
    participant FormFactoryInterface as FormFactory
    participant MessageBusInterface as MessageBus
    participant DisableUserHandler
    participant UserRepository
    participant Database

    Admin->>Browser: Click Disable button (open modal)
    Admin->>Browser: Confirm disable in modal form
    Browser->>EditUserController: POST /admin/users/{user}/edit (disable_user form)

    EditUserController->>FormFactory: createNamed(disable_user)
    FormFactory-->>EditUserController: disableUserForm
    EditUserController->>EditUserController: handleRequest(request) on disableUserForm
    EditUserController->>EditUserController: disableUserForm isSubmitted() && isValid()
    EditUserController->>MessageBus: dispatch(DisableUser(userId))

    MessageBus-->>DisableUserHandler: Handle DisableUser
    DisableUserHandler->>UserRepository: find(userId)
    UserRepository->>Database: Load user
    Database-->>UserRepository: User entity
    DisableUserHandler->>UserRepository: save(disabled user)
    UserRepository->>Database: Persist changes

    DisableUserHandler-->>MessageBus: return
    EditUserController->>EditUserController: addFlash(success, User successfully disabled.)
    EditUserController-->>Browser: Redirect to /admin/users/{user}/edit
    Browser-->>Admin: Updated edit user page with flash message
Loading

Class diagram for updated EditUserController and related messages

classDiagram
    class EditUserController {
        - FormFactoryInterface formFactory
        - TranslatorInterface translator
        - MessageBusInterface messageBus
        + __construct(FormFactoryInterface formFactory, TranslatorInterface translator, MessageBusInterface messageBus)
        + __invoke(Request request, User currentUser, User user) Response
    }

    class User {
        + getId() int
        + getEmail() string
        + isEnabled() bool
        + isConfirmed() bool
    }

    class DisableUser {
        + __construct(int userId)
        + getUserId() int
    }

    class EnableUser {
        + __construct(int userId)
        + getUserId() int
    }

    class SendConfirmation {
        + __construct(int userId, string locale)
        + getUserId() int
        + getLocale() string
    }

    class SendPasswordReset {
        + email string
    }

    class DisableUserHandler {
        - UserRepository userRepository
        + __construct(UserRepository userRepository)
        + __invoke(DisableUser message) void
    }

    class EnableUserHandler {
        - UserRepository userRepository
        + __construct(UserRepository userRepository)
        + __invoke(EnableUser message) void
    }

    class SendConfirmationHandler {
        - MailerInterface mailer
        - TranslatorInterface translator
        - RouterInterface router
        - UserRepository userRepository
        - Address from
        + __construct(MailerInterface mailer, TranslatorInterface translator, RouterInterface router, UserRepository userRepository, string from)
        + __invoke(SendConfirmation message) void
    }

    class SendPasswordResetHandler {
        - MailerInterface mailer
        - TranslatorInterface translator
        - RouterInterface router
        - UserRepository userRepository
        - Address from
        + __construct(MailerInterface mailer, TranslatorInterface translator, RouterInterface router, UserRepository userRepository, string from)
        + __invoke(SendPasswordReset message) void
    }

    EditUserController --> User : edits
    EditUserController --> DisableUser : dispatches
    EditUserController --> EnableUser : dispatches
    EditUserController --> SendConfirmation : dispatches
    EditUserController --> SendPasswordReset : dispatches

    DisableUserHandler ..> DisableUser : handles
    EnableUserHandler ..> EnableUser : handles
    SendConfirmationHandler ..> SendConfirmation : handles
    SendPasswordResetHandler ..> SendPasswordReset : handles

    DisableUserHandler --> UserRepository
    EnableUserHandler --> UserRepository
    SendConfirmationHandler --> UserRepository
    SendPasswordResetHandler --> UserRepository
Loading

File-Level Changes

Change Details Files
Convert admin user enable/disable and resend-confirmation actions on the edit page from direct links and separate controllers into inline Symfony forms handled within EditUserController, surfaced via Bootstrap modals in the Twig template.
  • Replaces anchor-based actions for enable, disable, and resend confirmation with buttons that open Bootstrap modals on the admin user edit page.
  • Adds conditional modal markup and form_start/form_end blocks for disableUserForm, enableUserForm, and resendConfirmationForm in the edit.html.twig template, including confirmation messages and icons.
  • Extends EditUserController to inject FormFactoryInterface and CurrentUser, create and handle named forms for disabling/enabling users and resending confirmation, dispatching DisableUser, EnableUser, and SendConfirmation messages and flashing success messages.
  • Passes the three new form instances (or null) to the Twig view so the modal triggers are only rendered when applicable.
  • Removes now-redundant dedicated controllers for admin enable, disable, and request confirmation operations.
templates/user/admin/edit.html.twig
src/Controller/User/Admin/EditUserController.php
src/Controller/User/Admin/DisableUserController.php
src/Controller/User/Admin/EnableUserController.php
src/Controller/User/Admin/RequestConfirmationController.php
Standardize various controllers to be final, tidy constructor and render/redirect calls, and make small behavioral tweaks.
  • Marks multiple controllers (home, login, register, profile, password, forgot/reset password, confirm, 2FA, admin user controllers, and AJAX password strength) as final for immutability/extension control.
  • Adds trailing commas in constructor dependency lists and rewrites render/redirectToRoute calls into multiline, array-style formatting for readability.
  • Adjusts breadcrumb labels for add/edit user actions to use capitalized keys ('Add', 'Edit') and updates a PHPDoc array annotation in HomeController.
  • Changes PasswordController to redirect back to the user profile after a successful password change.
src/Controller/HomeController.php
src/Controller/User/LoginController.php
src/Controller/User/RegisterController.php
src/Controller/User/ProfileController.php
src/Controller/User/Profile/PasswordController.php
src/Controller/User/Profile/TwoFactorController.php
src/Controller/User/Profile/TwoFactorQrCodeController.php
src/Controller/User/ForgotPasswordController.php
src/Controller/User/ResetPasswordController.php
src/Controller/User/ConfirmController.php
src/Controller/User/Admin/AddUserController.php
src/Controller/User/Admin/EditUserController.php
src/Controller/User/Admin/OverviewController.php
src/Controller/User/Ajax/PasswordStrengthController.php
Adjust message handlers to be final readonly where appropriate and relax property readonly-ness for dependencies that need late initialization, while cleaning up imports.
  • Marks several user-related message handlers (register, change password, create user, reset password, confirm user, disable user, enable user, update user, send confirmation, send password reset) as final readonly classes for immutability semantics.
  • Updates constructors to drop readonly on properties that are mutated during construction (e.g. Address $from) and adjusts property visibility to match readonly-class rules.
  • Removes an unused import of ResetPasswordController from SendPasswordResetHandler.
src/MessageHandler/User/RegisterUserHandler.php
src/MessageHandler/User/ChangePasswordHandler.php
src/MessageHandler/User/CreateUserHandler.php
src/MessageHandler/User/ResetPasswordHandler.php
src/MessageHandler/User/ConfirmUserHandler.php
src/MessageHandler/User/DisableUserHandler.php
src/MessageHandler/User/EnableUserHandler.php
src/MessageHandler/User/UpdateUserHandler.php
src/MessageHandler/User/SendConfirmationHandler.php
src/MessageHandler/User/SendPasswordResetHandler.php
Update translations and minor template details to support the new confirmation modals and labels.
  • Adds Dutch translations for new confirmation questions (enable/disable user, resend confirmation) and the generic 'Send' label, and removes obsolete lowercase add/edit keys.
  • Removes an unused name attribute from the password reset modal submit button in the admin edit template.
translations/messages+intl-icu.nl.yaml
templates/user/admin/edit.html.twig

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • Changing the breadcrumb labels from 'add'/'edit' to 'Add'/'Edit' while also removing the lowercase add/edit translation keys may break existing translations; either restore the old keys or add matching Add/Edit entries to the translation files.
  • In EditUserController::__invoke, the #[CurrentUser] User $currentUser argument is non-nullable; if there is any chance this admin route can be hit without an authenticated user, consider making it ?User and handling the null case explicitly to avoid 500s.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Changing the breadcrumb labels from `'add'/'edit'` to `'Add'/'Edit'` while also removing the lowercase `add`/`edit` translation keys may break existing translations; either restore the old keys or add matching `Add`/`Edit` entries to the translation files.
- In `EditUserController::__invoke`, the `#[CurrentUser] User $currentUser` argument is non-nullable; if there is any chance this admin route can be hit without an authenticated user, consider making it `?User` and handling the null case explicitly to avoid 500s.

## Individual Comments

### Comment 1
<location path="templates/user/admin/edit.html.twig" line_range="53-58" />
<code_context>
+
+    {% if disableUserForm is defined and disableUserForm %}
+        {{ form_start(disableUserForm) }}
+        <div class="modal fade" id="disableUserModal" tabindex="-1" aria-labelledby="disableUserModalLabel"
+             aria-hidden="true">
+            <div class="modal-dialog">
+                <div class="modal-content">
+                    <div class="modal-header">
+                        <h1 class="modal-title fs-5" id="disableUserModal">{{ 'Disable'|trans }}</h1>
+                        <button type="button" class="btn-close" data-bs-dismiss="modal"
+                                aria-label="{{ 'dialogs.close'|trans }}"></button>
</code_context>
<issue_to_address>
**issue (bug_risk):** The `aria-labelledby` attribute references an ID that does not exist in the modal header.

Here, `aria-labelledby="disableUserModalLabel"` references a non-existent ID; the heading’s ID is `disableUserModal`. Please align these by either changing `aria-labelledby` to `disableUserModal` or renaming the `<h1>` ID to `disableUserModalLabel`, and apply the same fix to the enable and resend confirmation modals for consistency and valid ARIA.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants