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

Event registration activity target #25650

Conversation

semseysandor
Copy link
Contributor

Overview

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

Before

Event Registration activity target is empty when registering through online form.

After

Event Registration activity target is the contact who registered (same as source).

Comments

Removing duplicate target_contact_id in CRM_Activity_BAO_Activity::addActivity() might be unneeded as according to my tests it works with duplicate ids also, it just seemed the right thing to do... 😄

@civibot
Copy link

civibot bot commented Feb 23, 2023

(Standard links)

@highfalutin
Copy link
Contributor

r-run: pass

Control:

  • On master, I verified that self-registration for an event via the public form, not logged in, results in an activity without a target.

Test:

  • With the PR, the same method of registration results in an activity with two activity_contacts: source and target are both the contact who registered for the event.
  • Likewise, when I register multiple contacts through the public form, the resulting activity for each contact has that participant as both the target and source. This bit has not been discussed but I think it's ok. One could argue that the "main" contact should be the source on all the activities, as that person is presumably the one filling out the form on behalf of all the others. But that would alter the existing contract.

@eileenmcnaughton
Copy link
Contributor

Thanks @highfalutin - re

"One could argue that the "main" contact should be the source on all the activities, as that person is presumably the one filling out the form on behalf of all the others. But that would alter the existing contract."

I agree that change of contract makes sense - although it is not changed by this PR so non-blocking on merging this. I'm merging this based on your imput/ testing & of @aydun

@eileenmcnaughton eileenmcnaughton merged commit 0f3ffa0 into civicrm:master Feb 23, 2023
@semseysandor semseysandor deleted the event-registration-activity-target branch April 24, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants