Enhancement dont require pk strictly related #2745 by bleib1dj · Pull Request #2754 · encode/django-rest-framework (original) (raw)

Rationale

I implemented the enhancement with the id_field to provide users using non-django ORM backends the ability to still utilize the check originally provided by if obj.pk is None, prior to providing a related reference. I believe this gives more flexibility but also potentially provides unnecessary functionality as alternative backends may not generate an identification value solely upon storage and could potentially have their identifier provided prior to save. If the backend does that this check would only serve to verify that the user properly assigned an identifier but would not provide the intended affect of verifying the object has been created. That being said, there is the potential for this to also be done with the django ORM as the following example illustrates:

from django.contrib.auth.models import User
from uuid import uuid1

new_uuid = str(uuid1())
my_user = User(pk=new_uuid, email="this@email.com", username="test_user")
print my_user.pk
query_user = User.objects.get(pk=new_uuid)

Out: '5412f4cc-d166-11e4-b274-080027a2dd8c'
Out: DoesNotExist: User matching query does not exist

This goes against using User.objects.create(...) but could be done. In either case providing the id_field gives the user the ability to choose, where hasattr() limits the check to only working when the pk attribute is present.

If this way of handling the enhancement is agreed upon I can add additional docs detailing the optional field and when to utilize it. Otherwise we can close out this PR and I can open one with my hassattr() modifications instead.

Additional references

Per our discussion in #2745 I found a couple additional locations of pk attribute references.

PrimaryKeyRelatedField

There's a reference in the to_representation method but I opted not to change it as I believe the SlugField provides a way of achieving this functionality for a non-django ORM based backend.

ManyRelatedField

There was a reference in the get_attribute method that I swapped out with self.id_field which also required the definition of self.id_field within it's own __init__ method.

I chose to define it there rather than up in the Field class as the ManyRelatedField is "Automagically" created in the RelatedField and passed its kwargs. So instead of giving users the ability to set an id_field on every field, which would be unnecessary, this should reduce it to them only needing to set it on the HyperlinkedRelatedField and HyperlinkedIdentityField if they choose to, while still supporting many=True.