-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: add custom form json column to sessions, speaker, attendees #6435
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
Conversation
Codecov Report
@@ Coverage Diff @@
## development #6435 +/- ##
===============================================
+ Coverage 64.51% 64.52% +<.01%
===============================================
Files 291 292 +1
Lines 15117 15147 +30
===============================================
+ Hits 9753 9773 +20
- Misses 5364 5374 +10
Continue to review full report at Codecov.
|
|
This will allow users to add arbritrarily nested JSON to the DB. Please add validation for only flat JSON |
|
@iamareebjamal Added a validation check to only allow flattened JSON. Verified locally by passing a two-level deep JSON object. |
app/api/schema/speakers.py
Outdated
| @validates_schema(pass_original=True) | ||
| def validate_json(self, data, original_data): | ||
| if data.get('complex_field_values'): | ||
| if any(isinstance(i, dict) for i in data['complex_field_values'].values()): |
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.
{
"a": [{... big nested data ...}, {... big nested data ...}, {... big nested data ...}]
}
This will be supported
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.
Also, let's add a limit to the fields in settings. By default, allow only 30 items. I think it is fair. If more are required, then it can be changed by the admin
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.
{ "a": [{... big nested data ...}, {... big nested data ...}, {... big nested data ...}] }This will be supported
@iamareebjamal for this can we impose that the value has to be a string or number or null, that will eliminate the possibility of a list or other unsupported types.
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.
Agree. string, number, null, or boolean
cefdc65 to
f1d74d3
Compare
|
@iamareebjamal Please take a look now. |
Fixes #6434
Fixes #6191
Short description of what this resolves:
Adds fields to accommodate complex custom form values in sessions, speakers and attendees.