This repository has been archived by the owner on Feb 6, 2023. It is now read-only.
OpenTracker - Create URL via CRM_Utils_System::externUrl() #56
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
This aims to improve cooperation between `flexmailer`, `civicrm-wordpress:wp-rest/`, and civicrm/civicrm-core#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`.
This was referenced May 14, 2020
Construct tracking URL with an alternate method to avoid language prefix be appended to base URL
#47
Closed
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.
|
This was originally written/tested on roughly I decided to give it a whirl on 5.21 as well -- and needed to fix the compatibility test ( I guess we just need to wait for the new test run on |
Closed
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.
This aims to improve cooperation between
flexmailer,civicrm-wordpress:wp-rest/, and civicrm/civicrm-core#17312 -- so that flexmailer produces compliant URLs.Before
The open-tracking URL is constructed as a reference to the standalone script
extern/open.phpusing certain$configproperties.It always links to the standalone script, even if you have a different end-point available (e.g.
civicrm/mailing/openorcivicrm-wordpress:wp-rest/).After
On Civi v5.23+, it uses
CRM_Utils_System::externUrl()API to request the full URL ofextern/open. Flexmailer is no longer responsible for figuring the URL, and other agents (viahook_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, 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...
civicrm-core'sCRM_Utils_System::externUrl()(not
flexmailer:OpenTracker.php). Which means:civirm-corein v5.23+.civicrm-core.Either way, addressing dev/mail#17 shouldn't require any further change in
flexmailer.