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

Serializer fields with required=False should be added to model validation exclusions. #1257

Closed
variable opened this issue Dec 2, 2013 · 17 comments
Labels

Comments

@variable
Copy link

variable commented Dec 2, 2013

I explained the problem in Stackoverflow http://stackoverflow.com/questions/19780731/django-cms-framework-serializer-field-required-false

I propose to make the change to truly respect the required=False parameter in serializer fields.

@tomchristie
Copy link
Member

Okay looks like we should make sure get_validation_exclusions automatically adds those fields in the same way that kevin stone recommends.

@variable
Copy link
Author

variable commented Dec 3, 2013

yah :D

Also, would it be possible, instead of manually define the fields just for the sake of setting required=False, define the non-required fields in a list?

@StErMi
Copy link

StErMi commented Jan 23, 2014

Yes I think that @variable is right also on the list of required (or not_required) fields

@kot-behemoth
Copy link

It would be great to see this issue addressed, as it's somewhat non-obvious behaviour if you follow the documentation!

@tomchristie
Copy link
Member

Happy to see it tackled. Add a test case, add a fix, submit a pull request, and it oughta sail on right through.

@kot-behemoth
Copy link

Sorry, I'm not sure I phrased it correctly — what I meant is I would appreciate this issue getting some attention, as I'm currently working around this problem in my code.

I would like to try and figure out how to fix the issue as a pull request, but unfortunately I don't have enough time just now, plus I'm not sure my python level is up to scratch for such a project yet.

carltongibson pushed a commit to carltongibson/django-rest-framework that referenced this issue Feb 11, 2014
carltongibson pushed a commit that referenced this issue Feb 11, 2014
@carltongibson
Copy link
Collaborator

Closed by 9a767c2

@tomchristie
Copy link
Member

Thx @carltongibson, rockin! 🌟

@xordoquy
Copy link
Collaborator

xordoquy commented Mar 6, 2014

Unfortunately, this change breaks the unique constraint management.
I now have IntegrityError instead of a nice message.

@carltongibson
Copy link
Collaborator

Hmmm. Ok. That sounds like a regression. @xordoquy Any chance you can knock up a failing test case for that?

@xordoquy
Copy link
Collaborator

xordoquy commented Mar 6, 2014

sure, I'm already working on it ;)

@alanwj
Copy link

alanwj commented Apr 5, 2014

This bug fix should warrant a new release as it also fixes a rather major security hole.

Because partial=True causes all fields to have field.required=False, no model fields are ever validated during a PATCH.

@procmail
Copy link

procmail commented Aug 7, 2014

It seems that having required=False is sufficient when using curl to submit data, but when accessing via the web API interface and then submitting via there, it's still necessary to use get_validation_exclusions() to add the non-required field in.

@darrenbates
Copy link

This bug is back in the latest version.

@xordoquy
Copy link
Collaborator

xordoquy commented May 7, 2017

@darrenbates code has changed a lot since. If you have an issue feel free to open a new bug with a failing test case to demonstrate the problem.

@yellowcap
Copy link

Just for reference: the required and allow_blank argument seems to have no effect on ModelSerializer fields, if unique_together constraints are set on the corresponding model, see #4456

@g-rd
Copy link

g-rd commented Feb 27, 2018

ip = serializers.IPAddressField(protocol='both', required=False, allow_blank=True)
also set on model and I get:
"ip": [
"This field may not be null."
],
Any ideas ?

Meta:
djangorestframework==3.7.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests