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

(dev/core#2673) Email Tokens - Custom tokens in subject block similar tokens in body #21080

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

twomice
Copy link
Contributor

@twomice twomice commented Aug 10, 2021

Overview

In an email, a token from an extension in a subject will inhibits the same token group in the email body.

Original ticket and proposed fix here: twomice/com.joineryhq.reltoken#15 and https://lab.civicrm.org/dev/core/-/issues/2673

Before

In an email, if I put a custom token {custom.name} in the subject and another custom token in the email body and then send the email, I will only receive the value of the {custom.name} that's in the subject. The other custom token in the email body will be blank. Example:

Subject: {custom.name}

Email Body
Name: {custom.name}
Age: {custom.age}
Gender: {custom.gender}

In the email, the {custom.name} is populated in subject and in body, but tokens {custom.age} and {custom.gender} are blank in body, even if they actually have values.

After

Tokens in body are correctly populated regardless of which tokens are used in the subject.

Technical Details

See ticket OP for technical analysis.

Comments

[none]

dev/core#2673: In an email, a token from an extension in a subject will inhibits the same token group in the email body.
@civibot
Copy link

civibot bot commented Aug 10, 2021

(Standard links)

@civibot civibot bot added the master label Aug 10, 2021
@twomice
Copy link
Contributor Author

twomice commented Aug 10, 2021

Note +1 from end user at https://lab.civicrm.org/dev/core/-/issues/2673#note_62139

@colemanw colemanw added the merge ready PR will be merged after a few days if there are no objections label Aug 10, 2021
@totten totten changed the title Use recursive array merge to fix issue with tokens in subject and body. (dev/core#2673) Email Tokens - Custom tokens in subject block similar tokens in body Aug 11, 2021
@totten
Copy link
Member

totten commented Aug 11, 2021

Change looks conceptually simple/good, and the discussion thread seems to indicate it works (r-run) for 3 people.

r-maint: Ordinarily, for a bug like this, a unit-test would be pretty tempting. You could expect similar issues in other code that follows the CRM_Utils_Token patterns. But there's really no sensible way to target such code with a test... CRM_Utils_Token patterns are too spread-out/diffuse. Really, more of the token-mechanics should be shared (e.g. more code should use TokenProcessor instead of CRM_Utils_Token).

I've added a tangential test in #21090. If one is looking for a place to train some unit-testing muscles, they could potentially make a test for #21080 by borrowing bits from tests/phpunit/CRM/Activity/BAO/ActivityTest.php and #21090. But it's not critical in this context.

+1 for merge ready

@totten totten merged commit 15f0727 into civicrm:master Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants