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

Return empty list rather than empty dict for serializers with many=True #3437

Closed
wants to merge 1 commit into from

Conversation

nip3o
Copy link

@nip3o nip3o commented Sep 23, 2015

Failing test cases for #3434

@tomchristie
Copy link
Member

Not obvious why the tests havn't run.

@jpadilla
Copy link
Member

Seems like Travis might be having some issues.

https://www.traviscistatus.com
On Wed, Sep 23, 2015 at 12:43 PM Tom Christie [email protected]
wrote:

Not obvious why the tests havn't run.


Reply to this email directly or view it on GitHub
#3437 (comment)
.

@jpadilla
Copy link
Member

Wonder if there's anything we can do to make travis run this. Not showing up since yesterday. Close this and open a new PR?

@xordoquy
Copy link
Collaborator

Adding a new commit should do

@nip3o
Copy link
Author

nip3o commented Sep 24, 2015

Rebased and pushed again, seems like Travis is running now.

@jpadilla
Copy link
Member

jpadilla commented Oct 1, 2015

Taking a look at this. First thing that came to mine was doing something like the following in ListSerializer:

    @property
    def errors(self):
        ret = super(ListSerializer, self).errors

        if ret == {}:
            return ReturnList([], serializer=self)

        if isinstance(ret, dict):
            return ReturnDict(ret, serializer=self)

        return ReturnList(ret, serializer=self)

    @property
    def validated_data(self):
        validated_data = super(ListSerializer, self).validated_data

        if validated_data == {}:
            return ReturnList([], serializer=self)

        return validated_data

All tests pass including these.

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

Successfully merging this pull request may close these issues.

4 participants