From ea32c5b97420bde0d67531bbaf6ab24a5029f3c9 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Sat, 3 Apr 2021 19:25:40 +0300 Subject: [PATCH 1/5] Get current serializer class --- example/serializers.py | 4 ++-- .../test_non_paginated_responses.py | 4 ++-- example/tests/integration/test_pagination.py | 2 +- example/tests/test_filters.py | 2 +- example/tests/test_views.py | 2 +- example/urls.py | 10 +++++----- example/urls_test.py | 10 +++++----- rest_framework_json_api/relations.py | 8 ++++++++ rest_framework_json_api/schemas/openapi.py | 18 +++++++++++------- rest_framework_json_api/views.py | 2 +- 10 files changed, 37 insertions(+), 25 deletions(-) diff --git a/example/serializers.py b/example/serializers.py index 4d80c87c..ca6abb15 100644 --- a/example/serializers.py +++ b/example/serializers.py @@ -111,8 +111,8 @@ def __init__(self, *args, **kwargs): comments = relations.ResourceRelatedField(many=True, read_only=True) # many related hyperlinked from model comments_hyperlinked = relations.HyperlinkedRelatedField( - related_link_view_name="entry-comments", - related_link_url_kwarg="entry_pk", + related_link_view_name="entry-related", + related_link_related_name='comments', self_link_view_name="entry-relationships", many=True, read_only=True, diff --git a/example/tests/integration/test_non_paginated_responses.py b/example/tests/integration/test_non_paginated_responses.py index 5a2e59c8..dc1d7932 100644 --- a/example/tests/integration/test_non_paginated_responses.py +++ b/example/tests/integration/test_non_paginated_responses.py @@ -42,7 +42,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): }, "commentsHyperlinked": { "links": { - "related": "http://testserver/entries/1/comments", + "related": "http://testserver/entries/1/comments/", "self": "http://testserver/entries/1/relationships/comments_hyperlinked", } }, @@ -97,7 +97,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): }, "commentsHyperlinked": { "links": { - "related": "http://testserver/entries/2/comments", + "related": "http://testserver/entries/2/comments/", "self": "http://testserver/entries/2/relationships/comments_hyperlinked", } }, diff --git a/example/tests/integration/test_pagination.py b/example/tests/integration/test_pagination.py index aedf8c6c..6263656d 100644 --- a/example/tests/integration/test_pagination.py +++ b/example/tests/integration/test_pagination.py @@ -42,7 +42,7 @@ def test_pagination_with_single_entry(single_entry, client): }, "commentsHyperlinked": { "links": { - "related": "http://testserver/entries/1/comments", + "related": "http://testserver/entries/1/comments/", "self": "http://testserver/entries/1/relationships/comments_hyperlinked", } }, diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index 68ad1452..df913d81 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -491,7 +491,7 @@ def test_search_keywords(self): "commentsHyperlinked": { "links": { "self": "http://testserver/entries/7/relationships/comments_hyperlinked", # noqa: E501 - "related": "http://testserver/entries/7/comments", + "related": "http://testserver/entries/7/comments/", } }, "suggested": { diff --git a/example/tests/test_views.py b/example/tests/test_views.py index 4b494b52..e08b26d1 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -651,7 +651,7 @@ def test_get_object_gives_correct_entry(self): "commentsHyperlinked": { "links": { "related": "http://testserver/entries/{}" - "/comments".format(self.second_entry.id), + "/comments/".format(self.second_entry.id), "self": "http://testserver/entries/{}/relationships" "/comments_hyperlinked".format(self.second_entry.id), } diff --git a/example/urls.py b/example/urls.py index 800b9f79..6504987a 100644 --- a/example/urls.py +++ b/example/urls.py @@ -45,11 +45,6 @@ BlogViewSet.as_view({"get": "retrieve"}), name="entry-blog", ), - url( - r"entries/(?P[^/.]+)/comments$", - CommentViewSet.as_view({"get": "list"}), - name="entry-comments", - ), url( r"entries/(?P[^/.]+)/authors$", AuthorViewSet.as_view({"get": "list"}), @@ -60,6 +55,11 @@ EntryViewSet.as_view({"get": "retrieve"}), name="entry-featured", ), + url( + r"^entries/(?P[^/.]+)/(?P\w+)/$", + EntryViewSet.as_view({"get": "retrieve_related"}), + name="entry-related", + ), url( r"^authors/(?P[^/.]+)/(?P\w+)/$", AuthorViewSet.as_view({"get": "retrieve_related"}), diff --git a/example/urls_test.py b/example/urls_test.py index 7e875936..b748082f 100644 --- a/example/urls_test.py +++ b/example/urls_test.py @@ -52,11 +52,6 @@ BlogViewSet.as_view({"get": "retrieve"}), name="entry-blog", ), - re_path( - r"^entries/(?P[^/.]+)/comments$", - CommentViewSet.as_view({"get": "list"}), - name="entry-comments", - ), re_path( r"^entries/(?P[^/.]+)/suggested/$", EntryViewSet.as_view({"get": "list"}), @@ -77,6 +72,11 @@ EntryViewSet.as_view({"get": "retrieve"}), name="entry-featured", ), + re_path( + r"^entries/(?P[^/.]+)/(?P\w+)/$", + EntryViewSet.as_view({"get": "retrieve_related"}), + name="entry-related", + ), re_path( r"^authors/(?P[^/.]+)/(?P\w+)/$", AuthorViewSet.as_view({"get": "retrieve_related"}), diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index c9dd765d..aa48e59d 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -27,6 +27,7 @@ "related_link_view_name", "related_link_lookup_field", "related_link_url_kwarg", + "related_link_related_name", ] @@ -68,6 +69,10 @@ def __init__(self, self_link_view_name=None, related_link_view_name=None, **kwar "related_link_url_kwarg", self.related_link_lookup_field ) + self.related_link_related_name = kwargs.pop( + "related_link_related_name", None + ) + # We include this simply for dependency injection in tests. # We can't add it as a class attributes or it would expect an # implicit `self` argument to be passed. @@ -119,6 +124,9 @@ def get_links(self, obj=None, lookup_field="pk"): self_kwargs.update({"related_field": format_link_segment(field_name)}) self_link = self.get_url("self", self.self_link_view_name, self_kwargs, request) + if self.related_link_related_name: + self_kwargs.update({"related_field": format_link_segment(self.related_link_related_name)}) + # Assuming RelatedField will be declared in two ways: # 1. url(r'^authors/(?P[^/.]+)/(?P\w+)/$', # AuthorViewSet.as_view({'get': 'retrieve_related'})) diff --git a/rest_framework_json_api/schemas/openapi.py b/rest_framework_json_api/schemas/openapi.py index b51f366f..cbf36e6f 100644 --- a/rest_framework_json_api/schemas/openapi.py +++ b/rest_framework_json_api/schemas/openapi.py @@ -357,14 +357,15 @@ def _expand_related(self, path, method, view, view_endpoints): ) if related_view: action = self._field_is_one_or_many(field, view) - result.append( - ( - path.replace("{related_field}", field), - method, - related_view, - action, + if action: + result.append( + ( + path.replace("{related_field}", field), + method, + related_view, + action, + ) ) - ) return result @@ -392,6 +393,9 @@ def _find_related_view(self, view_endpoints, related_serializer, parent_view): def _field_is_one_or_many(self, field, view): serializer = view.get_serializer() + if field not in serializer.fields: + return None + if isinstance(serializer.fields[field], ManyRelatedField): return "list" else: diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index 2f061cbb..3df27d1f 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -154,7 +154,7 @@ def get_related_serializer(self, instance, **kwargs): return serializer_class(instance, **kwargs) def get_related_serializer_class(self): - parent_serializer_class = super(RelatedMixin, self).get_serializer_class() + parent_serializer_class = self.get_serializer_class() if "related_field" in self.kwargs: field_name = self.kwargs["related_field"] From 42533f5a488d895240ab1e3da24f06acc3b8a064 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Sat, 3 Apr 2021 19:42:23 +0300 Subject: [PATCH 2/5] Format code --- example/serializers.py | 2 +- rest_framework_json_api/relations.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/example/serializers.py b/example/serializers.py index ca6abb15..1a807270 100644 --- a/example/serializers.py +++ b/example/serializers.py @@ -112,7 +112,7 @@ def __init__(self, *args, **kwargs): # many related hyperlinked from model comments_hyperlinked = relations.HyperlinkedRelatedField( related_link_view_name="entry-related", - related_link_related_name='comments', + related_link_related_name="comments", self_link_view_name="entry-relationships", many=True, read_only=True, diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index aa48e59d..1083fd94 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -69,9 +69,7 @@ def __init__(self, self_link_view_name=None, related_link_view_name=None, **kwar "related_link_url_kwarg", self.related_link_lookup_field ) - self.related_link_related_name = kwargs.pop( - "related_link_related_name", None - ) + self.related_link_related_name = kwargs.pop("related_link_related_name", None) # We include this simply for dependency injection in tests. # We can't add it as a class attributes or it would expect an @@ -125,7 +123,9 @@ def get_links(self, obj=None, lookup_field="pk"): self_link = self.get_url("self", self.self_link_view_name, self_kwargs, request) if self.related_link_related_name: - self_kwargs.update({"related_field": format_link_segment(self.related_link_related_name)}) + self_kwargs.update( + {"related_field": format_link_segment(self.related_link_related_name)} + ) # Assuming RelatedField will be declared in two ways: # 1. url(r'^authors/(?P[^/.]+)/(?P\w+)/$', From a450de6de59308692655114e0d1d238e61bd6373 Mon Sep 17 00:00:00 2001 From: Safa AlFulaij Date: Fri, 16 Apr 2021 19:27:42 +0300 Subject: [PATCH 3/5] Use current serializer class --- example/serializers.py | 4 ++-- .../test_non_paginated_responses.py | 4 ++-- example/tests/integration/test_pagination.py | 2 +- example/tests/test_filters.py | 2 +- example/tests/test_views.py | 2 +- example/urls.py | 10 +++++----- example/urls_test.py | 10 +++++----- rest_framework_json_api/relations.py | 8 -------- rest_framework_json_api/schemas/openapi.py | 18 +++++++----------- 9 files changed, 24 insertions(+), 36 deletions(-) diff --git a/example/serializers.py b/example/serializers.py index 1a807270..4d80c87c 100644 --- a/example/serializers.py +++ b/example/serializers.py @@ -111,8 +111,8 @@ def __init__(self, *args, **kwargs): comments = relations.ResourceRelatedField(many=True, read_only=True) # many related hyperlinked from model comments_hyperlinked = relations.HyperlinkedRelatedField( - related_link_view_name="entry-related", - related_link_related_name="comments", + related_link_view_name="entry-comments", + related_link_url_kwarg="entry_pk", self_link_view_name="entry-relationships", many=True, read_only=True, diff --git a/example/tests/integration/test_non_paginated_responses.py b/example/tests/integration/test_non_paginated_responses.py index dc1d7932..5a2e59c8 100644 --- a/example/tests/integration/test_non_paginated_responses.py +++ b/example/tests/integration/test_non_paginated_responses.py @@ -42,7 +42,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): }, "commentsHyperlinked": { "links": { - "related": "http://testserver/entries/1/comments/", + "related": "http://testserver/entries/1/comments", "self": "http://testserver/entries/1/relationships/comments_hyperlinked", } }, @@ -97,7 +97,7 @@ def test_multiple_entries_no_pagination(multiple_entries, client): }, "commentsHyperlinked": { "links": { - "related": "http://testserver/entries/2/comments/", + "related": "http://testserver/entries/2/comments", "self": "http://testserver/entries/2/relationships/comments_hyperlinked", } }, diff --git a/example/tests/integration/test_pagination.py b/example/tests/integration/test_pagination.py index 6263656d..aedf8c6c 100644 --- a/example/tests/integration/test_pagination.py +++ b/example/tests/integration/test_pagination.py @@ -42,7 +42,7 @@ def test_pagination_with_single_entry(single_entry, client): }, "commentsHyperlinked": { "links": { - "related": "http://testserver/entries/1/comments/", + "related": "http://testserver/entries/1/comments", "self": "http://testserver/entries/1/relationships/comments_hyperlinked", } }, diff --git a/example/tests/test_filters.py b/example/tests/test_filters.py index df913d81..68ad1452 100644 --- a/example/tests/test_filters.py +++ b/example/tests/test_filters.py @@ -491,7 +491,7 @@ def test_search_keywords(self): "commentsHyperlinked": { "links": { "self": "http://testserver/entries/7/relationships/comments_hyperlinked", # noqa: E501 - "related": "http://testserver/entries/7/comments/", + "related": "http://testserver/entries/7/comments", } }, "suggested": { diff --git a/example/tests/test_views.py b/example/tests/test_views.py index e08b26d1..4b494b52 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -651,7 +651,7 @@ def test_get_object_gives_correct_entry(self): "commentsHyperlinked": { "links": { "related": "http://testserver/entries/{}" - "/comments/".format(self.second_entry.id), + "/comments".format(self.second_entry.id), "self": "http://testserver/entries/{}/relationships" "/comments_hyperlinked".format(self.second_entry.id), } diff --git a/example/urls.py b/example/urls.py index 6504987a..800b9f79 100644 --- a/example/urls.py +++ b/example/urls.py @@ -45,6 +45,11 @@ BlogViewSet.as_view({"get": "retrieve"}), name="entry-blog", ), + url( + r"entries/(?P[^/.]+)/comments$", + CommentViewSet.as_view({"get": "list"}), + name="entry-comments", + ), url( r"entries/(?P[^/.]+)/authors$", AuthorViewSet.as_view({"get": "list"}), @@ -55,11 +60,6 @@ EntryViewSet.as_view({"get": "retrieve"}), name="entry-featured", ), - url( - r"^entries/(?P[^/.]+)/(?P\w+)/$", - EntryViewSet.as_view({"get": "retrieve_related"}), - name="entry-related", - ), url( r"^authors/(?P[^/.]+)/(?P\w+)/$", AuthorViewSet.as_view({"get": "retrieve_related"}), diff --git a/example/urls_test.py b/example/urls_test.py index b748082f..7e875936 100644 --- a/example/urls_test.py +++ b/example/urls_test.py @@ -52,6 +52,11 @@ BlogViewSet.as_view({"get": "retrieve"}), name="entry-blog", ), + re_path( + r"^entries/(?P[^/.]+)/comments$", + CommentViewSet.as_view({"get": "list"}), + name="entry-comments", + ), re_path( r"^entries/(?P[^/.]+)/suggested/$", EntryViewSet.as_view({"get": "list"}), @@ -72,11 +77,6 @@ EntryViewSet.as_view({"get": "retrieve"}), name="entry-featured", ), - re_path( - r"^entries/(?P[^/.]+)/(?P\w+)/$", - EntryViewSet.as_view({"get": "retrieve_related"}), - name="entry-related", - ), re_path( r"^authors/(?P[^/.]+)/(?P\w+)/$", AuthorViewSet.as_view({"get": "retrieve_related"}), diff --git a/rest_framework_json_api/relations.py b/rest_framework_json_api/relations.py index 1083fd94..c9dd765d 100644 --- a/rest_framework_json_api/relations.py +++ b/rest_framework_json_api/relations.py @@ -27,7 +27,6 @@ "related_link_view_name", "related_link_lookup_field", "related_link_url_kwarg", - "related_link_related_name", ] @@ -69,8 +68,6 @@ def __init__(self, self_link_view_name=None, related_link_view_name=None, **kwar "related_link_url_kwarg", self.related_link_lookup_field ) - self.related_link_related_name = kwargs.pop("related_link_related_name", None) - # We include this simply for dependency injection in tests. # We can't add it as a class attributes or it would expect an # implicit `self` argument to be passed. @@ -122,11 +119,6 @@ def get_links(self, obj=None, lookup_field="pk"): self_kwargs.update({"related_field": format_link_segment(field_name)}) self_link = self.get_url("self", self.self_link_view_name, self_kwargs, request) - if self.related_link_related_name: - self_kwargs.update( - {"related_field": format_link_segment(self.related_link_related_name)} - ) - # Assuming RelatedField will be declared in two ways: # 1. url(r'^authors/(?P[^/.]+)/(?P\w+)/$', # AuthorViewSet.as_view({'get': 'retrieve_related'})) diff --git a/rest_framework_json_api/schemas/openapi.py b/rest_framework_json_api/schemas/openapi.py index cbf36e6f..b51f366f 100644 --- a/rest_framework_json_api/schemas/openapi.py +++ b/rest_framework_json_api/schemas/openapi.py @@ -357,15 +357,14 @@ def _expand_related(self, path, method, view, view_endpoints): ) if related_view: action = self._field_is_one_or_many(field, view) - if action: - result.append( - ( - path.replace("{related_field}", field), - method, - related_view, - action, - ) + result.append( + ( + path.replace("{related_field}", field), + method, + related_view, + action, ) + ) return result @@ -393,9 +392,6 @@ def _find_related_view(self, view_endpoints, related_serializer, parent_view): def _field_is_one_or_many(self, field, view): serializer = view.get_serializer() - if field not in serializer.fields: - return None - if isinstance(serializer.fields[field], ManyRelatedField): return "list" else: From 3579ce000bfd02f02c6a378ca36ee138459b5199 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Fri, 16 Apr 2021 23:30:22 +0400 Subject: [PATCH 4/5] Update test to overwrite get_serializer_class without fallback --- example/tests/test_views.py | 7 +++---- example/views.py | 16 +++++++--------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/example/tests/test_views.py b/example/tests/test_views.py index 4b494b52..16b51622 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -417,12 +417,11 @@ def test_get_related_serializer_class_many(self): def test_get_serializer_comes_from_included_serializers(self): kwargs = {"pk": self.author.id, "related_field": "type"} view = self._get_view(kwargs) - related_serializers = view.serializer_class.related_serializers - delattr(view.serializer_class, "related_serializers") + related_serializers = view.get_serializer_class().related_serializers + delattr(view.get_serializer_class(), "related_serializers") got = view.get_related_serializer_class() self.assertEqual(got, AuthorTypeSerializer) - - view.serializer_class.related_serializers = related_serializers + view.get_serializer_class().related_serializers = related_serializers def test_get_related_serializer_class_raises_error(self): kwargs = {"pk": self.author.id, "related_field": "unknown"} diff --git a/example/views.py b/example/views.py index 65bcb301..6a1b15a6 100644 --- a/example/views.py +++ b/example/views.py @@ -208,17 +208,15 @@ class NoFiltersetEntryViewSet(EntryViewSet): class AuthorViewSet(ModelViewSet): queryset = Author.objects.all() - serializer_classes = { - "list": AuthorListSerializer, - "retrieve": AuthorDetailSerializer, - } - serializer_class = AuthorSerializer # fallback def get_serializer_class(self): - try: - return self.serializer_classes.get(self.action, self.serializer_class) - except AttributeError: - return self.serializer_class + serializer_classes = { + "list": AuthorListSerializer, + "retrieve": AuthorDetailSerializer, + } + + action = getattr(self, "action", "") + return serializer_classes.get(action, AuthorSerializer) class CommentViewSet(ModelViewSet): From 9275ae18201fd38bb931581906ce874a0590c087 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Fri, 16 Apr 2021 23:33:23 +0400 Subject: [PATCH 5/5] Add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b816f47d..c78a1b12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,10 @@ any parts of the framework not mentioned in the documentation should generally b * Added support for Django 3.2. +### Fixed + +* Allow `get_serializer_class` to be overwritten when using related urls without defining `serializer_class` fallback + ## [4.1.0] - 2021-03-08 ### Added