-
Notifications
You must be signed in to change notification settings - Fork 22
use of Symfony EventDispatcher instead of CRM_Utils_Hook #1
Comments
|
Tim - then let's obsolete the hook system in favor of events. But keeping both is only confusing and adds significantly to the CiviCRM learning curve as well as things that need to be tested/can break in each release. We must stop having 5 different ways of doing anything in CiviCRM because of NIH or 'I know better' syndrome. CiviCRM ends-up being the unmaintainable patchwork of half-baked ideas that it currently is. |
Tim - see https://forum.civicrm.org/index.php?topic=36086.msg157211#msg157211. Suggestion a) which is your 4) above is trivial to implement. a) is incremental, b) is revolutionary, but IMO either are preferable to the current YAC situation. |
Not trying to hijack this discussion but trying to compare hook vs. dispatcher for our needs:
@nganivet on 4.6 we did try to do it via hooks, it felt rather clunky (don't recall the details, we needed to alter the message composition too) @totten would EventDispatcher make it easier to cover these cases? |
1. Open TrackingThe default open-tracking is handled through a service ( namespace Civi\UberTrack;
class UberOpenTracker extends BaseListener {
public function onCompose(ComposeBatchEvent $e) {
if (!$this->isActive() || !$e->getMailing()->open_tracking) {
return;
}
Civi::service('civi_flexmailer_open_tracker')->setActive(FALSE); // Opt-out of default
foreach ($e->getTasks() as $task) {
// Add code using $task->getMailParam('html') and $task->setMailParam('html').
}
}
}
...
use Civi\FlexMailer\FlexMailer as FM;
$container->setDefinition('uber_open_tracker',
new Definition('Civi\UberTrack\UberOpenTracker'));
$container->findDefinition('dispatcher')->addMethodCall('addListenerService', array(
FM::EVENT_COMPOSE,
array('uber_open_tracker', 'onCompose'),
FM::WEIGHT_ALTER+10)
); Notably, this only works as long as the weight of I'm a bit on the fence about the best notation for opting out of I mention the swap-out issue because you also asked about click-tracking ... though it may take a moment to explain. 2. Click TrackingClick-tracking is also handled by a pair of services:
Right now, these services have a special place -- click-tracking is quite finicky about when we apply the transformation. URLs can have dynamic tokens which are tied into the templating language; to get backward-compatible semantics in the default scenarios, I had to apply click-tracking at a specific place in To actually change the click-tracker, we probably need some kind of improvement. Options:
3. OthersYou asked about open-tracking and click-tracking as examples. I think open-tracking is more representative because several other features follow the same pattern: basic headers, bounce-tracking, template-evaluation, and attachments are all handled by similar services. (They monitor |
Did we move forward on hooks vs. event dispatcher?
|
Xavier - Exciting: Tim has recently committed this to core! The Symphony event dispatcher is now the main 'event bus' in CiviCRM and triggering the legacy hooks. The events triggered by core are a superset of the hooks, and extensions can define their own events. You can get to Tim for more info. |
For a bit of follow-up, civicrm/civicrm-core#9949 unified Symfony Events and hooks. Now, events are canonically dispatched through Symfony. If the event is named This unification leaves a question open for FlexMailer -- perhaps we should rename the FlexMailer events to follow the |
Tim - It depends on whether you want extensions to migrate to Symfony events over time, or leave Symfony just for core usage. If the former, then we have to create all new hooks as Symfony events, write documentation, make an announcement, do some training, migrate all examples and sample extensions to events, and obsolete old hooks over time. If the later (or still undecided), then I would advise that we rename the FlexMailer events and document these in the develop[er guide as hook_*. |
Tim - note my current thinking is that, as 'core extension' are created, extension developers will need to be able to override and/or prioritize their extensions over the ones from core. So for example:
I do not know if this is easier implemented from Symfony, or if we can 'pass-through' the Symfony prioritization to the hook_* extension system. This might help determine the answer to my previous comment. |
Yeah, I probably should rename them and document as Regarding the ordering of events, the key things to know are:
|
Relevant PR and critique: #7 |
Doing #8 instead. |
This introduces YAC (Yet Another Construct) that developers will have to learn with no perceived benefit over the current hook system. Most of our developers come from the CMS world and are used to the hook construct. Despite the explanation in the README.md, I see no good reason to introduce another construct.
If the concept of priority listener and vetoing is really useful for CiviCRM, then let's introduce this in the hook system! This could be done quite easily with the hook cache in 4.7 (https://github.com/civicrm/civicrm-core/blob/master/CRM/Utils/Hook.php#L209).
See https://issues.civicrm.org/jira/browse/CRM-19813 for implementing 'core hooks' based on the above and further enabling the LExIM strategy.
The text was updated successfully, but these errors were encountered: