? ? ? Pending

User tests: Successful: Unsuccessful:

avatar mbabker
mbabker
3 Feb 2018

Summary of Changes

At the moment there is really no way to know what events a plugin is subscribed to without reading the code. The 3.x and earlier plugin convention arbitrarily assumes that all public methods in a plugin class are potentially event listeners and are treated as such. With the Framework's dispatcher in use in 4.0 and the beginning of the transition toward plugins which use a "configuration over convention" style approach, we can start finding this information and use it as practical.

So, for 4.0, for plugins which implement Joomla\Event\SubscriberInterface we can add a new tab to the plugin's edit page showing the list of events that are subscribed to. This PR accomplishes just that.

Testing Instructions

With the patch applied, go to the Plugin Manager and edit the "Quick Icon - Joomla Update Notification" plugin. You should see a new tab on this page as the plugin has been updated to implement the above referenced interface. When editing any other plugin, the tab won't be there as no other plugin has been updated.

Expected result

screen shot 2018-02-05 at 7 40 24 pm

Documentation Changes Required

N/A, plugin manager practices aren't documented anywhere.

avatar mbabker mbabker - open - 3 Feb 2018
avatar mbabker mbabker - change - 3 Feb 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 3 Feb 2018
Category Administration com_plugins Language & Strings Libraries
avatar Fedik
Fedik - comment - 4 Feb 2018

hm, I tried to test and got (when open "Quick Icon - Joomla Update Notification" ):

Notice: Trying to get property of non-object in <root>/libraries/src/Plugin/PluginHelper.php on line 130
Notice: Trying to get property of non-object in <root>/libraries/src/Plugin/PluginHelper.php on line 130
Warning: class_implements(): Class Plg does not exist and could not be loaded in <root>/administrator/components/com_plugins/Model/PluginModel.php on line 391
Warning: in_array() expects parameter 2 to be array, boolean given in <root>/administrator/components/com_plugins/Model/PluginModel.php on line 391

or I missed something?

avatar Fedik
Fedik - comment - 4 Feb 2018

okay, I found
@mbabker the error comes when the plugin is unpublished,
for published plugin all works fine

avatar laoneo
laoneo - comment - 5 Feb 2018

Like that, but for the end user we probably need so show some more meaningful text than onGetIcons.

avatar mbabker
mbabker - comment - 5 Feb 2018

The best you can do I think is link to a documentation page for core events. Otherwise you get to a point where you start trying to create language strings for event names that turn into full paragraph descriptions. Not to mention that we would probably need to arbitrarily load a language file from somewhere to ensure event names have meaningful text as extensions can (and do) implement their own events.

avatar brianteeman
brianteeman - comment - 5 Feb 2018

I know that subscribe is the correct technical word it will confuse non technical users especially if they have some plugins that are for user subscription type stuff. The best I can come up with right now is to rename the tab to be advanced or developer information

avatar laoneo
laoneo - comment - 5 Feb 2018

At least let the eventname run trough JText, so extension devs have then the possibility to add some explanation for it.

avatar mbabker
mbabker - comment - 5 Feb 2018

At least let the eventname run trough JText

What's the key prefix? If the string's not part of the globals or the plugin's language file, where are we loading this from? And do we really want to introduce 60+ language strings that are essentially JGLOBAL_EVENT_NAME_ONGETICONS? This is in part why I think we're better off hardcoding an alert div into the layout with a link to the docs, which at least covers the core events.

avatar brianteeman
brianteeman - comment - 5 Feb 2018

Agree with Michael

avatar mbabker mbabker - change - 6 Feb 2018
Labels Added: ? ?
avatar mbabker
mbabker - comment - 6 Feb 2018

The issues with disabled plugins are addressed now.

avatar mbabker mbabker - change - 6 Feb 2018
The description was changed
avatar mbabker mbabker - edited - 6 Feb 2018
avatar mbabker
mbabker - comment - 6 Feb 2018

As I hinted at, I've added a link to the docs wiki page describing the core plugin events to help give more useful information without turning this tab into a mini-docs page. Screenshot in original post updated.

avatar Fedik
Fedik - comment - 7 Feb 2018

I have tested this item successfully on 95e1e46


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

avatar Fedik Fedik - test_item - 7 Feb 2018 - Tested successfully
avatar Anu1601CS
Anu1601CS - comment - 10 Feb 2018

I have tested this item successfully on 95e1e46


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

avatar Anu1601CS Anu1601CS - test_item - 10 Feb 2018 - Tested successfully
avatar franz-wohlkoenig franz-wohlkoenig - change - 10 Feb 2018
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 10 Feb 2018

Ready to Commit after two successful tests.

avatar wilsonge
wilsonge - comment - 16 Feb 2018

OK So honestly my gut reaction was to actually close this PR as I think actually the number of times people use it is going to be relatively low and it's going to be interface clutter most the time. However most people think this is going to be useful during site building as a debugging tool. So I'm going to ask that we actually do just that, let's put this tab behind the debug config variable so that it's accessible when people are having plugin issues but in day to day usage there's just the parameters that people will be needing to use.

avatar brianteeman
brianteeman - comment - 16 Feb 2018

removing rtc

avatar alikon
alikon - comment - 16 Feb 2018

i'm not sure this is a day by day feature, but i'm just scaried about like i'm a 4.0 newcomers

avatar Fedik
Fedik - comment - 16 Feb 2018

let's put this tab behind the debug config variable so that it's accessible when people are having plugin issues but in day to day usage there's just the parameters that people will be needing to use.

A plugins parameters also not in day to day usage,
you configure it once and then forgets it exists. ?

I do not think that it a good idea to hide it behind "debug" flag, will be another "hidden feature".

I would like to see it as it is.

avatar wilsonge wilsonge - change - 25 Feb 2018
Status Ready to Commit Pending
avatar mbabker
mbabker - comment - 9 Apr 2018

If putting this into the UI is going to be a controversial option (FWIW I'm not a fan of hiding features behind JDEBUG, it really makes them forgotten and it has bitten us in the past), the other option is a console command. But this gets to be painful as all can be because that would be forced to load all plugins in the system or start passing arguments to limit plugin/group, but at that point you might as well RTFC and the command is useless). So I'd rather just roll with this as is, sure it adds some UI clutter but it's exposing a reference point that doesn't exist otherwise and IMO this is a self-documenting functionality that belongs in the code versus being cross-referenced into the docs wiki or some other place which lists plugins and the events they operate on.

avatar laoneo
laoneo - comment - 9 Apr 2018

Console command sounds also interesting. Don't think it would be so much of an issue to load all plugins. Something similar to what symfony has app/console debug:event-dispatcher.

avatar mbabker
mbabker - comment - 2 Jun 2018

Closing, has no real utility unless we force plugins to implement the subscriber interface, which at the earliest would be 5.0.

avatar mbabker mbabker - change - 2 Jun 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-06-02 00:59:52
Closed_By mbabker
Labels Added: ?
avatar mbabker mbabker - close - 2 Jun 2018

Add a Comment

Login with GitHub to post a comment