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

Use a marshmallow schema for regfom submissions #18

Merged

Conversation

tomasr8
Copy link

@tomasr8 tomasr8 commented Jan 21, 2022

Adds mm_field_class and mm_field_kwargs to RegistrationFormFieldBase
which are used to dynamically construct the regform schema.

What works:

  • regform submission (normal and from management)
  • Incorporating field validators into the marshmallow schema

What doesn't work yet:

  • Modifying existing registrations

@ThiefMaster
Copy link
Owner

Modifying existing registrations

not implemented anywhere in the new react code, so i'd just ignore that for now

@ThiefMaster
Copy link
Owner

ThiefMaster commented Jan 21, 2022

Incorporating field validators into the marshmallow schema (including csrf_token)

csrf_token can go away, it's not needed. we have global csrf checks for all requests that aren't GET

for field validation you could just pass validate=... when defining the field itself; no need to add methods to the class dynamically

@tomasr8 tomasr8 force-pushed the mm-regform-validation branch 2 times, most recently from a69e832 to baa6041 Compare January 24, 2022 12:34
indico/modules/events/registration/util.py Outdated Show resolved Hide resolved
indico/modules/events/registration/util.py Outdated Show resolved Hide resolved
indico/modules/events/registration/fields/base.py Outdated Show resolved Hide resolved
indico/modules/events/registration/fields/base.py Outdated Show resolved Hide resolved
indico/modules/events/registration/util.py Outdated Show resolved Hide resolved
# Some field validators require access to the previous registration
schema[form_item.html_field_name] = field_impl.create_mm_field(registration=registration)

# TODO: what to do with signal -> signals.event.registration_form_wtform_created
Copy link
Owner

Choose a reason for hiding this comment

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

Check indico#4642 for the rationale why it was added - so I think in the end we should probably provide a similar signal that has access to the schema in order to modify that one if needed. But I think we could consider getting rid of it; if the UN folks (cc @OmeGak) still need it when adapting to the react regform (I'd still like to provide some way for plugins to register new regform field types!) we can see what's the best way of restoring it (or let them send another PR)

Copy link

Choose a reason for hiding this comment

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

We are, indeed, still needing this signal (or something equivalent). You can see here how we are using it:
https://github.com/UNOG-Indico/indico-un/blob/master/un/modules/registration/forms.py#L60-L61

@ThiefMaster ThiefMaster merged commit 6e3e532 into ThiefMaster:regform-submission-react Jan 27, 2022
@ThiefMaster ThiefMaster deleted the mm-regform-validation branch January 27, 2022 13:29
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.

3 participants