-
Notifications
You must be signed in to change notification settings - Fork 298
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
Conversation
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.
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.
@orf how about adding to https://github.com/chibisov/drf-extensions
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
then try https://github.com/AltSchool/dynamic-rest this
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!
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.
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
pkattribute. This works for the following serializers/viewsets:This produces the following output:
{ "type": "Author", "id": "someemail@gmail.com", "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.