-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
Populate Template Params for Offline Event Confirmation Receipts #22878
Closed
Closed
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alifrumin Here's a fun bit since v5.42 - the
sendTemplate()
function accepts an argumenttokenContext
, as in:The upshot of using
tokenContext
is:{event.description}
or{participant.status_id:label}
) which match tokens used elsewhere (eg Scheduled Reminders).CRM_Event_BAO_Event::retrieve
-- it will autoload data that is actually used.Go to {event.title} at {event.location}
, then it effectively runsSELECT title, location FROM civicrm_event WHERE id = $eventId
)(There's another possible level to this - where we declare that
event_offline_receipt
expects parameters X,Y,Z. That makes it easier to provide admin tools - eg token-pickers and previews. I don't know if that's in-scope for what you're trying to address.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let @alifrumin confirm but I did want to comment on this that I think it's not about the message template it's about alterMailParams. This is similar to the earlier ticket regarding contact id. In hook_alterMailParams, you don't have access to anything except what is passed to Mail::send(), which is really only guaranteed to be the message text and the recipient email address (which might match on several contacts so isn't an identifier), and you don't have tokenContext available even when it's from a message template (unless I'm missing how to get at it).
But altermailparams can be called from so many different paths, not just message templates, and so what variables make sense at any given time is going to be different (it may not even have a related contact id). So it would be nice to handle in a clean and documented/unit-tested way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the list of fields (
contactId
vscontactId+eventId
vstplParams['event']['title']
) represents a contract that should (ideally) be documented. That contract impacts a few different parties - eg (a) the code that firesevent_offline_receipt
, (b) the admin editingevent_offline_receipt
, (c) any extensions that hook intoevent_offline_receipt
, and (d) any tests or previews involvingevent_offline_receipt
.If one just wants to make the
{event.*}
tokens functional, then it's not strictly necessary to define the contract. You just need to pass along$sendTemplateParams['tokenContext']['eventId']
.But if we're going to document the contract and use it for hooks, then I'd suggest two more pieces:
CRM_Event_WorkflowMessage_OfflineReceipt
. This would look a lot likeCRM_Contribute_WorkflowMessage_ContributionOfflineReceipt
which describescontribution_offline_receipt
. (Also, there's some draft docs in https://lab.civicrm.org/documentation/docs/dev/-/merge_requests/987.)hook_alterMailParams
which gives the message as an object. Here's a sketch for firing+consuming hook_alterWorkflowMessage (gist).