From 679a27250bdb73468d907882dcfc5e22719792cd Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Mon, 20 Feb 2017 16:05:06 -0800 Subject: [PATCH 1/4] Issue 155 Defer to lookup_field/custom id fields TODO Test coverage There are so many places where PK is assumed in djangorestframework-jsonapi and so many places/ways to change IDs/URLs in DRF itself, that getting coverage on all the edge cases will be quite a lot of work. Some of that work may be better spent re-factoring things to make the code easier to understand and reduce the number of edge cases in the first place. --- rest_framework_json_api/renderers.py | 51 ++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 1c74ffdf..99f3197e 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -111,7 +111,8 @@ def extract_relationships(cls, fields, resource, resource_instance): relation_data.append( OrderedDict([ ('type', relation_type), - ('id', encoding.force_text(related_object.pk)) + ('id', encoding.force_text(getattr( + related_object.pk, field.lookup_field))) ]) ) @@ -150,12 +151,17 @@ def extract_relationships(cls, fields, resource, resource_instance): if isinstance( field, (relations.PrimaryKeyRelatedField, relations.HyperlinkedRelatedField) ): - resolved, relation = utils.get_relation_instance( - resource_instance, '%s_id' % source, field.parent - ) - if not resolved: - continue - relation_id = relation if resource.get(field_name) else None + lookup_field = getattr(field, 'lookup_field', None) + if lookup_field: + relation_id = getattr( + getattr(resource_instance, source), lookup_field) + else: + resolved, relation = utils.get_relation_instance( + resource_instance, '%s_id' % source, field.parent + ) + if not resolved: + continue + relation_id = relation if resource.get(field_name) else None relation_data = { 'data': ( OrderedDict([ @@ -216,9 +222,15 @@ def extract_relationships(cls, fields, resource, resource_instance): utils.get_resource_type_from_instance(nested_resource_instance) ) + if hasattr(field.child_relation, 'lookup_field'): + _id = getattr(nested_resource_instance, + field.child_relation.lookup_field) + else: + _id = nested_resource_instance.pk + relation_data.append(OrderedDict([ ('type', nested_resource_instance_type), - ('id', encoding.force_text(nested_resource_instance.pk)) + ('id', encoding.force_text(_id)) ])) data.update({ field_name: { @@ -244,14 +256,23 @@ def extract_relationships(cls, fields, resource, resource_instance): if isinstance(serializer_data, list): for position in range(len(serializer_data)): nested_resource_instance = resource_instance_queryset[position] + nested_resource_data = serializer_data[position] nested_resource_instance_type = ( relation_type or utils.get_resource_type_from_instance(nested_resource_instance) ) + instance_pk_name = ( + getattr(field, 'lookup_field', None) or + nested_resource_instance._meta.pk.name) + if instance_pk_name in nested_resource_data: + pk = nested_resource_data[instance_pk_name] + else: + pk = nested_resource_instance.pk + relation_data.append(OrderedDict([ ('type', nested_resource_instance_type), - ('id', encoding.force_text(nested_resource_instance.pk)) + ('id', encoding.force_text(pk)) ])) data.update({field_name: {'data': relation_data}}) @@ -463,9 +484,19 @@ def build_json_resource_obj(cls, fields, resource, resource_instance, resource_n # Determine type from the instance if the underlying model is polymorphic if force_type_resolution: resource_name = utils.get_resource_type_from_instance(resource_instance) + if resource_instance is None: + pk = None + elif 'id' in fields: + pk = fields['id'].get_attribute(resource_instance) + elif 'url' in fields: + pk = getattr(resource_instance, fields['url'].lookup_field) + else: + # Check if the primary key exists in the resource by getting the + # primary keys attribute name. + pk = resource_instance.pk resource_data = [ ('type', resource_name), - ('id', encoding.force_text(resource_instance.pk) if resource_instance else None), + ('id', encoding.force_text(pk)), ('attributes', cls.extract_attributes(fields, resource)), ] relationships = cls.extract_relationships(fields, resource, resource_instance) From 50fdfe02dce8b15498a0a6c2f87e19121bbd4080 Mon Sep 17 00:00:00 2001 From: Ola Tarkowska Date: Sun, 18 Feb 2018 19:50:56 +0000 Subject: [PATCH 2/4] minor fixes --- rest_framework_json_api/renderers.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 99f3197e..bd14cdd9 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -112,7 +112,7 @@ def extract_relationships(cls, fields, resource, resource_instance): OrderedDict([ ('type', relation_type), ('id', encoding.force_text(getattr( - related_object.pk, field.lookup_field))) + related_object, field.lookup_field))) ]) ) @@ -223,14 +223,14 @@ def extract_relationships(cls, fields, resource, resource_instance): ) if hasattr(field.child_relation, 'lookup_field'): - _id = getattr(nested_resource_instance, - field.child_relation.lookup_field) + pk = getattr(nested_resource_instance, + field.child_relation.lookup_field) else: - _id = nested_resource_instance.pk + pk = nested_resource_instance.pk relation_data.append(OrderedDict([ ('type', nested_resource_instance_type), - ('id', encoding.force_text(_id)) + ('id', encoding.force_text(pk)) ])) data.update({ field_name: { From 232d580c6c2799f00aa3f8c4bf4fd30a95937cc8 Mon Sep 17 00:00:00 2001 From: Ola Tarkowska Date: Thu, 5 Oct 2017 17:26:36 +0100 Subject: [PATCH 3/4] test if relation is not empty --- rest_framework_json_api/renderers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index bd14cdd9..8984c415 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -154,7 +154,8 @@ def extract_relationships(cls, fields, resource, resource_instance): lookup_field = getattr(field, 'lookup_field', None) if lookup_field: relation_id = getattr( - getattr(resource_instance, source), lookup_field) + getattr(resource_instance, source), + lookup_field, None) else: resolved, relation = utils.get_relation_instance( resource_instance, '%s_id' % source, field.parent From 1b0e853ab5fa7de0459c445e0a3f1ef623d7bf46 Mon Sep 17 00:00:00 2001 From: Ola Tarkowska Date: Mon, 20 Nov 2017 13:23:11 +0000 Subject: [PATCH 4/4] relation must have either links, data or meta in relationship --- rest_framework_json_api/renderers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index 8984c415..ab50a915 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -168,7 +168,7 @@ def extract_relationships(cls, fields, resource, resource_instance): OrderedDict([ ('type', relation_type), ('id', encoding.force_text(relation_id)) ]) - if relation_id is not None else None) + if relation_id is not None else {'data': None}) } if (