Skip to content

WIP: Issue #155 - support serializers with custom primary keys #165

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

Closed
wants to merge 1 commit into from
Closed

Conversation

orf
Copy link

@orf orf commented Nov 27, 2015

This is a first attempt at adding support for alternative primary keys. This is done by removing any hard-coded 'instance.pk' calls, instead first checking if the PK attributes name is in any serialized output. If it is then we return that instead of the pk attribute. This works for the following serializers/viewsets:

class AuthorSerializer(serializers.ModelSerializer):
    id = serializers.CharField(source='email')

    class Meta:
        model = Author  # has (id, email) fields
        fields = ('id',)

class AuthorViewSet(viewsets.ModelViewSet):
    queryset = Author.objects.all()
    serializer_class = AuthorSerializer

    lookup_field = 'email'

This produces the following output:

 {
      "type": "Author",
      "id": "[email protected]",
      "attributes": {},
}

Works for foreign keys and m2m relationships as well as creating new instances (as far as I can tell). I started to write some tests but quickly got a bit lost, so I thought I would submit this for comments before the weekend and maybe tidy it up on Monday if you think it looks promising.

@grjones
Copy link

grjones commented Oct 28, 2016

+1 For this. There are edge cases where this is needed and would have parity with DRF serializer support for custom primary keys.

@orf
Copy link
Author

orf commented Oct 28, 2016

I tried to explain the issue and premise of this PR in #155 but the maintainers didn't seem to get it. I don't think this will ever be added which is a great shame as it seems pretty arbitrary. DRF supports it, but this library doesn't, because reasons.

@auvipy
Copy link
Contributor

auvipy commented Jan 19, 2017

@orf how about adding to https://github.com/chibisov/drf-extensions

@orf
Copy link
Author

orf commented Jan 19, 2017

It's not an extension to DRF, DRF already supports this. It's a bugfix for this library that seems to always assume you want to expose your pk

@auvipy
Copy link
Contributor

auvipy commented Jan 19, 2017

@mblayman
Copy link
Collaborator

mblayman commented Mar 1, 2017

Hi, @orf. Thanks for the contribution! This branch is now in conflict in the main branch and rather old. Are you interested in fixing this up or should we close out this PR? I'm stepping up to maintain the PR queue and get DRF JA changes flowing again, but I can't merge conflicted stuff. Also, this branch is marked as a work in progress so I'm not sure if it was ever finished.

If this is still valuable, resolving the merge conflicts will give me a chance to review it well. If this is no longer important to you, closing the PR is right next step.

Please let me know what you'd like to do. Thanks!

@orf
Copy link
Author

orf commented Mar 1, 2017

Hey @mblayman, I believe a more up-to-date patch is here: rpatterson@6334ceb

Feel free to close this MR, if @rpatterson is willing to make a MR with that change it would be great. I could contribute there rather than resurrect this branch.

@orf orf closed this Mar 1, 2017
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