[#46827] - [5.4] Fix duplicate getLayoutData execution in mod_tags_popular
- Pending
- Medium
- Build: 5.4-dev
- # 46827
- Diff
- sathwikre:fix-mod-tags-popular-getlayoutdata
User tests: Successful: Unsuccessful:
The popular tags module executes getLayoutData() twice due to an
overridden dispatch() method.
This change removes the custom dispatch() override and moves the
render-stop logic into getLayoutData(), preventing duplicate execution
and unnecessary queries.
Fixes #46816
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Repository Front End com_contact Modules |
| Labels |
Added:
PR-5.4-dev
|
||
| Category | Repository Front End com_contact Modules | ⇒ | Repository Modules Front End |
Thanks for checking.
You’re right — those changes were unrelated. I’ve cleaned up the branch so
this PR now only contains the fix for mod_tags_popular.
@sathwikre There is still an unrelated file build/build.php. You can check it here https://github.com/joomla/joomla-cms/pull/46827/changes
Thanks for pointing that out.
You’re right — the branch still contained unrelated changes due to an
accidental merge earlier. I’ve now reset the branch and reapplied only the
mod_tags_popular fix, so the PR contains a single relevant file change.
That's not right. The change to the file build/build.php is still there. You can check it yourself https://github.com/joomla/joomla-cms/pull/46827/changes
| Category | Repository Front End Modules | ⇒ | Modules Front End |
Thanks for your patience — you were right.
I had been resetting the branch against my fork instead of the upstream
joomla/5.4-dev branch. I’ve now reset against upstream and reapplied only
the mod_tags_popular fix.
The PR now contains a single relevant file change.
Yes, looks good now. Thanks !
@sathwikre When creating a pull request you should always check the result on GitHub, not only in your IDE. Depending on your editor settings, if it shows white space (spaces and tabs) or not, you might not see code style issues in the IDE, and also unwanted changes can be avoided when checking the PR on GitHub.
@sathwikre P.S.: And you should stick with our pull request template and not remove all these things from it. You PR is missing testing instructions. I am pretty sure I've already told you elsewhere in some other PR.
@joomdonation Is it right that the method modified by this PR now returns false in some case? It did not do that before. Technically it could even be considered a b/c break. It needs to check where that method is called to see if false is ok or if it should be null or an empty array. Maybe you can advise?
@richard67 The PR is fine. Return false will prevent the module from being rendered, see https://github.com/joomla/joomla-cms/blob/5.4-dev/libraries/src/Dispatcher/AbstractModuleDispatcher.php#L66
@richard67 The PR is fine. Return false will prevent the module from being rendered, see https://github.com/joomla/joomla-cms/blob/5.4-dev/libraries/src/Dispatcher/AbstractModuleDispatcher.php#L66
@joomdonation I see. Thanks for checking.
@sathwikre Your fix looks good. However, the change in the two files build/build.php and components/com_contact/tmpl/contact/default_form.php should not be included in this PR. Could you please check?