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

dev/core#3778 Add code hook to set civicrm_participant.created_id #24304

Merged
merged 2 commits into from Sep 5, 2022

Conversation

mattwire
Copy link
Contributor

Overview

This extracts the part of #23936 that populates the created_id field.

Before

civicrm_participant.created_id not populated.

After

civicrm_participant.created_id populated.

Technical Details

Set the "created_id" field.
The created_id should always be the person that actually did the registration.
That might be the first participant, but it might be someone registering someone without registering themselves.

  1. Prefer logged in contact id
  2. Fall back to 'registered_by_id' param.
  3. Fall back to participant contact_id (for anonymous person registering themselves)

Comments

@civibot
Copy link

civibot bot commented Aug 18, 2022

(Standard links)

@civibot civibot bot added the master label Aug 18, 2022
@mattwire mattwire changed the title Add code hook to set civicrm_participant.created_id dev/core#3778 Add code hook to set civicrm_participant.created_id Aug 18, 2022
@mattwire mattwire marked this pull request as ready for review August 18, 2022 21:00
* @throws CRM_Core_Exception
*/
public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) {
if ($event->entity === 'Participant' && $event->action === 'create' && !array_key_exists('created_id', $event->params)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good but perhaps this should be !empty since I don't think we are encouraging NULL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton Makes sense. I've updated the PR to check if the field is empty() so that we set the field if it has not been set, or is set to NULL.

// 2. Fall back to 'registered_by_id' param.
// 3. Fall back to participant contact_id (for anonymous person registering themselves)
$event->params['created_id'] = CRM_Core_Session::getLoggedInContactID();
if (empty($event->params['created_id'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is redundant now - same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton No. Because we probably set it in the line above to the logged in contact ID (step 1). The other two conditions are only triggered if the created_id is still empty

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok right

@eileenmcnaughton
Copy link
Contributor

I think this is mergeable with just one minor further fix - the empty() check is the same in the main if as the later if so it is redundant

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Aug 19, 2022
@colemanw
Copy link
Member

Ideally the unit test would cover all 3 scenarios (get from logged in user, get from registered_by_id, and explicitly provided value passed in).

@seamuslee001 seamuslee001 merged commit f7e48e9 into civicrm:master Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
4 participants