Skip to content

Re-enabled overwriting of URL field #1237

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 24, 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
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
Loading