Skip to content

[5.4] Add missing page parameter to contentEventArguments - Article Module#47476

Open
LadySolveig wants to merge 1 commit intojoomla:5.4-devfrom
LadySolveig:5.4/ref/missing-page-article-module
Open

[5.4] Add missing page parameter to contentEventArguments - Article Module#47476
LadySolveig wants to merge 1 commit intojoomla:5.4-devfrom
LadySolveig:5.4/ref/missing-page-article-module

Conversation

@LadySolveig
Copy link
Copy Markdown
Contributor

Pull Request resolves # .

  • I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.

Summary of Changes

The AI took the fun approach here #47467 when generating the test plugin by pointing out that the page argument is missing, resulting in inconsistent test results for the legacy modules compared to the new article module.

grafik
if ($event->getPage() !== null) {
    return;
}

This would work at the moment in the new Article Module, but nowhere else as all others have an offset or 0 default.
As it is more consistent with the rest of the codebase to include the page argument and give it value 0 instead of null as we do everywhere else, here is the PR for that.

Testing Instructions

Code Review

Actual result BEFORE applying this Pull Request

Unconsistent

Expected result AFTER applying this Pull Request

Consistent

Link to documentations

Please select:

  • Documentation link for guide.joomla.org:

  • No documentation changes for guide.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@ramalama
Copy link
Copy Markdown

I have tested this item ✅ successfully on a7fe11e

Test via Code Review successful


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/47476.

@richard67 richard67 added the bug label Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants