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 registrants to withdraw their registration #4585

Merged
merged 4 commits into from Aug 10, 2020

Conversation

OmeGak
Copy link
Member

@OmeGak OmeGak commented Aug 5, 2020

Closes: #2715
Supersedes and thus closes: #4166

Replying to @ThiefMaster's comment on the superseded PR:

Should we check for the modification period here at all? I think withdrawal should always be possible, especially if no payment is involved. After paying it does make sense that you need to contact the organizer though (regardless of the modification setting) - and they should then have a button in the management view to mark the registration as withdrawn.

The logic I have applied to allow withdrawals:

  • The registration has not been paid.
  • The event the registration is associated to has not finished yet.

Regarding allowing managers to undo withdrawals, this is now possible via the "Reset registration" button in the registration management view.

Screenshots

image
image

@ThiefMaster
Copy link
Member

ThiefMaster commented Aug 5, 2020

How does it currently work for events where the registration feature is used solely for participants, i.e. the registration was never open/scheduled and no modification is allowed (and the event's type is not conference?).

I think in such a case it doesn't make much sense to allow withdrawing, especially since you'd only be able to get there by using the url to the registrations directly (in case of a meeting)...

@OmeGak OmeGak force-pushed the wip/reg-withdraw branch 3 times, most recently from 2d7ad63 to 5c67206 Compare Aug 6, 2020
@OmeGak
Copy link
Member Author

OmeGak commented Aug 6, 2020

How does it currently work for events where the registration feature is used solely for participants, i.e. the registration was never open/scheduled and no modification is allowed.

Same as with conferences. If the registration is not paid and it's before the end of the event, the withdraw button will be visible. Modifications being allowed and regform being open/scheduled are not taken into account to allow or disallow withdrawals.

I think in such a case it doesn't make much sense to allow withdrawing, especially since you'd only be able to get there by using the url to the registrations directly (in case of a meeting)...

I would not disallow this. Someone may want to update their RSVP to a meeting after having accepted an invitation.

@ThiefMaster
Copy link
Member

ThiefMaster commented Aug 6, 2020

I was more thinking about cases like our weekly meeting where we added everyone as participants but do not use the user-facing registration module at all. People withdrawing there would also mess up clones of the event where they'd be withdrawn from the beginning...

@OmeGak
Copy link
Member Author

OmeGak commented Aug 6, 2020

While testing, I found that inviting existing Indico users to a registration is currently broken.

Traceback (most recent call last):
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/web/flask/util.py", line 83, in wrapper
    return obj().process()
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/web/rh.py", line 275, in process
    res = self._do_process()
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/web/rh.py", line 245, in _do_process
    rv = self._process()
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/modules/events/registration/controllers/management/invitations.py", line 70, in _process
    if form.validate_on_submit():
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/flask_wtf/form.py", line 100, in validate_on_submit
    return self.is_submitted() and self.validate()
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/web/forms/base.py", line 141, in validate
    valid = super(IndicoForm, self).validate()
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/wtforms/form.py", line 318, in validate
    return super(Form, self).validate(extra)
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/wtforms/form.py", line 150, in validate
    if not field.validate(self, extra):
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/wtforms/fields/core.py", line 226, in validate
    stop_validation = self._run_validation_chain(form, chain)
  File "/Users/omegak/Code/orgs/indico/indico-un/.venv/lib/python2.7/site-packages/wtforms/fields/core.py", line 246, in _run_validation_chain
    validator(form, self)
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/modules/events/registration/forms.py", line 191, in validate_users_field
    emails = {x['email'].lower() for x in field.data}
  File "/Users/omegak/Code/orgs/indico/indico-un/indico/indico/modules/events/registration/forms.py", line 191, in <setcomp>
    emails = {x['email'].lower() for x in field.data}
TypeError: 'User' object has no attribute '__getitem__'

This may be due to recent changes in PrincipalListField. I've added a commit in this PR that solves it, but I don't know if it's not taking into account some of the possible return values of principal_from_identifier in PrincipalListField._convert_principal. Could you take a look and confirm? Also, let me know if you want me to add this bug fix in CHANGES.rst.

@OmeGak
Copy link
Member Author

OmeGak commented Aug 6, 2020

I was more thinking about cases like our weekly meeting where we added everyone as participants but do not use the user-facing registration module at all. People withdrawing there would also mess up clones of the event where they'd be withdrawn from the beginning...

This sounds very specific to you workflow and this makes me think that perhaps it should be configurable. I would apply the same logic as for hiding/showing the "Modify" button and let managers configure until when withdrawals will be allowed with "modification deadline".

Hard-coding end-of-event as last possible moment to withdraw is currently not documented and conference managers will have no way to control corner cases like the one you are describing.

Let me know what you think. If you agree I will change the logic.

@ThiefMaster
Copy link
Member

ThiefMaster commented Aug 6, 2020

I cherrypicked the invitation fix into master.

Also, let me know if you want me to add this bug fix in CHANGES.rst.

No, this bug was introduced in 2.3, so it's not part of any released version.

but I don't know if it's not taking into account some of the possible return values

The way the field is configured, it only returns User objects. so everything fine

@ThiefMaster
Copy link
Member

ThiefMaster commented Aug 6, 2020

This sounds very specific to you workflow

It's actually the vast majority for meetings (event type 2). People add participants (like they did with the legacy "participants" feature in 1.x) but do not use any actual registration features.

indico_prod=> select e.type, count(*) from event_registration.forms rf join events.events e on (e.id = rf.event_id) where not e.is_deleted and not rf.is_deleted and not (rf.start_dt is null and rf.end_dt is null and rf.modification_mode = 3) and exists (select 1 from event_registration.registrations r where r.id = rf.id) group by e.type;
 type | count
------+-------
    1 |   284
    2 |  5443
    3 |  7442
(3 rows)

indico_prod=> select e.type, count(*) from event_registration.forms rf join events.events e on (e.id = rf.event_id) where not e.is_deleted and not rf.is_deleted and (rf.start_dt is null and rf.end_dt is null and rf.modification_mode = 3) and exists (select 1 from event_registration.registrations r where r.id = rf.id
) group by e.type;
 type | count
------+-------
    1 |   116
    2 | 38385
    3 |    53
(3 rows)

We could add a new column to control the withdrawal mode and set a reasonable default depending on the event type (not allowed for meetings, allowed for the other event types).

@OmeGak
Copy link
Member Author

OmeGak commented Aug 6, 2020

We could add a new column to control the withdrawal mode and set a reasonable default depending on the event type (not allowed for meetings, allowed for the other event types).

Isn't this already the case for allowing modifications and modification deadline? Modifications are enabled by default in conferences and disabled by default in lectures and meetings. Then, it's unlikely that conference organizers expect registrants to withdraw when there is a modification deadline date, so we can also use modification_end_dt for that.

In the end, I've gone for the following logic:

@property
def can_be_withdrawn(self):
    from indico.modules.events.registration.models.forms import ModificationMode
    if self.is_paid:
        return False
    elif self.event.end_dt < now_utc():
        return False
    elif self.registration_form.modification_mode == ModificationMode.not_allowed:
        return False
    elif (self.registration_form.modification_end_dt < now_utc()
            if self.registration_form.modification_end_dt else False):
        return False
    else:
        return True

@ThiefMaster ThiefMaster changed the title Allow registrants withdraw their registration Allow registrants to withdraw their registration Aug 10, 2020
@ThiefMaster ThiefMaster force-pushed the wip/reg-withdraw branch 2 times, most recently from 7cf145d to 97689bf Compare Aug 10, 2020
@ThiefMaster ThiefMaster merged commit e3b756a into indico:master Aug 10, 2020
4 checks passed
@ThiefMaster ThiefMaster deleted the wip/reg-withdraw branch Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Registration Form: allow users to cancel their registration
2 participants