This repository has been archived by the owner on Feb 6, 2023. It is now read-only.
Upgrade flexmailer symfony code to run on v3.4 and 4.4 as per core PR… #57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`CIVICRM_FLEXMAILER_HACK_LISTENERS` and `registerListeners()` function were used in older versions to wire-up Flexmailer's dispatchers. This is now accomplished via public API, `hook_civicrm_container` - seems to work, no reason to go back. Note: I was confused by two things: 1. The previous calls didn't look like they should work. 2. Somehow, the API override still worked - even though there was no obvious callpath to `addSubscriberService()` It turns out that `registerApiProvider()` handles registration with the dispatcher, so `addSubscriberService()` isn't actually used.
totten
reviewed
May 27, 2020
src/Services.php
Outdated
| $dispatcher->addListenerService($listenerSpec[0], $listenerSpec[1], $listenerSpec[2]); | ||
| $dispatcher->addSubscriberService('civi_flexmailer_api_overrides', 'Civi\FlexMailer\API\Overrides'); | ||
| $dispatcher->addListener($listenerSpec[0], [new SercviceClosureArgument(new Reference($listenerSpec[1][0])), $listenerSpec[1][1]], $listenerSpec[2]); | ||
| $dispatcher->addSubscriber(new Reference('civi_flexmailer_api_overrides')); |
There was a problem hiding this comment.
Aah, so this example change is much clearer than the upstream docs. 👍 I'm not certain that these lines work per se (more in a sec), but it was enlightening to read.
Digging in on this PR/function was a bit of rabbit hole...
- Neither the upstream docs, changelog, deprecation PR, nor docs issue provide an example snippet or reference to the alternative helpers. Kudos to you for figuring a work-a-like in Symfony 4.x's class model.
- There's a typo in the class name (
Sercvice). Digging into this revealed that the functionregisterListeners()is never actually called. It's leftover from some alpha/beta hackery and was disabled (17f142e) whenhook_civicrm_containerbecame more reliable. This also explains things worked despite some more flaws in the old variant ofregisterListeners(). - How does
civi_flexmailer_api_overrideswork at all? Well, it does. ✅ It's because there's a call toregisterApiProvider(), and that is what really handles registration.
In core's #17380, we ultimately preserved `addListenerService()`, so this doesn't have to be removed.
|
Since the core PR restored |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… #13780
ping @MikeyMJCO @totten @KarinG