dev/core#348 Participant tokens on scheduled reminders #21666

eileenmcnaughton · 2021-09-29T11:24:13Z

Overview

This PR adds support for participant tokens in scheduled reminders

Before

Only event tokens were available. The query to add them was convoluted and had an unindexed join

After

Participant tokens are available...

Technical Details

I followed the domain model for events & cached the accessed events to an array. We know from the fact people never noticed the bad join that this isn't being used for high volume :-) But, it seems unlikely to me that, due to the time based nature of events, people would be sending to thousands of events in one hit and, even if they were, it's not a UI action & anyimpact of a huge array is likely to be in the realm of 'annoying in browser' rather than slows the job down (I doubt that it would slow it down anyway as browser lag is usually from initially building a large array not loading to it as required

Also note that event needs to listen to 'participant' AND 'event' - I put the handling for the former in the participant class.

Now that I've gotten to this point I realise how hard I was working around a function on the abstract subscriber class & I have some ideas for a cleanup to follow on

Comments

@magnolia61 some progress - I've gotten tests to pass locally but need to run through this again in the morning - will see if any thing else fails...

It should be testable - but back out quickly if you hit issues as it could be broken

civibot · 2021-09-29T11:24:16Z

(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).

magnolia61 · 2021-09-29T18:52:17Z

@eileenmcnaughton I will try to test. Since you mention the need for an upgrade script, what kind of functionality could I best test first?

eileenmcnaughton · 2021-09-29T18:53:36Z

CRM/Event/Tokens.php

-    $e->query->select('ov.label as event_type, ev.title, ev.id as event_id, ev.start_date, ev.end_date, ev.summary, ev.description, address.street_address, address.city, address.state_province_id, address.postal_code, email.email as contact_email, phone.phone as contact_phone');
-    $e->query->join('participant_stuff', "
-!casMailingJoinType civicrm_event ev ON e.event_id = ev.id
-!casMailingJoinType civicrm_option_group og ON og.name = 'event_type'


Look - an unindexed join!

…d at minimum)

eileenmcnaughton · 2021-09-29T19:07:01Z

@magnolia61 I added the upgrade script just now & woohoo it's passing tests - the upgrade script just replaces a few tokens with name changes - {event.event_id} to {event.id}- see https://lab.civicrm.org/documentation/docs/sysadmin/-/blob/master/docs/upgrade/version-specific.md

magnolia61 · 2021-09-29T21:13:53Z

Thank you Eileen for this PR!

First results: participant core and custom tokens work in scheduled reminders. Yeah!

I have not been able to get participant custom dates working though.
In combination with other participant custom tokens they cause no token at all to be resolved.

I will continue to test and update my findings here.

eileenmcnaughton · 2021-09-29T21:49:30Z

@magnolia61 by custom dates you mean

{participant.register_date|crmDate:short}

OR

{participant.custom_3} where 3 is a date type custom field

magnolia61 · 2021-09-29T22:02:50Z

The later: register date works fine. It's the custom ones indeed.

I just double checked and the date fields that fail have no time field.

I added time to one of those (through the gui) and the token starts working.

magnolia61 · 2021-09-29T22:07:51Z

Also {participant.status_id:name} renders fine as the status name.
I expected {participant.status_id:nlabel} to render the integer value but left the token empty.

eileenmcnaughton · 2021-09-29T22:21:20Z

@magnolia61 is that a typo?

{participant.status_id:nlabel}

should be

{participant.status_id:label}

I think - which would render the label not the integer

eileenmcnaughton · 2021-09-29T22:25:51Z

@magnolia61 can you log a gitlab for the custom date fields & add the label 'sig:token' - I think it will be broader than participant tokens so it should be tracked & fixed but non-blocking on this PR

magnolia61 · 2021-09-30T04:29:41Z

@magnolia61 is that a typo?

{participant.status_id:nlabel}

should be

{participant.status_id:label}

I think - which would render the label not the integer

I checked and the typo is only here in the issue. I used the proper token in the email.

X {participant.status_id:label}
V {participant.status_id:name}

but also

X {participant.role_id:label}
V {participant.role_id:name}

magnolia61 · 2021-09-30T04:37:17Z

@magnolia61 can you log a gitlab for the custom date fields & add the label 'sig:token' - I think it will be broader than participant tokens so it should be tracked & fixed but non-blocking on this PR

https://lab.civicrm.org/dev/core/-/issues/2878 (but I could not add the sig:token label)

eileenmcnaughton · 2021-09-30T05:05:51Z

@magnolia61 I just test - like this

It correctly put in the right label syntax

And that WAS rendered as 'Registered' for me - it took a bit of doing because first I tried to use the 'also include' but of course they don't have participant records

eileenmcnaughton · 2021-09-30T05:08:09Z

@agileware-justin Fyi - I am lucky to have @magnolia61 helping with testing here but this is the participant tokens

agileware-justin · 2021-09-30T06:51:33Z

I can check this ~~tomorrow~~ as soon as I can @eileenmcnaughton - have run out of Gifs today.

magnolia61 · 2021-10-01T07:40:05Z

@magnolia61 I just test - like this

It correctly put in the right label syntax

And that WAS rendered as 'Registered' for me - it took a bit of doing because first I tried to use the 'also include' but of course they don't have participant records

ahhh, found it. For some reason a snug into the token in html
{participant.status_id</span>:label}
Removed the culprit and of course the mentioned tokens work as expected (except for the date without time ones https://lab.civicrm.org/dev/core/-/issues/2878)

eileenmcnaughton · 2021-10-01T08:03:24Z

@magnolia61 so this passes your testing now?

magnolia61 · 2021-10-01T08:10:47Z

@eileenmcnaughton yep!
All participant core tokens work, and all participant custom ones
(except for the date-without-time ones, but you suggested they are out of scope and need a broader fix).

eileenmcnaughton · 2021-10-01T08:13:14Z

@seamuslee001 @colemanw see ^^

test cover is good

magnolia61 · 2021-10-01T08:26:20Z

To specify: I only tested participant core and custom tokens.
Are event core and custom tokens also in scope for this PR. If so I can also test those.

eileenmcnaughton · 2021-10-01T08:36:10Z

@magnolia61 yes the event tokens are refactored too (contact tokens should be unchanged but more testing = more better)

magnolia61 · 2021-10-01T08:40:55Z

CRM/Upgrade/Incremental/php/FiveFortyThree.php

+      'updateActionScheduleToken', 'event.fee_amount', 'participant.fee_amount', $rev
+    );
+    $this->addTask('Replace event type token in action schedule',
+      'updateActionScheduleToken', 'event.event_type_id', 'participant.event_type_id:label', $rev


I am not sure why event type should move from event to participant entity as is is a static value that belongs to the event (but maybe I just don;t get it)

@magnolia61 I think you're right, #21738

magnolia61 · 2021-10-01T09:39:36Z

Testing the event core tokens I found that the reminder choke on the {event.description} field
I cannot find relevant information in the drupal log or civicrm log

When I run the scheduled reminder from civicrm/admin/job
The website encountered an unexpected error. Please try again later.

It's just the description field. And fails whether the field is empty or not.

eileenmcnaughton · 2021-10-01T10:15:35Z

@magnolia61 I just tested

My reminder looks like

And I got this 'sent'

magnolia61 · 2021-10-01T10:27:40Z

I will try to test different combinations for the {event.description}

magnolia61 · 2021-10-01T10:28:39Z

I couldnt't get event custom tokens to work
Like: {event.custom_440}

eileenmcnaughton · 2021-10-01T10:39:05Z

Ok so I retried

& I got

Which does seem like it's working for me

magnolia61 · 2021-10-01T10:49:09Z

I will try to test different combinations for the {event.description}

I re-downloaded civicrm master and applied this pr again and somehow the {event.description} token does work now.
Not sure why and how, but at least i'm testing real life templates and a copy of our production DB

eileenmcnaughton · 2021-10-01T10:58:13Z

@magnolia61 thanks for that - maybe you can test again once the rc is cut so that you double check what the final release

magnolia61 · 2021-10-01T10:59:52Z

I just double checked the event custom ones and they also work now!
So summary:
event core tokens work
event custom tokens work
participant core tokens work
participant custom tokens work

agileware-justin · 2021-10-02T01:56:54Z

Can confirm the Event and Participant tokens are working.

However, I did note this weird issue:

Set the Event start date to 3/10 9am
Registered the Participant
Set up the Scheduled Reminder to run 1 hour before the Event. Scheduled reminders are executed manually.
Realised that the Event start date wouldn't cause the Scheduled Reminder to be sent.
Changed the Event start date to 2/10 12:48pm - which is 1 hour from now
Executed the Scheduled Reminder
The Event Start Date token returned the original Event Start Date, not the updated Event Start Date

Update 6/10 - Apparently this was indeed a bug, #21740

agileware-justin · 2021-10-02T02:05:46Z

Why are there now tokens called "Machine Name: ...". What is the reasoning there for this label? Can this be changed because we're going to have a pretty hard time explaining to end users what this is supposed to mean.

And have you noticed that there are now three tokens to refer to the Participant Role?
Machine Name: Participant Role
Participant Role
Participant Role ID

Do we really need the Participant Role ID token?
Can the two Machine Name: Participant Role and Participant Role tokens be resolved to just one?

One thing I have noticed in this token listing is that end users do find it very confusing to scroll through a long list of tokens which are totally not relevant for the current context, eg. case in point, if you are creating an Event Reminder, why list tokens that are not related at all to that context. Don't show tokens for Activity, Membership, Contribution entities. Additionally, some filtering of what tokens are actually useful could be applied.

eileenmcnaughton · 2021-10-02T02:16:46Z

@agileware-justin yeah - we are looking at being able to filter by audience in the tokenlist api once sorted - those things are useful when conditionals & such like are in play but not when it isn't - I have a hard stop on the token work for when the rc is cut on Wednesday so depending how well the other PRs I have progress through the system I hope we get to improving the token list functionality

colemanw · 2021-10-02T02:19:26Z

Thanks all for the thorough reviews. I think all the major issues have been addressed so let's merge this and continue to make improvements.

magnolia61 · 2021-10-02T07:20:12Z

Thank you all! 👍

eileenmcnaughton · 2021-10-02T07:27:09Z

thanks for your testing @magnolia61 - If you have the appetite for more I've started on adding participant tokens to pdf letters - #21695 - I haven't gotten test to pass there yet - maybe this time- but once they do that will be the last PR in the 'pdf strand' of this work - still more to do on the 'email strand'

magnolia61 · 2021-10-05T07:57:33Z

Just found something strange on participant custom date fields:
When the field has a value the scheduled reminder tokens render fine:

When the participant date value is empty, it is not only not shown (obviously), but causes no tokens at all to render:

I think this was not the case when I first tested this pr and is probably caused by another commit that has to do with dates and tokens.

Would it be an idea to also include event and participant custom tokens in the test? (textfield, datefield)

eileenmcnaughton · 2021-10-05T08:11:20Z

@magnolia61 if you find further things can you log a new issue in gitlab so we can track them

magnolia61 · 2021-10-05T08:18:02Z

https://lab.civicrm.org/dev/core/-/issues/2891

eileenmcnaughton · 2021-10-05T08:33:02Z

thanks!

Included in CiviCRM 5.43.0 PR: civicrm#21666

civibot bot added the master label Sep 29, 2021

eileenmcnaughton force-pushed the ev branch 3 times, most recently from 08af4a9 to b60f0e4 Compare September 29, 2021 18:48

eileenmcnaughton commented Sep 29, 2021

View reviewed changes

eileenmcnaughton force-pushed the ev branch 2 times, most recently from 9b17e30 to e65bf88 Compare September 29, 2021 19:00

Participant tokens on scheduled reminders (wip- upgrade script neede…

e80f226

…d at minimum)

eileenmcnaughton force-pushed the ev branch from e65bf88 to e80f226 Compare September 29, 2021 19:11

eileenmcnaughton changed the title ~~Participant tokens on scheduled reminders (wip- upgrade script needed at minimum)~~ Participant tokens on scheduled reminders Sep 29, 2021

eileenmcnaughton changed the title ~~Participant tokens on scheduled reminders~~ dev/core#348 Participant tokens on scheduled reminders Sep 29, 2021

eileenmcnaughton added the has-test label Oct 1, 2021

magnolia61 reviewed Oct 1, 2021

View reviewed changes

colemanw merged commit abf64eb into civicrm:master Oct 2, 2021

colemanw deleted the ev branch October 2, 2021 02:19

This was referenced Oct 6, 2021

Flush metadata cache on event edit #21740

Merged

Tokens - Hide more sophisticated tokens. Refactor to use "$this->tokenMetadata". #21706

Merged

olayiwola-compucorp added a commit to compucorp/civicrm-core that referenced this pull request Jan 6, 2022

WHF-220: Add support for participant tokens to scheduled reminders

8bf3c29

Included in CiviCRM 5.43.0 PR: civicrm#21666

olayiwola-compucorp added a commit to compucorp/civicrm-core that referenced this pull request Jan 6, 2022

WHF-220: Add support for participant tokens to scheduled reminders

d272c68

Included in CiviCRM 5.43.0 PR: civicrm#21666

olayiwola-compucorp added a commit to compucorp/civicrm-core that referenced this pull request Jan 6, 2022

WHF-220: Add support for participant tokens to scheduled reminders

c8e2b55

Included in CiviCRM 5.43.0 PR: civicrm#21666

olayiwola-compucorp mentioned this pull request Jan 6, 2022

WHF-220: Add support for participant tokens to scheduled reminders compucorp/civicrm-core#83

Merged

olayiwola-compucorp added a commit to compucorp/civicrm-core that referenced this pull request Jan 21, 2022

WHF-220: Add support for participant tokens to scheduled reminders

9c2c3b2

Included in CiviCRM 5.43.0 PR: civicrm#21666

dev/core#348 Participant tokens on scheduled reminders #21666

dev/core#348 Participant tokens on scheduled reminders #21666

Conversation

eileenmcnaughton commented Sep 29, 2021 • edited Loading

Overview

Before

After

Technical Details

Comments

civibot bot commented Sep 29, 2021

magnolia61 commented Sep 29, 2021

eileenmcnaughton Sep 29, 2021

Choose a reason for hiding this comment

eileenmcnaughton commented Sep 29, 2021

magnolia61 commented Sep 29, 2021 • edited Loading

eileenmcnaughton commented Sep 29, 2021

magnolia61 commented Sep 29, 2021 • edited Loading

magnolia61 commented Sep 29, 2021 • edited Loading

eileenmcnaughton commented Sep 29, 2021 • edited Loading

eileenmcnaughton commented Sep 29, 2021

magnolia61 commented Sep 30, 2021 • edited Loading

magnolia61 commented Sep 30, 2021 • edited Loading

eileenmcnaughton commented Sep 30, 2021

eileenmcnaughton commented Sep 30, 2021

agileware-justin commented Sep 30, 2021 • edited Loading

magnolia61 commented Oct 1, 2021

eileenmcnaughton commented Oct 1, 2021

magnolia61 commented Oct 1, 2021

eileenmcnaughton commented Oct 1, 2021

magnolia61 commented Oct 1, 2021

eileenmcnaughton commented Oct 1, 2021

magnolia61 Oct 1, 2021 • edited Loading

Choose a reason for hiding this comment

agileware-justin Oct 6, 2021

Choose a reason for hiding this comment

magnolia61 commented Oct 1, 2021

eileenmcnaughton commented Oct 1, 2021

magnolia61 commented Oct 1, 2021 • edited Loading

magnolia61 commented Oct 1, 2021

eileenmcnaughton commented Oct 1, 2021

magnolia61 commented Oct 1, 2021

eileenmcnaughton commented Oct 1, 2021

magnolia61 commented Oct 1, 2021 • edited Loading

agileware-justin commented Oct 2, 2021 • edited Loading

agileware-justin commented Oct 2, 2021

eileenmcnaughton commented Oct 2, 2021

colemanw commented Oct 2, 2021

magnolia61 commented Oct 2, 2021

eileenmcnaughton commented Oct 2, 2021

magnolia61 commented Oct 5, 2021 • edited Loading

eileenmcnaughton commented Oct 5, 2021

magnolia61 commented Oct 5, 2021

eileenmcnaughton commented Oct 5, 2021

eileenmcnaughton commented Sep 29, 2021 •

edited

Loading

magnolia61 commented Sep 29, 2021 •

edited

Loading

magnolia61 commented Sep 29, 2021 •

edited

Loading

magnolia61 commented Sep 29, 2021 •

edited

Loading

eileenmcnaughton commented Sep 29, 2021 •

edited

Loading

magnolia61 commented Sep 30, 2021 •

edited

Loading

magnolia61 commented Sep 30, 2021 •

edited

Loading

agileware-justin commented Sep 30, 2021 •

edited

Loading

magnolia61 Oct 1, 2021 •

edited

Loading

magnolia61 commented Oct 1, 2021 •

edited

Loading

magnolia61 commented Oct 1, 2021 •

edited

Loading

agileware-justin commented Oct 2, 2021 •

edited

Loading

magnolia61 commented Oct 5, 2021 •

edited

Loading