diff --git a/AUTHORS b/AUTHORS index c3865471..84e52286 100644 --- a/AUTHORS +++ b/AUTHORS @@ -24,6 +24,7 @@ Léo S. Luc Cary Mansi Dhruv Matt Layman +Mehdy Khoshnoody Michael Haselton Mohammed Ali Zubair Nathanael Gordon diff --git a/CHANGELOG.md b/CHANGELOG.md index 30ef91ed..217ebb14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ any parts of the framework not mentioned in the documentation should generally b * Adjusted error messages to correctly use capital "JSON:API" abbreviation as used in the specification. * Avoid error when `parser_context` is `None` while parsing. * Raise comprehensible error when reserved field names `meta` and `results` are used. +* Use `relationships` in the error object `pointer` when the field is actually a relationship. ### Changed diff --git a/example/tests/snapshots/snap_test_errors.py b/example/tests/snapshots/snap_test_errors.py index 528b831b..dd5ca4f8 100644 --- a/example/tests/snapshots/snap_test_errors.py +++ b/example/tests/snapshots/snap_test_errors.py @@ -44,6 +44,17 @@ ] } +snapshots["test_relationship_errors_has_correct_pointers 1"] = { + "errors": [ + { + "code": "incorrect_type", + "detail": "Incorrect type. Expected resource identifier object, received str.", + "source": {"pointer": "/data/relationships/author"}, + "status": "400", + } + ] +} + snapshots["test_second_level_array_error 1"] = { "errors": [ { diff --git a/example/tests/test_errors.py b/example/tests/test_errors.py index 93cdb235..6b322d72 100644 --- a/example/tests/test_errors.py +++ b/example/tests/test_errors.py @@ -1,11 +1,11 @@ import pytest from django.test import override_settings from django.urls import path, reverse -from rest_framework import views +from rest_framework import generics from rest_framework_json_api import serializers -from example.models import Blog +from example.models import Author, Blog # serializers @@ -30,6 +30,9 @@ class EntrySerializer(serializers.Serializer): comment = CommentSerializer(required=False) headline = serializers.CharField(allow_null=True, required=True) body_text = serializers.CharField() + author = serializers.ResourceRelatedField( + queryset=Author.objects.all(), required=False + ) def validate(self, attrs): body_text = attrs["body_text"] @@ -40,13 +43,12 @@ def validate(self, attrs): # view -class DummyTestView(views.APIView): +class DummyTestView(generics.CreateAPIView): serializer_class = EntrySerializer resource_name = "entries" - def post(self, request, *args, **kwargs): - serializer = self.serializer_class(data=request.data) - serializer.is_valid(raise_exception=True) + def get_serializer_context(self): + return {} urlpatterns = [ @@ -191,3 +193,21 @@ def test_many_third_level_dict_errors(client, some_blog, snapshot): } snapshot.assert_match(perform_error_test(client, data)) + + +def test_relationship_errors_has_correct_pointers(client, some_blog, snapshot): + data = { + "data": { + "type": "entries", + "attributes": { + "blog": some_blog.pk, + "bodyText": "body_text", + "headline": "headline", + }, + "relationships": { + "author": {"data": {"id": "INVALID_ID", "type": "authors"}} + }, + } + } + + snapshot.assert_match(perform_error_test(client, data)) diff --git a/rest_framework_json_api/renderers.py b/rest_framework_json_api/renderers.py index e86c0c0b..b84666db 100644 --- a/rest_framework_json_api/renderers.py +++ b/rest_framework_json_api/renderers.py @@ -65,7 +65,7 @@ def extract_attributes(cls, fields, resource): if fields[field_name].write_only: continue # Skip fields with relations - if isinstance(field, (relations.RelatedField, relations.ManyRelatedField)): + if utils.is_relationship_field(field): continue # Skip read_only attribute fields when `resource` is an empty @@ -105,9 +105,7 @@ def extract_relationships(cls, fields, resource, resource_instance): continue # Skip fields without relations - if not isinstance( - field, (relations.RelatedField, relations.ManyRelatedField) - ): + if not utils.is_relationship_field(field): continue source = field.source @@ -298,9 +296,7 @@ def extract_included( continue # Skip fields without relations - if not isinstance( - field, (relations.RelatedField, relations.ManyRelatedField) - ): + if not utils.is_relationship_field(field): continue try: diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 19c72809..65cbc645 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -13,7 +13,7 @@ from django.http import Http404 from django.utils import encoding from django.utils.translation import gettext_lazy as _ -from rest_framework import exceptions +from rest_framework import exceptions, relations from rest_framework.exceptions import APIException from .settings import json_api_settings @@ -368,6 +368,10 @@ def get_relation_instance(resource_instance, source, serializer): return True, relation_instance +def is_relationship_field(field): + return isinstance(field, (relations.RelatedField, relations.ManyRelatedField)) + + class Hyperlink(str): """ A string like object that additionally has an associated name. @@ -394,9 +398,24 @@ def format_drf_errors(response, context, exc): errors.extend(format_error_object(message, "/data", response)) # handle all errors thrown from serializers else: + # Avoid circular deps + from rest_framework import generics + + has_serializer = isinstance(context["view"], generics.GenericAPIView) + if has_serializer: + serializer = context["view"].get_serializer() + fields = get_serializer_fields(serializer) or dict() + relationship_fields = [ + name for name, field in fields.items() if is_relationship_field(field) + ] + for field, error in response.data.items(): field = format_field_name(field) - pointer = "/data/attributes/{}".format(field) + pointer = None + # pointer can be determined only if there's a serializer. + if has_serializer: + rel = "relationships" if field in relationship_fields else "attributes" + pointer = "/data/{}/{}".format(rel, field) if isinstance(exc, Http404) and isinstance(error, str): # 404 errors don't have a pointer errors.extend(format_error_object(error, None, response))