CRM-11153 Custom tokens in scheduled reminders

jarune:

ping us on irc after the meeting and maybe we can help debug and figure out whats happening and why

Closing due to lack of activity.
I haven't tested but this may be fixed already in the latest version.

wondering if there is any evidence of this existing. I can't see anything, and this is set for a 'future version' but is closed.

Not disputing status of the issue, but a a few quick thoughts:

1. There are multiple ways to trigger background tasks (like scheduled reminders) – e.g. via drush, wp-cli, bin/cron.php, bin/cli.php. In the past, some combinations wouldn't use any hooks properly. (I'd call that a bug, although it's debatable, and... off the top of my head... I'm not sure what combinations were/are good/bad.) One might try triggering background tasks a different way.

2. As part of CRM-13244, the handling of tokens for scheduled reminders has been overhauled. (PR #6687 for 4.7/master was accepted; PR #6297 for 4.6 is closed.) Looking at TokenCompatSubscriber, I'd expect the hook-based custom tokens to work, although I don't remember testing them.

3. If one needs to define custom tokens that are particular to scheduled reminders (e.g. referencing the activity or event that matched the schedule), take a look at the links in CRM-13244 and at http://wiki.civicrm.org/confluence/display/CRMDOC/Token+Reference .

I think it's likely that this issue was fixed as a by-product of scheduled reminder fixes in 4.7, but haven't tested. If anyone does test custom tokens with 4.7 reminders pls update this issue FV or status.

Note that this issue is for allowing custom tokens to work in scheduled reminders, not adding a global.url token to CiviCRM core. That would imo also be a useful addition and would welcome a PR for adding it.

ah yes Coleman you are right with your last comment. my 'hope' is for a global.url token in core. will wait and see if original author comes back with some info.

FYI – I appear to have just run into this with 4.6.10 – custom tokens are completely ignored in scheduled reminder emails.
I reviewed CRM-13244 but there is significant refactoring there. Any chance someone is aware of a reasonably easy way to backport a fix for the custom tokens via hooks to 4.6?

FWIW – the tokenValues hook is getting hit by the scheduled reminders action, and the values are getting constructed. They just aren't being replaced in the email content.

Brian Shaughnessy This looks like same / similar issue as https://issues.civicrm.org/jira/browse/CRM-17038. Maybe you can coordinate w/ Niels Heinemann and Jon-man Cheung.

All I know is that tokens are no longer work in scheduled reminders in CiviCRM 4.6.10
Including the 'core' tokens Coleman is referring to. Core team are unable to work on this issue.

Core tokens were working fine for me in schedule reminders. Only custom tokens were not working.

I found the issue –
in CRM_Core_BAO_ActionSchedule::sendReminder() the $contactID is passed as an int, and $contact (list of existing token values) is passed as a single-level array. the tokenValues hook expects $contactIDs as an array and $contact as a multi-level array, keyed off the contact ID. this dates back to earlier versions of the hooks when the data could have been passed as a single contact or array of contacts. but there was refactoring a few versions back to make it consistent – this chunk of code seems to have been missed.

it's possible to deal with the discrepancy in the hook implementation. early in tokenValues put:

$single = false;
if (!is_array($contactIDs)) {
    $contactIDs = array( $contactIDs );
    $origValues = $values;
    $single = true;
}

then at the very bottom put:

if ($single) {
    $values = $origValues + $values[$id];
}

this effectively lets you work with the tokenValues in the standard way and then pass them back in the way required by this usage. of course – we really need to fix this usage so that it's consistent with the other use cases.

I only tested on 4.6.10 – so this may be fixed in 4.7 with the new refactoring there.

Monish Deb Given Brian's latest comment, I think it makes sense to review this and make sure the data structures pass and expected are consistent in 4.7.

Submitted the PR https://github.com/civicrm/civicrm-core/pull/7535

Brian Shaughnessy can you please verify my changes ?

Thanks!

I took a look at the code in the PR & I am +1 on merging it. It makes sense to me

Hi Monish Deb The "fixversion" on this ticket says "unscheduled," but the PR is merged and has tag 4.7. Would it be correct to update "fixversion" to "4.7.[something]"?

Allen Shaw thanks for addressing it. The PR was merged in 4.7.beta8 as per the commit history, so updated the fix version to 4.7

Thanks, Monish Deb!