Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit how plugin listen / register to events #7241

Open
tsteur opened this issue Feb 18, 2015 · 1 comment
Open

Revisit how plugin listen / register to events #7241

tsteur opened this issue Feb 18, 2015 · 1 comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.

Comments

@tsteur
Copy link
Member

tsteur commented Feb 18, 2015

To register an event one currently has to implement the method getListHooksRegistered in the plugin descriptor class. Eg

public function getListHooksRegistered()
    {
        return array(
            'API.getSegmentsMetadata' => 'getSegmentsMetadata'
        );
    }

Since the wording at some point changed to "Events" we should rename it to getListEventsRegistered in a backwards compatible way. Maybe there could be even a better name for this method?

Maybe, there would be also another way to listen to events. Eg having an Events.php (could be confusing with Events plugin etc). Because of having this getListEventsRegistered in the plugin file we often do have some kind of logic in that class although it shouldn't really contain any logic. Even worse we can't inject anything in tests there which makes it really hard to test this kinda stuff as the plugin instance is created in the background see eg https://github.com/piwik/piwik/blob/2.11.0-b2/plugins/BulkTracking/tests/Framework/TestCase/BulkTrackingTestCase.php#L34-L48.

Alternatively or on top maybe we can also check whether it is possible to register an event like this 'API.getSegmentsMetadata' => array('\Any\Other\ClassName', 'getSegmentsMetadata'). I'm almost sure this is already possible, maybe we should make it the recommended way to move logic out of pluginname.php in which it would be ok to register events in that class.

Maybe we could also somehow simplify this but I think it is kinda ok this way.

array(
        'API.getSegmentsMetadata' => array(
            'before'   => true,
            'function' => 'getSegmentsMetadata',
        )
    );

Any ideas welcome

@tsteur tsteur added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. labels Feb 18, 2015
@tsteur tsteur added this to the Piwik 3.0.0 milestone Feb 18, 2015
@mattab mattab modified the milestones: Mid term, 3.0.0 Aug 13, 2015
@mattab
Copy link
Member

mattab commented Aug 13, 2015

renamegetListHooksRegistered to getListEventsRegistered (keep BC) was extracted in #8565

@mattab mattab modified the milestones: Long term, Mid term Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.
Projects
None yet
Development

No branches or pull requests

2 participants