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

Allow multiple registrations from search actions #26303

Conversation

larssandergreen
Copy link
Contributor

@larssandergreen larssandergreen commented May 22, 2023

Overview

If Allow same email and multiple registrations? is enabled for an event, contacts can be registered for the same event more than once. This is currently possible from the front end and from Register Event Participant and Add Event Registration, but not when using Register participants for event from search actions.

Before

When attempting to register a contact for an event, the contact is not registered again and a message pops up to tell you N contacts were already registered.

After

Now if Allow same email and multiple registrations? is enabled, contacts are registered as usual even if they are already registered for the event.

@civibot
Copy link

civibot bot commented May 22, 2023

(Standard links)

@civibot civibot bot added the master label May 22, 2023
@eileenmcnaughton
Copy link
Contributor

Yay - this is in the part of the form that is separated out from the parent class - way less scary!

I wonder if \Civi\Api4\Event::get() needs to be \Civi\Api4\Event::get(FALSE)

@larssandergreen
Copy link
Contributor Author

@eileenmcnaughton Presumably you shouldn't have been able to select an event that you don't have permission to access, so couldn't be trying to register people for an event you don't have access to, but I'm happy to add a FALSE if you think it is less likely to cause problems.

@eileenmcnaughton
Copy link
Contributor

@larssandergreen just trying to think through the user experience of this - it feels possible people are used to using this action 'safe in the knowledge' the contacts won't be double registered & quietly double registering them might actually be unexpected. Perhaps on validate we should check & then ? something - a check box? a confirm dialog to check its what they meant

Also - when I tried the register form in testing I hit notices / undeclared properties. That, combined with this makes me think we should do something along the lines of

#26305

to split out the 2 forms validations

(I haven't given that a spin yet - I suspect this block should be outside the other IFs https://github.com/civicrm/civicrm-core/pull/26305/files#diff-bf1322d204b564c54597cb3cea15d8cd1479303297fab55c9eb6d20eb4e05e9bR175-R177 )

@eileenmcnaughton
Copy link
Contributor

Yeah re permissions we can probably get away with the implicit true (famous last words)

@larssandergreen
Copy link
Contributor Author

@eileenmcnaughton Maybe it would be enough to just report to the user something like: N contacts were already registered for this event, but have been registered again. Then they'll know for next time, at least. But either way, I feel like this is a fairly rare case (my thought with this, besides just being consistent, was if you wanted to register participants for a bunch of price set options and didn't want to go in and edit every registration, this is how you would do it, even though you end up with double registrations, which may or may not matter). But if you think this is more likely to be a problem than helpful, we can just leave it as is.

@eileenmcnaughton
Copy link
Contributor

@larssandergreen I've been meaning to get to doing the next dev-digest so I'll include this in there

@Stoob
Copy link
Contributor

Stoob commented Jun 17, 2023

If the event has 'multiple registrations' turn on (which is off by default) then the user wants to be able to register folks more than once and it's OK to allow that to happen. But I agree with Lars that the popup notification should be changed.

@larssandergreen larssandergreen force-pushed the Allow-multiple-registrations-from-search-actions branch from a60ec7b to 8e19e29 Compare June 19, 2023 21:43
@larssandergreen
Copy link
Contributor Author

I have added an alert when this happens: "%1 contacts were already registered for this event, but have been added a second time."

@wmortada
Copy link
Contributor

This change makes sense to me.

@eileenmcnaughton
Copy link
Contributor

Just coming back to this & noting support from @Stoob & @wmortada with the message change - merging

@eileenmcnaughton eileenmcnaughton merged commit 809c6bb into civicrm:master Aug 3, 2023
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.

4 participants