-
Notifications
You must be signed in to change notification settings - Fork 411
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
Improved error handling to gracefully manage invalid inputs #6280
base: master
Are you sure you want to change the base?
Conversation
b1fc2d4
to
b22ae37
Compare
@@ -125,7 +125,7 @@ def view_data(self): | |||
|
|||
def get_validators(self, existing_registration): | |||
def _check_number_of_places(new_data): | |||
if not new_data: | |||
if not new_data or not hasattr(new_data, '__iter__'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed, the marshmallow validation only accepts dicts... on your instance you're still on the old regform, which accepts any valid JSON
marshmallow handles this in v3.2+
b22ae37
to
da804de
Compare
places_used_dict[k] += new_data[k] | ||
try: | ||
places_used_dict[k] += new_data[k] | ||
except TypeError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I don't see how this can happen in v3.2+ since new_data
is always guaranteed to be dict mapping strings to ints, and any k
in here is actually one that's guaranteed to be a valid choice and not some random nonsense comint from the user.
FWIW this is the error I get when sending File "/home/adrian/dev/indico/py3/src/indico/modules/events/registration/util.py", line 487, in modify_registration
new_data = snapshot_registration_data(registration)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/adrian/dev/indico/py3/src/indico/modules/events/registration/util.py", line 951, in snapshot_registration_data
'friendly_data': regdata.friendly_data}
^^^^^^^^^^^^^^^^^^^^^
File "/home/adrian/dev/indico/py3/src/indico/modules/events/registration/models/registrations.py", line 829, in friendly_data
return self.get_friendly_data()
^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/adrian/dev/indico/py3/src/indico/modules/events/registration/models/registrations.py", line 836, in get_friendly_data
return self.field_data.field.get_friendly_data(self, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/adrian/dev/indico/py3/src/indico/modules/events/registration/models/form_fields.py", line 100, in get_friendly_data
return self.field_impl.get_friendly_data(registration_data, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/adrian/dev/indico/py3/src/indico/modules/events/registration/fields/choices.py", line 237, in get_friendly_data
caption = registration_data.field_data.field.data['captions'][uuid]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
KeyError: 'lol' So there are indeed cases that need to be fixed. In fact, we should probably fail validation earlier when there's data that's not a valid choice (careful so it still works in case of a multiselect field where one of the already-selected choices has been removed, since you can still have and keep it in an existing registration). |
No description provided.