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#2814 Remove redundant call to replaceContactTokens #21414

Merged
merged 1 commit into from
Sep 9, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 9, 2021

Overview

Remove redundant call

Before

replaceContactTokens called in replaceGreetingTokens and after it too

After

poof

Technical Details

See #21413 for what it looks like if we copy replaceGreetingTokens back into the TokenCompatSubscriber ready to clean it up - it becomes clear this call is already happening and this is a duplicate

(the same appears to be true of the hook call).

Logically no contact tokens remain by the time we get to this point - replaceGreetingTokens actually replaces any contact or hook tokens in the provided string - and test cover is very solid

Comments

@civibot
Copy link

civibot bot commented Sep 9, 2021

(Standard links)

@civibot civibot bot added the master label Sep 9, 2021
@eileenmcnaughton eileenmcnaughton changed the title Remove redundant call [wip] Remove redundant call Sep 9, 2021
@eileenmcnaughton eileenmcnaughton changed the title Remove redundant call Remove redundant call to replaceContactTokens Sep 9, 2021
@@ -149,7 +149,6 @@ public function onRender(TokenRenderEvent $e) {

if (!empty($e->context['contact'])) {
\CRM_Utils_Token::replaceGreetingTokens($e->string, $e->context['contact'], $e->context['contact']['contact_id'], NULL, $useSmarty);
$e->string = \CRM_Utils_Token::replaceContactTokens($e->string, $e->context['contact'], $isHtml, $e->message['tokens'], TRUE, $useSmarty);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only difference between how this is called here & how it is called in replaceGreetingTokens is that in the former case it always sets TRUE for $isHtml - however it wouldn't matter because the tokens are replaced in the first function so true vs false is meaningless here.

@eileenmcnaughton eileenmcnaughton changed the title Remove redundant call to replaceContactTokens dev/core#2814 Remove redundant call to replaceContactTokens Sep 9, 2021
@eileenmcnaughton
Copy link
Contributor Author

@totten I closed #21413 but I think this should be merged - that way if we lose momentum to do the eval dance now we won't have to figure out that this line does nothing all over again

@seamuslee001 seamuslee001 merged commit 9b17d74 into civicrm:master Sep 9, 2021
@seamuslee001 seamuslee001 deleted the greet branch September 9, 2021 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants