(Regression) CRM_Mailing_ActionTokens - Degrade gracefully

[totten]

Most of the CRM_*_Tokens classes include a checkActive() function which prevents them from trying to do anything if they don't have the necessary data. However, this one was missing it, which means that it attempts to evalute {action.*} tokens even when they're not valid.

Note: This was reported on Stack Exchange. As of this writing, I haven't had a chance to reproduce or test the fix in a full deployment, but I'm opening the PR to get the test-suite and discussion going.

[colemanw]

@totten several test failures related to this.

[totten]

OK, I've updated and rebased. It now passes those tests locally.

[totten]

The reported failure, CRM_Contact_BAO_ContactType_ContactTest.testCRM19133, is a common false-negative.

[totten]

To test this with a real scheduled reminder, used this procedure;

• Create an activity (scheduled meeting)
• Create a scheduled-reminder (for scheduled meetings) with this body:

Hello

Optout: {action.optOutUrl}

Name: {contact.display_name}

• Iteratively execute: echo 'delete from civicrm_action_log' | drush cvsqlc && cv api -U admin job.send_reminder

This procedure reproduced the hard-error (in master) and showed an improvement (in master-acttok).

In the case where one uses an invalid token, this PR doesn't provide a proper warning (that's a separate issue, CRM-20143). But at least we're back to not-crashing.

[eileenmcnaughton]

test this please

[seamuslee001]

@eileenmcnaughton I think this seems sane should this get merged in?

[eileenmcnaughton]

It feels sane (esp due to the sanity of the committer) but I'm struggling to understand how to determine if it is safe

[totten]

Yeah, I imagine that's tough if you haven't gotten your around the CRM_*_Tokens stuff. For a general reader, the main thing to note is

• (a) it makes the class look more consistent with other/similar classes
• (b) the manual test procedure (which i described above) shows better results
• (c) this was new code in 4.7.16 which was intended to be inactive during most use-cases (it just wasn't properly deactivated).
• (d) the tests still pass