dev/core#3177 Switch sms to use flexmailer token rendering

[eileenmcnaughton]

Overview

dev/core#3177 Switch sms to use flexmailer token rendering

Note that I have only been able to do limited testing on this so it needs much more robust testing

I found that what I COULD do was

1. install twillio https://civicrm.org/extensions/twilio-sms-provider
2. add a new SMS provider - with made up credentials
3. Make sure users have the 'Create SMS' permission
4. At this point I could create mass mailings and 'send' them - I have no idea what happened to them when sent - I was just inspecting the values up until the send was hit

UPDATE - look what I found - https://github.com/3sd/io.3sd.dummysms - haven't tried it though

Before

Regardless of whether flexmailer is installed tokens for SMS are rendered through smarty

After

I think the answer is to split out SMS processing from Mailing Job processing. To this end I have Created a new class SMSJob that extends MailingJob and overrides the deliverGroup function with one that does only the much lesser amount of work that is required for SMS - which really just needs body-text & a phone number it seems

Technical Details

I suspect that when flexmailer is NOT installed this will result in a loop - options are to use tokenProcessor directly, not mailing_preview api or to simply call parent::deliverGroup absent flexmailer. Note that I didn't see any evidence of the SMS screen offering any non-contact tokens so the latter might be fine

Comments

@totten @seamuslee001

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

I'm surprised this worked at all in your testing? This should be $field['phone'].

[eileenmcnaughton]

Opps - yeah that change was a tidy up I made right before commit

[demeritcowboy]

I suspect that when flexmailer is NOT installed this will result in a loop

Seems ok with or without flexmailer, subject to the $field thing above. I only tested a simple scenario - was there a reason you think it would loop?

[demeritcowboy]

Noting that if flexmailer isn't installed you see this in the preview section if you open it:

<br />
<b>Deprecated</b>:  Deprecated function CRM_Utils_Token::getTokenDetails, use If you hit this in mailing code you should use flexmailer - otherwise use the token processor.

[eileenmcnaughton]

I thought it might loop cos I thought that api_mailing_preview might call compose which might wind up back in deliverGroup - but I guess it doesn't (and shouldn't)

The deprecation message is there currently - but people with flexmailer should have a deprecation-free experience..

[demeritcowboy]

Ok cool give me another day to check some things but otherwise this was good.

[eileenmcnaughton]

@demeritcowboy wow - this is looking easier than I thought!

[demeritcowboy]

When not using flexmailer and sending to multiple recipients, custom field tokens are blank whereas they're fine before the patch. According to http://stats.civicrm.org/?tab=sites about half of sites aren't using flexmailer. I'll try to see if it's something weird locally.

But otherwise ok. The greeting token as in the original ticket works with flexmailer now but still doesn't without flexmailer, but I think that's expected here.

(For reference mostly used RingCentral for testing.)

[demeritcowboy]

Hmm yes it seems somehow this breaks custom field tokens when not using flexmailer.

[eileenmcnaughton]

that's weird - I feel it shoulfd be no change when not using flexmailer

[demeritcowboy]

The difference seems to be there is a bug in Mailing.preview - when the contact does not have an email it returns blank for custom fields. It seems partly because it doesn't pass on_hold = 0 to CRM_Contact_BAO_Query::apiQuery in getTokenDetails. You can see this if you make a bulk sms (or even a bulk emailing) and call cv api3 Mailing.preview id=1 contact_id=1234 on it for the contact that doesn't have an email.

So it's a little bit independent of the PR, except that the PR is relying on Mailing.preview. Maybe if there was some way for this line to know if it's an sms and then pass false for the skipOnHold param.

[eileenmcnaughton]

@demeritcowboy maybe what I just pushed up will work

[eileenmcnaughton]

thanks @demeritcowboy