Skip to content

Docs about default value for dotted source, additional tests #5489

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 16, 2017
Merged

Docs about default value for dotted source, additional tests #5489

merged 2 commits into from
Oct 16, 2017

Conversation

rpkilby
Copy link
Member

@rpkilby rpkilby commented Oct 9, 2017

WIP changes to docs, and additional tests for behavior of source w/ dotted notation.

Closes #5488.

Waiting for a nested ModelSerializer example from @xordoquy.

Also, is the current exception sufficient, or do we need to mention the necessity of a default value?
eg, this serializer:

class ColleagueUserSerializer(BaseModelSerializer):
    city = serializers.IntegerField(source='profile.contact.address.city')

generates this exception:

AttributeError: Got AttributeError when attempting to get a value for field `city` on serializer `ColleagueUserSerializer`.
The serializer field might be named incorrectly and not match any attribute or key on the `User` instance.
Original exception text was: 'NoneType' object has no attribute 'address'.

@carltongibson carltongibson added this to the 3.7.1 Release milestone Oct 11, 2017
@carltongibson
Copy link
Collaborator

@rpkilby What's the status here? (Ta!)

@xordoquy Can you review and advise? Are you happy that #5375 was correct? Do we need more here?

I'm going to look at getting the Django 2.0 compat (actually) in place and then roll a 3.7.1. That'll be next week so there's plenty of time here.

@xordoquy
Copy link
Collaborator

Well, I think it's correct.
I'm just not so happy that a missing default values leads to an attribute error with ModelSerializers with DRF 3.7 but I couldn't find the time to write a test case out of it.

@rpkilby
Copy link
Member Author

rpkilby commented Oct 11, 2017

@xordoquy, if you have a simplified example of the nested ModelSerializer setup, I could work on turning that into a test case.

@carltongibson
Copy link
Collaborator

I'm going to go with this as is. (We can always come back again.)

@carltongibson carltongibson merged commit 6221124 into encode:master Oct 16, 2017
@rpkilby rpkilby deleted the dotted-source-docs branch October 16, 2017 09:34
@bhomnick bhomnick mentioned this pull request Mar 28, 2018
Closed
6 tasks
@stephenfin stephenfin mentioned this pull request Sep 26, 2020
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xordoquy xordoquy Awaiting requested review from xordoquy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.7.1 Release
Development

Successfully merging this pull request may close these issues.
3 participants
@rpkilby @carltongibson @xordoquy