Skip to content

fix(manager): correct OnBeforeManagerPageInit / OnManagerPageInit order - #16937

Open
Ibochkarev wants to merge 1 commit into
modxcms:3.xfrom
Ibochkarev:fix/9461-manager-page-events-order
Open

fix(manager): correct OnBeforeManagerPageInit / OnManagerPageInit order#16937
Ibochkarev wants to merge 1 commit into
modxcms:3.xfrom
Ibochkarev:fix/9461-manager-page-events-order

Conversation

@Ibochkarev

Copy link
Copy Markdown
Collaborator

What does it do?

  • Fires manager page init events from modManagerController::render() via invokeManagerPageInitEvents(): OnBeforeManagerPageInit first, then OnManagerPageInit.
  • Removes the early OnManagerPageInit invocation from modManagerRequest::handleRequest() (it ran before OnBeforeManagerPageInit in render()).
  • Passes OnManagerPageInit a payload of controller config merged with the current request action and namespace (request values win when set).
  • Adds PHPUnit coverage for event order and merge behavior (ManagerPageInitEventsTest).

Backward-compatibility note: OnManagerPageInit now runs when the controller renders, not at the beginning of handleRequest(). Code paths that redirect or exit before render() may no longer trigger these two events; listeners that must run earlier should use another system event (e.g. OnHandleRequest).

Why is it needed?

Fixes the incorrect ordering described in the issue: OnBeforeManagerPageInit must run before OnManagerPageInit so plugins can prepare state before the main init event.

How to test

  • Run: phpunit -c _build/test/phpunit.xml --filter ManagerPageInitEventsTest
  • Manually: open the MODX manager and exercise pages that rely on plugins hooked to OnBeforeManagerPageInit / OnManagerPageInit; confirm expected order and that action / namespace in the event payload match the request.

Related issue(s)/PR(s)

Resolves #9461

@Ibochkarev
Ibochkarev marked this pull request as ready for review March 28, 2026 04:55
@smg6511

smg6511 commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

@opengeek - Was taking a look at this and wanted your stance on the potential BC implications and whether they would cause this change to be blocked anyway. Otherwise it of course makes sense for these events to run in the expected order, although I find the wrapping of them in a new method just to accomodate a test to be unnecessary.

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.

Wrong events order

2 participants