Skip to content

Commit

Permalink
Re-enabled overwriting of URL field (#1237)
Browse files Browse the repository at this point in the history
Enabled overwriting of URL field

URL_FIELD_NAME is usually used as self-link in links.
However it should be allowed to be overwritten as long
as not HyperlinkedIdentifyField has been used.
  • Loading branch information
sliverc authored Jun 24, 2024
1 parent 3f1ea67 commit 53469f3
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 8 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
Note that in line with [Django REST framework policy](https://www.django-rest-framework.org/topics/release-notes/),
any parts of the framework not mentioned in the documentation should generally be considered private API, and may be subject to change.

## [Unreleased]

### Fixed

* Re-enabled overwriting of url field (regression since 7.0.0)

## [7.0.1] - 2024-06-06

### Added
Expand Down
11 changes: 6 additions & 5 deletions rest_framework_json_api/renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,11 @@ def extract_attributes(cls, fields, resource):
and relationships are not returned.
"""

invalid_fields = {"id", api_settings.URL_FIELD_NAME}

return {
format_field_name(field_name): value
for field_name, value in resource.items()
if field_name in fields
and field_name not in invalid_fields
and field_name != "id"
and not is_relationship_field(fields[field_name])
}

Expand Down Expand Up @@ -449,7 +447,10 @@ def _filter_sparse_fields(cls, serializer, fields, resource_name):
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
or (
field.field_name == api_settings.URL_FIELD_NAME
and isinstance(field, relations.HyperlinkedIdentityField)
)
}

return fields
Expand Down Expand Up @@ -486,7 +487,7 @@ def build_json_resource_obj(
resource_data["relationships"] = relationships
# Add 'self' link if field is present and valid
if api_settings.URL_FIELD_NAME in resource and isinstance(
fields[api_settings.URL_FIELD_NAME], relations.RelatedField
fields[api_settings.URL_FIELD_NAME], relations.HyperlinkedIdentityField
):
resource_data["links"] = {"self": resource[api_settings.URL_FIELD_NAME]}

Expand Down
10 changes: 7 additions & 3 deletions rest_framework_json_api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.utils.module_loading import import_string as import_class_from_dotted_path
from django.utils.translation import gettext_lazy as _
from rest_framework.exceptions import ParseError
from rest_framework.relations import HyperlinkedIdentityField

# star import defined so `rest_framework_json_api.serializers` can be
# a simple drop in for `rest_framework.serializers`
Expand Down Expand Up @@ -94,9 +95,12 @@ 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
# URL_FIELD_NAME is the field used as self-link to resource
# however only when it is a HyperlinkedIdentityField
or (
field.field_name == api_settings.URL_FIELD_NAME
and isinstance(field, HyperlinkedIdentityField)
)
# ID is a required field which might have been overwritten
# so need to keep it
or field.field_name == "id"
Expand Down
6 changes: 6 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
ManyToManySource,
ManyToManyTarget,
NestedRelatedSource,
URLModel,
)


Expand Down Expand Up @@ -36,6 +37,11 @@ def model(db):
return BasicModel.objects.create(text="Model")


@pytest.fixture
def url_instance(db):
return URLModel.objects.create(text="Url", url="https://example.com")


@pytest.fixture
def foreign_key_target(db):
return ForeignKeyTarget.objects.create(name="Target")
Expand Down
8 changes: 8 additions & 0 deletions tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ class Meta:
ordering = ("id",)


class URLModel(DJAModel):
url = models.URLField()
text = models.CharField(max_length=100)

class Meta:
ordering = ("id",)


# Models for relations tests
# ManyToMany
class ManyToManyTarget(DJAModel):
Expand Down
10 changes: 10 additions & 0 deletions tests/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
ManyToManySource,
ManyToManyTarget,
NestedRelatedSource,
URLModel,
)


Expand All @@ -17,6 +18,15 @@ class Meta:
model = BasicModel


class URLModelSerializer(serializers.ModelSerializer):
class Meta:
fields = (
"text",
"url",
)
model = URLModel


class ForeignKeyTargetSerializer(serializers.ModelSerializer):
class Meta:
fields = ("name",)
Expand Down
38 changes: 38 additions & 0 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
ForeignKeyTargetViewSet,
ManyToManySourceViewSet,
NestedRelatedSourceViewSet,
URLModelViewSet,
)


Expand Down Expand Up @@ -182,6 +183,42 @@ def test_list_with_default_included_resources(self, client, foreign_key_source):
}
] == result["included"]

@pytest.mark.urls(__name__)
def test_list_allow_overwriting_url_field(self, client, url_instance):
"""
Test overwriting of url is possible.
URL_FIELD_NAME which is set to 'url' per default is used as self in links.
However if field is overwritten and not a HyperlinkedIdentityField it should be allowed
to use as a attribute as well.
"""

url = reverse("urlmodel-list")
response = client.get(url)
assert response.status_code == status.HTTP_200_OK
data = response.json()["data"]
assert data == [
{
"type": "URLModel",
"id": str(url_instance.pk),
"attributes": {"text": "Url", "url": "https://example.com"},
}
]

@pytest.mark.urls(__name__)
def test_list_allow_overwiritng_url_with_sparse_fields(self, client, url_instance):
url = reverse("urlmodel-list")
response = client.get(url, data={"fields[URLModel]": "text"})
assert response.status_code == status.HTTP_200_OK
data = response.json()["data"]
assert data == [
{
"type": "URLModel",
"id": str(url_instance.pk),
"attributes": {"text": "Url"},
}
]

@pytest.mark.urls(__name__)
def test_retrieve(self, client, model):
url = reverse("basic-model-detail", kwargs={"pk": model.pk})
Expand Down Expand Up @@ -495,6 +532,7 @@ def patch(self, request, *args, **kwargs):
# configuration in general
router = SimpleRouter()
router.register(r"basic_models", BasicModelViewSet, basename="basic-model")
router.register(r"url_models", URLModelViewSet)
router.register(r"foreign_key_sources", ForeignKeySourceViewSet)
router.register(r"foreign_key_targets", ForeignKeyTargetViewSet)
router.register(
Expand Down
8 changes: 8 additions & 0 deletions tests/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
ForeignKeyTarget,
ManyToManySource,
NestedRelatedSource,
URLModel,
)
from tests.serializers import (
BasicModelSerializer,
Expand All @@ -13,6 +14,7 @@
ForeignKeyTargetSerializer,
ManyToManySourceSerializer,
NestedRelatedSourceSerializer,
URLModelSerializer,
)


Expand All @@ -22,6 +24,12 @@ class BasicModelViewSet(ModelViewSet):
ordering = ["text"]


class URLModelViewSet(ModelViewSet):
serializer_class = URLModelSerializer
queryset = URLModel.objects.all()
ordering = ["url"]


class ForeignKeySourceViewSet(ModelViewSet):
serializer_class = ForeignKeySourceSerializer
queryset = ForeignKeySource.objects.all()
Expand Down

0 comments on commit 53469f3

Please sign in to comment.