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

Add support for Contact tokens in welcome email using TokenProcessor #24235

Merged
merged 5 commits into from
Sep 2, 2022

Conversation

masetto
Copy link
Contributor

@masetto masetto commented Aug 12, 2022

Overview

Add the possibility of using contact tokens in the welcome email

Before

In the welcome email contact tokens can not be used.

After

In the welcome email contact tokens can be used. For example:

Dear {contact.first_name} {contact.last_name},
you are welcome to the {welcome.group}.

To view or update your communication preferences: {contact.comm_pref_supporter_link}

Technical Details

Add new token processor.
We continue to uses replaceWelcomeTokens because I don't think it is possible to replace them using token processor.

Note

This is a partial pull request. I would like to understand if it is OK the way I did it. Then it would be replicated forResubscribe, Subscribe, Unsubscribe messages.

@civibot
Copy link

civibot bot commented Aug 12, 2022

(Standard links)

@civibot civibot bot added the master label Aug 12, 2022
@masetto masetto changed the title contact tokens in welcome email WIP - REF - contact tokens in welcome email Aug 12, 2022
@masetto masetto changed the title WIP - REF - contact tokens in welcome email (WIP) (REF) contact tokens in welcome email Aug 12, 2022
@mlutfy mlutfy changed the title (WIP) (REF) contact tokens in welcome email (WIP) Add support for Contact tokens in welcome email using TokenProcessor Aug 12, 2022
$text = CRM_Utils_Token::replaceWelcomeTokens($text, $group->title, FALSE);

$tokenProcessor = new \Civi\Token\TokenProcessor(\Civi::dispatcher(), [
'controller' => __CLASS__,
'smarty' => FALSE,
Copy link
Contributor

Choose a reason for hiding this comment

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

Also specifiy 'schema' => ['contactId'] here as best practice

@eileenmcnaughton
Copy link
Contributor

Yes this looks great - ideally the welcome tokens WOULD be moved to the token processor but that is a slightly bigger challenge

I think the way we would do it is

@masetto
Copy link
Contributor Author

masetto commented Aug 16, 2022

@eileenmcnaughton I think it is too complex for me to do what you are asking ....maybe we do this in another PR?

In welcome email I'd also like to replace action tokens using CRM_Utils_Token::replaceActionTokens: would allow us to write for example:

You can <a href="{action.unsubscribeUrl}" >unsubscribe from this mailing</a> at any time.

But I would have to use this instruction CRM_Mailing_BAO_Mailing::getVerpAndUrls($job, $queue_id, $eq->hash, $eq->email) as it is done in CRM_Mailing_Event_BAO_Resubscribe class, but I don't have the "job" here...

I took a look at "Resubscribe.php" and there in addition it is called replaceMailingTokens: token process have a specific schema for that?

@eileenmcnaughton
Copy link
Contributor

@masetto - yes I think it's fine for the scope of this PR to just be replacing those CRM_Utils_Token::replaceDomainTokens functions & adding in the ContactToken support. Ideallly it would have a test but I think these functions are pretty tricky to write tests on so we could probably let that slip

@masetto
Copy link
Contributor Author

masetto commented Aug 26, 2022

@eileenmcnaughton I would have finished and I just replace CRM_Utils_Token::replaceDomainTokens. Let me know if I can squash commits.

@masetto
Copy link
Contributor Author

masetto commented Aug 26, 2022

@eileenmcnaughton, my changes cause a regression 😢 : in Subscribe.php and Confirm.php I removed this code:

    $bao = new CRM_Mailing_BAO_Mailing();
    $bao->body_text = $text;
    $bao->body_html = $html;
    $tokens = $bao->getTokens();

since $tokens was no longer used.
But the function getTokens calls the hook alterMailContent with this stack trace (for subscription):

#0  CRM_Mailing_BAO_Mailing->getTemplates() called at [CRM/Mailing/BAO/Mailing.php:784]
#1  CRM_Mailing_BAO_Mailing->_getTokens(html) called at [CRM/Mailing/BAO/Mailing.php:730]
#2  CRM_Mailing_BAO_Mailing->getTokens() called at [CRM/Mailing/Event/BAO/Subscribe.php:226]
#3  CRM_Mailing_Event_BAO_Subscribe->send_confirm_request called at [CRM/Mailing/Event/BAO/Subscribe.php:357]
#4  CRM_Mailing_Event_BAO_Subscribe::commonSubscribe called at [CRM/Mailing/Form/Subscribe.php:154]

It does not seem appropriate to re-introduce this code, because anyone would wonder what it is for.
What do you suggest we do?

I have not checked but maybe 'getTokens' does something else...

@eileenmcnaughton
Copy link
Contributor

@masetto is the regression that alterMessageContent is now skipped? It seems like ideally we would still call getTemplates to retrieve the html, text & subject - but bypass the getTokens part?

@masetto
Copy link
Contributor Author

masetto commented Aug 31, 2022

@eileenmcnaughton I had a mistake: the alterMessageContent hook was called by getTokens function, but the content of templates was never used.
So I (re)declare CRM_Mailing_BAO_Mailing object, then I call getTemplates function and I use $templates['html'] and $templates['text'] instead of $html and $text. Now alterMessageContent hook is called and the message content is really the "altered content".

@eileenmcnaughton
Copy link
Contributor

@masetto hmm - so the hook wasn't working before - is it an issue for you for it to work? Otherwise if it wasn't working I'd just let it drop it (since we still have more cleanup to get that code off the other tokens in that lot & it would be easier not to work around that hook if it never worked in the first place)

@masetto
Copy link
Contributor Author

masetto commented Sep 1, 2022

@eileenmcnaughton Now the hook works and I think it's fine 😛
I realise that it solves an issue unrelated to this PR, but I would need it: I am using an extension that adds a header and footer to all system emails and automated messages are not being intercepted. Now they do.

But if you want I can do another PR after you merge this one.

@eileenmcnaughton
Copy link
Contributor

@masetto that's fine - it can stay as it is - are you happy with this now / ready to remove the WIP

@masetto masetto changed the title (WIP) Add support for Contact tokens in welcome email using TokenProcessor Add support for Contact tokens in welcome email using TokenProcessor Sep 1, 2022
@eileenmcnaughton
Copy link
Contributor

Looks good - great improvement

@eileenmcnaughton eileenmcnaughton merged commit 639523a into civicrm:master Sep 2, 2022
@eileenmcnaughton
Copy link
Contributor

@masetto I took a look at the last place with the replaceDomainTokens but then as I dug into it I started to think that this actually causes a regression - the templates loaded by $templates = $bao->getTemplates(); are actually different components I think.... it's loading the header & footer rather than the relevant component

#24446

@eileenmcnaughton
Copy link
Contributor

We really should add an api for this stuff - there is the sendTemplate function but it is really hard to understand how to use it & I think there is another 'light weight' version but we just need an api with sensible params not a bunch of internal functions

@eileenmcnaughton
Copy link
Contributor

OK - I updated ^^ with what I think will work to use the right template AND call the hook....

@masetto
Copy link
Contributor Author

masetto commented Sep 2, 2022

Oh, great! Now we could also adapt these classes to use CRM_Core_BAO_MessageTemplate ...

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