[#6320] - Added module event triggers
- Closed
- 15 Mar 2015
- Medium
- Build: staging
- Easy Test
- # 6320
- Diff
- nonumber:modules.events
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This PR adds 4 event triggers in the module rendering process.
This gives the ability to extensions to manipulate the module list and modules themselves.
Currently extensions that do this (like MetaMod, Advanced Module Manager, T3 framework) are forced to override the module helper class. Which causes compatibility issues.
By adding these extra events in core, it opens up flexibility and possibilities for everyone.
Module list creation:
onPrepareModuleList
$app->triggerEvent('onPrepareModuleList', array(&$modules));
This is triggered just before the module list is fetched from the database.
If a handler creates a module list itself (by setting the $modules variable to an array), then the core will not attempt to fetch modules itself (after this event).
onAfterModuleList
$app->triggerEvent('onAfterModuleList', array(&$modules));
This is triggered just after the module list is fetched from the database.
This can be used to manipulate the module list by adding/removing modules, or even changing the modules data.
onAfterCleanModuleList
$app->triggerEvent('onAfterCleanModuleList', array(&$modules));
After the previous onAfterModuleList trigger, the module list is cleaned by removing any duplicates.
The onAfterCleanModuleList event is triggered just after this cleaning. Again, so that the list can be manipulated before it is returned.
Module list creation:
onRenderModule
$app->triggerEvent('onRenderModule', array(&$module, &$attribs));
This is triggered near the end of the renderModule method.
It passes the $module(and$attribs) to the event handler. This makes it possible to manipulate the modules data. If the event handler nulls the$module``, it will return an empty string, instead of passing the module's content to the templates chrome.
Code syntax cleanup
This PR also moves some of the code in the module list method load to some separate methods to make the code clearer and cleaner.
Added methods are:
getModuleList which does the main database querying to get the module list
cleanModuleList which cleans the final module list (removing duplicates)
Testing
As an example for the onRenderModule you could do this in a plugin:
class plgSystemMyPlugin
{
public function onRenderModule(&$module)
{
// Module is nulled
if (is_null($module))
{
return;
}
// Just add some text before the module text
$module->content = '<p>TESTING</p>' . $module->content;
}
}
Votes
joomla-cms-bot
- | Labels |
Added:
?
|
||
It's an issue with one of the dependencies of pear/Cache_Lite (the dependency apparently released a new version today). See https://pear.php.net/bugs/bug.php?id=20511
You can also merge staging in and Travis will work right again. Made some tweaks to not use PEAR for any of the testing dependencies.
| Easy | No | ⇒ | Yes |
| Category | ⇒ | Modules Plugins |
All is well. Please test and/or give feedback...
Tested and it works.
When you create your module make sure to not return an array in the onPrepareModuleList method, but to actually set the $modules variable to array.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6320.
Tested and it works 
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6320.
Cool, thanks!
| Status | Pending | ⇒ | Ready to Commit |
RTC based on testing. Thanks!
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6320.
brianteeman
- | Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-03-15 08:52:23 |
Merged. Thanks!
Thank you :)
@Mathewlenning You are right, I would not merge it into the 3.4.x branch, it should have been merged in 3.5.0 because it introduces a new feature (behavior)!
Issues are created when 3rd party extensions overrule core functionality.
Other extensions rely on (new) core features. If the extensions that override the functionality do not support these features, stuff breaks. Which is the case now with this.
On the other hand, this PR is there to prevent this in the future, as the reason these 3rd party extensions are overriding the core module helper, is because of the missing triggers added through this PR.
@nonumber The problem isn't caused by 3rd party extensions. Because those extensions worked until this PR. The problem is that the core changed behavior in a patch release.
This PR might make it easier for 3PD to accomplish the same thing in the future, but that doesn't justify breaking their extensions without so much as a warning.
If we are SemVer then we must be SemVer, if we are not then we need to stop pretending we are.
@Mathewlenning and @Kubik-Rubik are right, this shouldn't have been merged into a patch release. It should have gone into the next minor (3.5.0). We're basically create new APIs here, which can only happen in minor releases, not patch releases.
The main question is if we want to revert it or not.
@nonumber_nl Is there a way to make this backwards compatible now, so it
can stay in? Like adding it as a new method/function/whatever instead of
overwriting existing.
Just thinking out load...
On Mar 24, 2015 2:51 PM, "Thomas Hunziker" notifications@github.com wrote:
@Mathewlenning https://github.com/Mathewlenning and @Kubik-Rubik
https://github.com/Kubik-Rubik are right, this shouldn't have been
merged into a patch release. It should have gone into the next minor
(3.5.0). We're basically create new APIs here, which can only happen in
minor releases, not patch releases.Reply to this email directly or view it on GitHub
#6320 (comment).
This doesn't break BC.
These are new triggers that you can use or not.
The issue here is that 3rd party extensions have decided to override the module helper. These are not updated to use these new triggers yet.
So if there are other extensions that do use these new triggers, they will fail, as these triggers are not present in the 3rd party overrides.
Solution: 3rd party devs need to update their code to stay in sync with core!
The issue isn't with B/C. That would be fine.
The real issue is that according to SemVer it wasn't allowed to go into a patch release. Since it introduces new features.
The two options at this point are break B/C by reverting the patch just to re-apply it for 3.5 or to carry on and add it to lessons learned.
Exactly 
If those are the options, I vote for lessons learned.
Sincerely,
Mathew Lenning
Babel-university.com
P.S. This message was sent via iPhone, so please forgive any errors
On Mar 24, 2015, at 11:57 PM, Michael Babker notifications@github.com wrote:
The two options at this point are break B/C by reverting the patch just to re-apply it for 3.5 or to carry on and add it to lessons learned.
\
Reply to this email directly or view it on GitHub.
| Labels |
Removed:
?
|
||
Looks like Travis is having issues...