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

CiviCRM Scheduled Reminders, adds support for Participant fields and Participant Custom Fields as Tokens in Scheduled Reminders #20335

Closed
wants to merge 2 commits into from

Conversation

agileware-justin
Copy link
Contributor

@agileware-justin agileware-justin commented May 18, 2021

Overview

Adds support for Participant fields and Participant Custom Fields as Tokens in Scheduled Reminders.

Solves this Gitlab Feature Request, Custom Participant tokens not working in scheduled reminders
https://lab.civicrm.org/dev/core/-/issues/348

Before

Cannot use Participant fields or Participant Custom Fields as Tokens in Scheduled Reminders, which is sad 😢

After

Now able to use Participant fields or Participant Custom Fields as Tokens in Scheduled Reminders 😀

Technical Details

Includes commits from PR #20334 as the two are dependant on a change to the getCustomFieldTokens function.

Comments

Ping @mattwire

Agileware Ref: CIVICRM-1731

@civibot
Copy link

civibot bot commented May 18, 2021

(Standard links)

@civibot civibot bot added the master label May 18, 2021
@agileware-justin agileware-justin force-pushed the CIVICRM-1731 branch 2 times, most recently from 4974054 to aca31c5 Compare May 18, 2021 03:02
@eileenmcnaughton
Copy link
Contributor

@totten probably best if you review this one once you get to that part of the message template work - it does extend our TokenProcessor surface & I want to be really clear about our test cover and about how we want to represent pseudoconstants etc before we extend our surface area on this

@magnolia61
Copy link
Contributor

magnolia61 commented May 19, 2021

Thanks a lot for submitting this PR :-) I opened up an issue about this exact matter a while ago: https://lab.civicrm.org/dev/core/-/issues/348

I have been successful in testing this wonderful pr, except for custom date fields.
When I include a custom participant date field token in a reminder mail, it will stop all tokens from being processed.
Update: A custom event date field does work as expected. It's only the participant date fields.

Also the participant ID would be very very helpful if possible (for constructing links etc)

If you need help with further testing, I would be glad to do so. Cheers, Richard

@agileware-justin
Copy link
Contributor Author

@magnolia61 glad you appreciate the work and thanks for testing too 😃

Update: A custom event date field does work as expected. It's only the participant date fields.

I've tested this by creating two additional date fields in the Participant, custom field set. This is working when I tested this using CiviCRM 5.37.

Would you mind providing a few screenshots:

  1. Custom field configuration
  2. Scheduled reminder template
  3. Email or Activity that was generated

My test results are below.

  1. Custom field configuration

Screenshot_20210520_171406

  1. Scheduled reminder template

Screenshot_20210520_171728

  1. Activity that was generated

Screenshot_20210520_171443

@agileware-justin
Copy link
Contributor Author

Also the participant ID would be very very helpful if possible (for constructing links etc)

@magnolia61 and I've added the Participant ID token, too. You're right this is useful.

… available as tokens in Scheduled Reminders. Add implementation to TokenProcessor
@agileware-justin
Copy link
Contributor Author

@magnolia61 ready for testing again :)

@totten
Copy link
Member

totten commented May 20, 2021

I was chatting a bit with @eileenmcnaughton about this. (You may remember she'd spent a chunk of time on testing/comparing tokens for #19806.) She raised the point that we ought to have some test-coverage.

It can be a bit daunting to write a first test for this, but I think there's a good example at https://github.com/civicrm/civicrm-core/blob/5.38/tests/phpunit/CRM/Mailing/TokensTest.php#L6. It's really just the two functions -- getExampleTokens() and testTokensWithMailingId().

@agileware-justin
Copy link
Contributor Author

@totten

(You may remember she'd spent a chunk of time on testing/comparing tokens for #19806.) She raised the point that we ought to have some test-coverage.

Not on my radar at all, thanks for pointing out

It's really just the two functions -- getExampleTokens() and testTokensWithMailingId().

Thanks for the tip. Will see what we can do here. Might be worthwhile to submit that as a follow-up PR because this one is a bit of a PITA to update already each time.

@MegaphoneJon
Copy link
Contributor

Scheduled Reminders can also go to an "Additional Contact", and those contacts don't have to have a participant record. When doing manual testing, we should make sure that participant tokens handle the lack of a participant gracefully (which I think means returning NULL).

@agileware-justin
Copy link
Contributor Author

@magnolia61 or @MegaphoneJon would either of you have time to do a follow-up PR with the unit tests for this PR?

@petednz
Copy link

petednz commented May 27, 2021

or @jitendrapurohit

@agileware-justin
Copy link
Contributor Author

Thanks @petednz

@magnolia61
Copy link
Contributor

I am (technically) not able to write a test. @petednz are you suggesting that @jitendrapurohit would be able to do so?

@petednz
Copy link

petednz commented May 31, 2021

technically he can, in terms of when he can get to it with our current workload is an open-ended question. it is not high on our list, just there as 'help the community' task when we can

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.

6 participants