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#4109) Fix tokens like {contact.email_primary.email} #25548

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Feb 10, 2023

Overview

Fix advertised tokens tokens for contact-related tokens (such as {contact.email_primary.email}).

See: https://lab.civicrm.org/dev/core/-/issues/4109

Before

The preferred advertised token style stopped working in the fight to get all the weird tested options to work - and because unit tests were not added for the new preferred style ..... so {contact.email} worked but not {contact.email_primary.email}

After

Preferred style works again & has a test

Technical Details

Comments

@MegaphoneJon

@civibot
Copy link

civibot bot commented Feb 10, 2023

No issue was found matching the number given in the pull request title. Please check the issue number.

@civibot
Copy link

civibot bot commented Feb 10, 2023

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

OK - I think this is the test that I was fixing when the advertised tokens broke... but now I have the test cover to not regress....

CRM_Core_BAO_MessageTemplateTest::testContactTokens
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
'{contact.im_primary.provider_id:label}' => 'IM Provider'
'{contact.im_primary.name}' => 'IM Screen Name'
'{contact.address_primary.country_id.region_id:name}' => 'World Region'

  • '{contact.website_first.url}' => 'Website'
    '{contact.custom_9}' => 'Contact reference field :: Cu... Group'
    '{contact.custom_7}' => 'Country :: Custom Group'
    '{contact.custom_8}' => 'Country-multi :: Custom Group'
    @@ @@
    '{contact.checksum}' => 'Checksum'
    '{contact.id}' => 'Contact ID'
    '{important_stuff.favourite_emoticon}' => 'Best coolest emoticon'
  • '{contact.url}' => 'Website'
    )

/home/jenkins/bknix-dfl/build/build-x/web/sites/all/modules/civicrm/tests/phpunit/CRM/Core/BAO/MessageTemplateTest.php:437
/home/jenkins/bknix-dfl/build/build-x/web/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:235
/home/jenkins/bknix-dfl/extern/phpunit8/phpunit8.phar:1721

@eileenmcnaughton
Copy link
Contributor Author

OK this wasn't fixed by the last website fix ...

Civi\Token\TokenProcessorTest::testRenderDoubleUrl
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<a href="https://example.com">blah'
+'<a href="https://">blah'

@totten totten changed the title dev/core#4109 Fix advertised tokens to work (dev/core#4109) Fix tokens like {contact.email_primary.email} Feb 15, 2023
@totten
Copy link
Member

totten commented Feb 15, 2023

To r-run this, I took the problematic example from dev/core#4109 and tossed it into an adhoc call to CRM_Core_TokenSmarty::render(). I initially had some trouble with it (so I wound up inspecting more examples to see if I was crazy -- e.g. https://gist.github.com/totten/6b0700bbd82c7408aca0a5ba9e848568), but the problem was just caching. When you switch back/forth between the original code and the updated code, you need to cv flush.

Beyond that, tests are passing, and the new test looks good+relevant. Merging.

@totten totten merged commit 490bdff into civicrm:5.59 Feb 15, 2023
@totten totten deleted the 559_token branch February 15, 2023 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants