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

Add disabling of declared fields on serializer subclasses #4764

Merged
merged 5 commits into from Jan 3, 2017

Conversation

rpkilby
Copy link
Member

@rpkilby rpkilby commented Dec 23, 2016

This corresponds to a similar PR for django-filter. In short, the django form API supports a syntax for disabling fields on child forms (django ticket), but this does not work on serializers.

Example:

class Parent(forms.Form):
     field = forms.CharField()

class Child(Parent)
    field = None

@rpkilby
Copy link
Member Author

rpkilby commented Dec 23, 2016

Side note - some of the CI jobs for the first commit passed because I pushed the second commit while the whole build was running. This is related to travis-ci/travis-ci#6337

@tomchristie
Copy link
Member

Nice change! We'll want to add some docs corresponding to this, perhaps in a subsection underneath this?... http://www.django-rest-framework.org/api-guide/serializers/#advanced-serializer-usage

@tomchristie tomchristie added this to the 3.5.4 Release milestone Jan 1, 2017
@rpkilby
Copy link
Member Author

rpkilby commented Jan 3, 2017

I noticed that there wasn't a section that generally covered serializer inheritance, so I kind of rolled the docs request into the existing ModelSerializer.Meta inheritance docs, and then pulled relevant content from the ModelForm inheritance docs. The end result looks pretty similar.

  • Not sure if a full serializer inheritance section is necessary. I can cut this down to just the note on declared field disabling.
  • Is 478fe0a superfluous or is it useful to keep? Should it be moved to the model serializer tests, or is it small enough to keep it where it is in the general serializer tests?

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

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

Looks great. An example in the docs and I think this is fully cooked.

@@ -1025,6 +1015,32 @@ If any of the validation fails, then the method should raise a `serializers.Vali

The `data` argument passed to this method will normally be the value of `request.data`, so the datatype it provides will depend on the parser classes you have configured for your API.

## Serializer Inheritance
Copy link
Member

Choose a reason for hiding this comment

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

I like the extra section here - the notes on Meta are worth it.

Additionally, the following caveats apply to serializer inheritance:

* Normal Python name resolution rules apply. If you have multiple base classes that declare a `Meta` inner class, only the first one will be used. This means the child’s `Meta`, if it exists, otherwise the `Meta` of the first parent, etc.
* It’s possible to declaratively remove a `Field` inherited from a parent class by setting the name to be `None` on the subclass. You can only use this technique to opt out from a field defined declaratively by a parent class; it won’t prevent the `ModelSerializer` from generating a default field. To opt-out from default fields, see [Specifying which fields to include](#specifying-which-fields-to-include).
Copy link
Member

Choose a reason for hiding this comment

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

Let's include an example for this.

@tomchristie
Copy link
Member

Fabulous!

@tomchristie tomchristie merged commit 11fd3bf into encode:master Jan 3, 2017
@rpkilby rpkilby deleted the child-disable-field branch January 3, 2017 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants