-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Validation problem in term_translation_update_many #1033
Conversation
Now checks that data is present and that it is a list.
Now checks that data is present and that it is a list.
{'error': | ||
'term_translation_update_many needs to have a list of dicts in field data'} | ||
{'error': ['term_translation_update_many needs to have a ' | ||
'list of dicts in field data']} |
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.
do we want to change this from a string to a list?
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.
The API was not correctly returning the error message when this was a string (just the first letter). This should probably have a proper schema and go through the usual validation process at some point.
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.
cool
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.
@tobes Sorry my mistake: my original change was made in 1.8.1 but this seems to have been something that was fixed since then (str now uses ValidationError.error_dict instead of ValidationError.error_summary), so this can go back to being a string. I will push a fix.
…summary so this can go back to being a string.
…summary so this can go back to being a string.
@tobes Cheers |
If no data is given then num is not defined here:
https://github.com/okfn/ckan/blob/master/ckan/logic/action/update.py#L776
This should be caught here, but for some reason we are also checking to see if data_dict is a list:
https://github.com/okfn/ckan/blob/master/ckan/logic/action/update.py#L768