From f4bf887ce90d2e60c7815d7e7228b178446f2297 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Thu, 14 May 2020 01:10:05 -0700 Subject: [PATCH 1/2] OpenTracker - Create URL via CRM_Utils_System::externUrl() This aims to improve cooperation between `flexmailer`, `civicrm-wordpress:wp-rest/`, and https://github.com/civicrm/civicrm-core/pull/17312 -- so that flexmailer produces compliant URLs. Before ------ The open-tracking URL is constructed as a reference to the standalone script `extern/open.php` using certain `$config` properties. After ----- On Civi v5.23+, it uses `CRM_Utils_System::externUrl()` API to request the full URL of `extern/open`. Flexmailer is no longer responsible for figuring the URL, and other agents (via `hook_civicrm_alterExternUrl`) can do so. On Civi v5.22 and earlier, it continues using the same old formula. This provides drop-in/bug-level-compatibility on older versions. This can be removed in a couple months. Comments -------- I suspect this also fixes [dev/mail#17](https://lab.civicrm.org/dev/mail/issues/17), wherein the open tracker has flawed URLs in certain multilingual configurations. However, I can't confirm for certain because I don't have that configuration. But just to game this out... * If the current patch fixes dev/mail#17, then huzza! * If it doesn't, then the problem is in `civicrm-core`'s `CRM_Utils_System::externUrl()` (not `flexmailer:OpenTracker.php`). Which means: * The problem already affects other use-cases in `civirm-core` in v5.23+. * The fix should go in `civicrm-core`. Either way, addressing dev/mail#17 shouldn't require any further change in `flexmailer`. --- src/Listener/OpenTracker.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Listener/OpenTracker.php b/src/Listener/OpenTracker.php index 1c51ab3..4b5b2f8 100644 --- a/src/Listener/OpenTracker.php +++ b/src/Listener/OpenTracker.php @@ -26,14 +26,19 @@ public function onCompose(ComposeBatchEvent $e) { $config = \CRM_Core_Config::singleton(); + // TODO: After v5.21 goes EOL, remove the $isLegacy check. + $isLegacy = !is_callable(['CRM_Utils_System', 'externUrl']); + foreach ($e->getTasks() as $task) { /** @var \Civi\FlexMailer\FlexMailerTask $task */ $mailParams = $task->getMailParams(); if (!empty($mailParams) && !empty($mailParams['html'])) { - $mailParams['html'] .= "\n" . '"; + $openUrl = $isLegacy + ? $config->userFrameworkResourceURL . "extern/open.php?q=" . $task->getEventQueueId() + : \CRM_Utils_System::externUrl('extern/open', "q=" . $task->getEventQueueId()); + + $mailParams['html'] .= "\n" . '"; $task->setMailParams($mailParams); } From c4f95e1b90e5c58ce8d89745a21981fcdc594dd4 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Sat, 16 May 2020 00:29:32 -0700 Subject: [PATCH 2/2] OpenTracker - Check compat by version# instead of is_callable() Generally, I think the `is_callable()` formulation is better. However, this specific class (`CRM_Utils_System`) defines a `__callStatic()` magic-methods, which prevents `is_callable()` from being meaningful. So here's Plan B. --- src/Listener/OpenTracker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Listener/OpenTracker.php b/src/Listener/OpenTracker.php index 4b5b2f8..1e0dc84 100644 --- a/src/Listener/OpenTracker.php +++ b/src/Listener/OpenTracker.php @@ -27,7 +27,7 @@ public function onCompose(ComposeBatchEvent $e) { $config = \CRM_Core_Config::singleton(); // TODO: After v5.21 goes EOL, remove the $isLegacy check. - $isLegacy = !is_callable(['CRM_Utils_System', 'externUrl']); + $isLegacy = version_compare(\CRM_Utils_System::version(), '5.23.alpha', '<'); foreach ($e->getTasks() as $task) { /** @var \Civi\FlexMailer\FlexMailerTask $task */