@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?

@joomdonation

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

@joomdonation

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.

I have tested this item ✅ successfully on 0396a32

I have tested this item ✅ successfully on 0396a32

Hello @sahwikre, I have tested this successfully and was able to confirm that in Debug mode there is no longer a duplicate query for /modules/mod_tags_popular/src/Helper/TagsPopularHelper.php

I assume that this is distinct from the issue about Popular Tags module not reporting the correct number of tags.

RTC

✅ Final test before merge using JBT

• Before PR, created and placed 'Tags - Popular' site module
  • Seen 17 statements were executed, 2 of which were duplicates for rendering home, with SELECT ... FROM __contentitem_tag_map queries twice
• Applied PR with Patch Tester
• Only 16 statements were executed to render home, no duplicates anymore
• 'Tags - Popular' is shown w/o tags and 'Show "No results"' text configured
• Module is correct not shown without 'Show "No results"'
• Created three tags and one article with the tree tags, 'Tags - Popular' shows the tags and still home needs only 16 statements
• No unusual Joomla log messages

Thank you @sathwikre for your contribution. Thank you @joomdonation and @richard67 for supporting this PR. Thank you @Denitz and @exlemor for testing.