Skip to content

Ensured that URL and id field are kept when using sparse fields #1231

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 1 commit into from
Jun 4, 2024
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ any parts of the framework not mentioned in the documentation should generally b

* Added `429 Too Many Requests` as a possible error response in the OpenAPI schema.

### Fixed

* Ensured that URL and id field are kept when using sparse fields (regression since 7.0.0)

## [7.0.0] - 2024-05-02

### Added
Expand Down
5 changes: 4 additions & 1 deletion rest_framework_json_api/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,10 @@ def _filter_sparse_fields(cls, serializer, fields, resource_name):
return {
field_name: field
for field_name, field, in fields.items()
if field_name in sparse_fields
if field.field_name in sparse_fields
# URL field is not considered a field in JSON:API spec
# but a link so need to keep it
or field.field_name == api_settings.URL_FIELD_NAME
}

return fields
Expand Down
7 changes: 6 additions & 1 deletion rest_framework_json_api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,15 @@ def _readable_fields(self):
field
for field in readable_fields
if field.field_name in sparse_fields
# URL field is not considered a field in JSON:API spec
# but a link so need to keep it
or field.field_name == api_settings.URL_FIELD_NAME
# ID is a required field which might have been overwritten
# so need to keep it
or field.field_name == "id"
)
except AttributeError:
# no type on serializer, must be used only as only nested
# no type on serializer, may only be used nested
pass

return readable_fields
Expand Down
12 changes: 12 additions & 0 deletions tests/serializers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from rest_framework.settings import api_settings

from rest_framework_json_api import serializers
from tests.models import (
BasicModel,
Expand Down Expand Up @@ -32,6 +34,16 @@ class Meta:
)


class ForeignKeySourcetHyperlinkedSerializer(serializers.HyperlinkedModelSerializer):
class Meta:
model = ForeignKeySource
fields = (
"name",
"target",
api_settings.URL_FIELD_NAME,
)


class ManyToManyTargetSerializer(serializers.ModelSerializer):
class Meta:
fields = ("name",)
Expand Down
64 changes: 57 additions & 7 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
from tests.serializers import BasicModelSerializer, ForeignKeyTargetSerializer
from tests.views import (
BasicModelViewSet,
ForeignKeySourcetHyperlinkedViewSet,
ForeignKeySourceViewSet,
ForeignKeyTargetViewSet,
ManyToManySourceViewSet,
NestedRelatedSourceViewSet,
)
Expand Down Expand Up @@ -87,7 +89,7 @@ def test_list(self, client, model):

@pytest.mark.urls(__name__)
def test_list_with_include_foreign_key(self, client, foreign_key_source):
url = reverse("foreign-key-source-list")
url = reverse("foreignkeysource-list")
response = client.get(url, data={"include": "target"})
assert response.status_code == status.HTTP_200_OK
result = response.json()
Expand Down Expand Up @@ -156,7 +158,7 @@ def test_list_with_include_nested_related_field(

@pytest.mark.urls(__name__)
def test_list_with_invalid_include(self, client, foreign_key_source):
url = reverse("foreign-key-source-list")
url = reverse("foreignkeysource-list")
response = client.get(url, data={"include": "invalid"})
assert response.status_code == status.HTTP_400_BAD_REQUEST
result = response.json()
Expand Down Expand Up @@ -195,7 +197,7 @@ def test_retrieve(self, client, model):

@pytest.mark.urls(__name__)
def test_retrieve_with_include_foreign_key(self, client, foreign_key_source):
url = reverse("foreign-key-source-detail", kwargs={"pk": foreign_key_source.pk})
url = reverse("foreignkeysource-detail", kwargs={"pk": foreign_key_source.pk})
response = client.get(url, data={"include": "target"})
assert response.status_code == status.HTTP_200_OK
result = response.json()
Expand All @@ -208,6 +210,20 @@ def test_retrieve_with_include_foreign_key(self, client, foreign_key_source):
}
] == result["included"]

@pytest.mark.urls(__name__)
def test_retrieve_hyperlinked_with_sparse_fields(self, client, foreign_key_source):
url = reverse(
"foreignkeysourcehyperlinked-detail", kwargs={"pk": foreign_key_source.pk}
)
response = client.get(url, data={"fields[ForeignKeySource]": "name"})
assert response.status_code == status.HTTP_200_OK
data = response.json()["data"]
assert data["attributes"] == {"name": foreign_key_source.name}
assert "relationships" not in data
assert data["links"] == {
"self": f"http://testserver/foreign_key_sources/{foreign_key_source.pk}/"
}

@pytest.mark.urls(__name__)
def test_patch(self, client, model):
data = {
Expand Down Expand Up @@ -239,7 +255,7 @@ def test_delete(self, client, model):

@pytest.mark.urls(__name__)
def test_create_with_sparse_fields(self, client, foreign_key_target):
url = reverse("foreign-key-source-list")
url = reverse("foreignkeysource-list")
data = {
"data": {
"id": None,
Expand Down Expand Up @@ -379,6 +395,28 @@ def test_patch_with_custom_id(self, client):
}
}

@pytest.mark.urls(__name__)
def test_patch_with_custom_id_with_sparse_fields(self, client):
data = {
"data": {
"id": 2_193_102,
"type": "custom",
"attributes": {"body": "hello"},
}
}

url = reverse("custom-id")

response = client.patch(f"{url}?fields[custom]=body", data=data)
assert response.status_code == status.HTTP_200_OK
assert response.json() == {
"data": {
"type": "custom",
"id": "2176ce", # get_id() -> hex
"attributes": {"body": "hello"},
}
}


# Routing setup

Expand Down Expand Up @@ -415,13 +453,16 @@ class CustomModelSerializer(serializers.Serializer):
id = serializers.IntegerField()


class CustomIdModelSerializer(serializers.Serializer):
class CustomIdSerializer(serializers.Serializer):
id = serializers.SerializerMethodField()
body = serializers.CharField()

def get_id(self, obj):
return hex(obj.id)[2:]

class Meta:
resource_name = "custom"


class CustomAPIView(APIView):
parser_classes = [JSONParser]
Expand All @@ -443,14 +484,23 @@ class CustomIdAPIView(APIView):
resource_name = "custom"

def patch(self, request, *args, **kwargs):
serializer = CustomIdModelSerializer(CustomModel(request.data))
serializer = CustomIdSerializer(
CustomModel(request.data), context={"request": self.request}
)
return Response(status=status.HTTP_200_OK, data=serializer.data)


# TODO remove basename and use default (lowercase of model)
# this makes using HyperlinkedIdentityField easier and reduces
# configuration in general
router = SimpleRouter()
router.register(r"basic_models", BasicModelViewSet, basename="basic-model")
router.register(r"foreign_key_sources", ForeignKeySourceViewSet)
router.register(r"foreign_key_targets", ForeignKeyTargetViewSet)
router.register(
r"foreign_key_sources", ForeignKeySourceViewSet, basename="foreign-key-source"
r"foreign_key_sources_hyperlinked",
ForeignKeySourcetHyperlinkedViewSet,
"foreignkeysourcehyperlinked",
)
router.register(
r"many_to_many_sources", ManyToManySourceViewSet, basename="many-to-many-source"
Expand Down
15 changes: 15 additions & 0 deletions tests/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
from tests.models import (
BasicModel,
ForeignKeySource,
ForeignKeyTarget,
ManyToManySource,
NestedRelatedSource,
)
from tests.serializers import (
BasicModelSerializer,
ForeignKeySourceSerializer,
ForeignKeySourcetHyperlinkedSerializer,
ForeignKeyTargetSerializer,
ManyToManySourceSerializer,
NestedRelatedSourceSerializer,
)
Expand All @@ -25,6 +28,18 @@ class ForeignKeySourceViewSet(ModelViewSet):
ordering = ["name"]


class ForeignKeySourcetHyperlinkedViewSet(ModelViewSet):
serializer_class = ForeignKeySourcetHyperlinkedSerializer
queryset = ForeignKeySource.objects.all()
ordering = ["name"]


class ForeignKeyTargetViewSet(ModelViewSet):
serializer_class = ForeignKeyTargetSerializer
queryset = ForeignKeyTarget.objects.all()
ordering = ["name"]


class ManyToManySourceViewSet(ModelViewSet):
serializer_class = ManyToManySourceSerializer
queryset = ManyToManySource.objects.all()
Expand Down
Loading