Don't strictly require 'pk' in HyperLinkedRelatedField.

[bleib1dj]

A direct check on an object for a pk attribute seems to tightly couple the HyperLinkedRelatedField with the Django ORM. It seems to reduce the ability to utilize the HyperLinkedRelatedField with other backends such as DynamoDB, Neo4j, and potentially others. It can be circumvented by adding a pk value to these data stores but I was wondering if it would make sense to instead use a definable attribute that defaulted to pk but could be overwritten if necessary.

The use_pk_only_optimization function that is used for the PKOnlyObject optimization could also be used prior to the direct check. I'm not sure if this would generate expected behavior though, as the majority of users who do use the ORM would end up losing the check whenever they changed their lookup_field.

[tomchristie]

'pk' is just the default. Use lookup_field=... if you want something else.

[bleib1dj]

class HyperlinkedRelatedField(RelatedField):
    ...
    def get_url(self, obj, view_name, request, format):
        """
        ...
        """
        if obj.pk is None:
            return None

        lookup_value = getattr(obj, self.lookup_field)
        ...

Hey @tomchristie, I might be missing something but how would setting the lookup_field resolve the AttributeError you'd receive by the obj.pk call?

[tomchristie]

Okay, so that line should could be tweaked to...

if hasattr(obj, 'pk') and obj.pk is None

Would make sense.

[tomchristie]

It'd also be worth checking for any other points in the relationships that do the same and also adjusting them.

[bleib1dj]

Alright I'll put that together and submit a pull request. I had added self.id_field = kwargs.pop('id_field', 'pk') in the __init__ and then referenced that with a getattr(obj, self.id_field) in place of the obj.pk is None and that seemed to work as well. If you'd like that approach instead just let me know. Otherwise I'll look around for any additional items doing the same thing and replace with a hasattr check.

[tomchristie]

👍

[xordoquy]

The associated PR was merged and released in the 3.1.2 release.
Can someone confirm this ticket should be closed ?

[tomchristie]

Closing the ticket unless someone can confirm that it shouldn't be :)

[xordoquy]

Good point ;)