Skip to content

Add relationships support for pointers in errors #986

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

Merged
merged 4 commits into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Léo S. <[email protected]>
Luc Cary <[email protected]>
Mansi Dhruv <[email protected]>
Matt Layman <https://www.mattlayman.com>
Mehdy Khoshnoody <[email protected]>
Michael Haselton <[email protected]>
Mohammed Ali Zubair <[email protected]>
Nathanael Gordon <[email protected]>
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 11 additions & 0 deletions example/tests/snapshots/snap_test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
{
Expand Down
32 changes: 26 additions & 6 deletions example/tests/test_errors.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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"]
Expand All @@ -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 = [
Expand Down Expand Up @@ -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))
10 changes: 3 additions & 7 deletions rest_framework_json_api/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
23 changes: 21 additions & 2 deletions rest_framework_json_api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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))
Expand Down