From ff191c89150f18bc87ad177c76e25e2bf90d71f8 Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Thu, 5 Nov 2020 19:23:28 +0100 Subject: [PATCH 1/9] This attempt to fix the issues breaks tests An attempt to fix #859, which unfortunately breaks two tests in test_views. --- example/tests/test_views.py | 14 +++++++------- rest_framework_json_api/views.py | 13 +++++++++++-- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/example/tests/test_views.py b/example/tests/test_views.py index 9cd493f7..25eeca89 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -367,16 +367,16 @@ def test_get_related_instance_model_field(self): got = view.get_related_instance() self.assertEqual(got, self.author.id) - def test_get_serializer_class(self): + def test_get_related_serializer_class(self): kwargs = {'pk': self.author.id, 'related_field': 'bio'} view = self._get_view(kwargs) - got = view.get_serializer_class() + got = view.get_related_serializer_class() self.assertEqual(got, AuthorBioSerializer) - def test_get_serializer_class_many(self): + def test_get_related_serializer_class_many(self): kwargs = {'pk': self.author.id, 'related_field': 'entries'} view = self._get_view(kwargs) - got = view.get_serializer_class() + got = view.get_related_serializer_class() self.assertEqual(got, EntrySerializer) def test_get_serializer_comes_from_included_serializers(self): @@ -384,15 +384,15 @@ def test_get_serializer_comes_from_included_serializers(self): view = self._get_view(kwargs) related_serializers = view.serializer_class.related_serializers delattr(view.serializer_class, 'related_serializers') - got = view.get_serializer_class() + got = view.get_related_serializer_class() self.assertEqual(got, AuthorTypeSerializer) view.serializer_class.related_serializers = related_serializers - def test_get_serializer_class_raises_error(self): + def test_get_related_serializer_class_raises_error(self): kwargs = {'pk': self.author.id, 'related_field': 'unknown'} view = self._get_view(kwargs) - self.assertRaises(NotFound, view.get_serializer_class) + self.assertRaises(NotFound, view.get_related_serializer_class) def test_retrieve_related_single_reverse_lookup(self): url = reverse('author-related', kwargs={'pk': self.author.pk, 'related_field': 'bio'}) diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index 2e0b22ef..3bc057a4 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -144,10 +144,19 @@ def retrieve_related(self, request, *args, **kwargs): if isinstance(instance, Iterable): serializer_kwargs['many'] = True - serializer = self.get_serializer(instance, **serializer_kwargs) + serializer = self.get_related_serializer(instance, **serializer_kwargs) return Response(serializer.data) - def get_serializer_class(self): + def get_related_serializer(self, *args, **kwargs): + """ + Return the serializer instance that should be used for validating and + deserializing input, and for serializing output. + """ + serializer_class = self.get_related_serializer_class() + kwargs.setdefault('context', self.get_serializer_context()) + return serializer_class(*args, **kwargs) + + def get_related_serializer_class(self): parent_serializer_class = super(RelatedMixin, self).get_serializer_class() if 'related_field' in self.kwargs: From 9f69484cb6bbf4fc4cebd2882460b07c53f97341 Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Fri, 6 Nov 2020 13:11:37 +0100 Subject: [PATCH 2/9] Removed misleading comment Tha basic method was copied over from the GenericAPIView. The comment here does not fit, though. --- rest_framework_json_api/views.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index 3bc057a4..d72b9643 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -148,10 +148,6 @@ def retrieve_related(self, request, *args, **kwargs): return Response(serializer.data) def get_related_serializer(self, *args, **kwargs): - """ - Return the serializer instance that should be used for validating and - deserializing input, and for serializing output. - """ serializer_class = self.get_related_serializer_class() kwargs.setdefault('context', self.get_serializer_context()) return serializer_class(*args, **kwargs) From a2b57046f87051f917bc8f733eb17b1d7cb72c25 Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Fri, 6 Nov 2020 15:33:04 +0100 Subject: [PATCH 3/9] bein more explicit about the changes --- rest_framework_json_api/views.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index d72b9643..7c874e7a 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -147,10 +147,10 @@ def retrieve_related(self, request, *args, **kwargs): serializer = self.get_related_serializer(instance, **serializer_kwargs) return Response(serializer.data) - def get_related_serializer(self, *args, **kwargs): + def get_related_serializer(self, instance, **kwargs): serializer_class = self.get_related_serializer_class() kwargs.setdefault('context', self.get_serializer_context()) - return serializer_class(*args, **kwargs) + return serializer_class(instance, **kwargs) def get_related_serializer_class(self): parent_serializer_class = super(RelatedMixin, self).get_serializer_class() @@ -184,7 +184,8 @@ def get_related_field_name(self): def get_related_instance(self): parent_obj = self.get_object() - parent_serializer = self.serializer_class(parent_obj) + parent_serializer_class = self.get_serializer_class() + parent_serializer = parent_serializer_class(parent_obj) field_name = self.get_related_field_name() field = parent_serializer.fields.get(field_name, None) From 5c4cb88a38ef809f182c5fc6eb25ee6d3db74fd6 Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Mon, 9 Nov 2020 10:06:21 +0100 Subject: [PATCH 4/9] Use correct serializer for rendering Ensure that the correct resource is returned during rendering, which in turn requires to use the correct serializer, depending on if it is a related resource or the parent resource. sliverc pointed out the issue here. --- rest_framework_json_api/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rest_framework_json_api/utils.py b/rest_framework_json_api/utils.py index 2443575f..b99de91a 100644 --- a/rest_framework_json_api/utils.py +++ b/rest_framework_json_api/utils.py @@ -52,7 +52,10 @@ def get_resource_name(context, expand_polymorphic_types=False): resource_name = getattr(view, 'resource_name') except AttributeError: try: - serializer = view.get_serializer_class() + if 'kwargs' in context and 'related_field' in context['kwargs']: + serializer = view.get_related_serializer_class() + else: + serializer = view.get_serializer_class() if expand_polymorphic_types and issubclass(serializer, PolymorphicModelSerializer): return serializer.get_polymorphic_types() else: From 0366ceb81db63d01de2248391155aa84de2ffc1f Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Mon, 9 Nov 2020 15:40:41 +0100 Subject: [PATCH 5/9] Add provision for tests --- example/serializers.py | 6 ++++++ example/views.py | 13 ++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/example/serializers.py b/example/serializers.py index 9dc84a4a..257c752b 100644 --- a/example/serializers.py +++ b/example/serializers.py @@ -260,6 +260,12 @@ class Meta: def get_first_entry(self, obj): return obj.entries.first() +class AuthorListSerializer(AuthorSerializer): + pass + +class AuthorDetailSerializer(AuthorSerializer): + pass + class WriterSerializer(serializers.ModelSerializer): included_serializers = { diff --git a/example/views.py b/example/views.py index 8c80d145..8d303f93 100644 --- a/example/views.py +++ b/example/views.py @@ -16,6 +16,8 @@ from example.models import Author, Blog, Comment, Company, Entry, Project, ProjectType from example.serializers import ( AuthorSerializer, + AuthorListSerializer, + AuthorDetailSerializer, BlogDRFSerializer, BlogSerializer, CommentSerializer, @@ -185,7 +187,16 @@ class NoFiltersetEntryViewSet(EntryViewSet): class AuthorViewSet(ModelViewSet): queryset = Author.objects.all() - serializer_class = AuthorSerializer + 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 class CommentViewSet(ModelViewSet): From 7e577d9369d1570211c1ceb00f2fc5dd5be1961d Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Tue, 10 Nov 2020 08:07:16 +0100 Subject: [PATCH 6/9] Describe the fix. --- AUTHORS | 1 + CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/AUTHORS b/AUTHORS index b440df81..d4c03b2c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -33,4 +33,5 @@ Sergey Kolomenkin Stas S. Tim Selman Tom Glowka +Ulrich Schuster Yaniv Peer diff --git a/CHANGELOG.md b/CHANGELOG.md index 12b3e805..c56f304e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ any parts of the framework not mentioned in the documentation should generally b ### Added * Ability for the user to select `included_serializers` to apply when using `BrowsableAPI`, based on available `included_serializers` defined for the current endpoint. +* Fixed #859: Allow users to overwrite a view's `get_serializer()` and `get_serializer_class()` methods for views that have related fields. ## [4.0.0] - 2020-10-31 From 67d79ff392f9081647bac1c0e0994f845bec928a Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Tue, 10 Nov 2020 20:07:03 +0100 Subject: [PATCH 7/9] Fixed failing tests As pointed out by n2ygk, the schema names here must reflect the Serializer class names, which I changed in the two affected test cases. --- example/tests/snapshots/snap_test_openapi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/example/tests/snapshots/snap_test_openapi.py b/example/tests/snapshots/snap_test_openapi.py index ec8da388..aca41d70 100644 --- a/example/tests/snapshots/snap_test_openapi.py +++ b/example/tests/snapshots/snap_test_openapi.py @@ -65,7 +65,7 @@ "properties": { "data": { "items": { - "$ref": "#/components/schemas/Author" + "$ref": "#/components/schemas/AuthorList" }, "type": "array" }, @@ -171,7 +171,7 @@ "schema": { "properties": { "data": { - "$ref": "#/components/schemas/Author" + "$ref": "#/components/schemas/AuthorDetail" }, "included": { "items": { From 41785b859860b3985df4070c2aba707a658f9775 Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Wed, 11 Nov 2020 17:06:52 +0100 Subject: [PATCH 8/9] Update CHANGELOG.md Clarified changelog entry --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c56f304e..4613aa18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,10 @@ any parts of the framework not mentioned in the documentation should generally b ### Added * Ability for the user to select `included_serializers` to apply when using `BrowsableAPI`, based on available `included_serializers` defined for the current endpoint. -* Fixed #859: Allow users to overwrite a view's `get_serializer()` and `get_serializer_class()` methods for views that have related fields. + +### Fixed + +* Allow users to overwrite a view's `get_serializer_class()` method when using [related urls](https://django-rest-framework-json-api.readthedocs.io/en/stable/usage.html#related-urls) ## [4.0.0] - 2020-10-31 From e27b7a4895c3610ae15b7cb13af813f9b8806202 Mon Sep 17 00:00:00 2001 From: Ulrich Schuster Date: Wed, 11 Nov 2020 17:32:47 +0100 Subject: [PATCH 9/9] Fixed linting issues --- example/serializers.py | 2 ++ example/views.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/example/serializers.py b/example/serializers.py index 257c752b..566f39d5 100644 --- a/example/serializers.py +++ b/example/serializers.py @@ -260,9 +260,11 @@ class Meta: def get_first_entry(self, obj): return obj.entries.first() + class AuthorListSerializer(AuthorSerializer): pass + class AuthorDetailSerializer(AuthorSerializer): pass diff --git a/example/views.py b/example/views.py index 8d303f93..99a54193 100644 --- a/example/views.py +++ b/example/views.py @@ -15,9 +15,9 @@ from example.models import Author, Blog, Comment, Company, Entry, Project, ProjectType from example.serializers import ( - AuthorSerializer, - AuthorListSerializer, AuthorDetailSerializer, + AuthorListSerializer, + AuthorSerializer, BlogDRFSerializer, BlogSerializer, CommentSerializer, @@ -190,7 +190,7 @@ class AuthorViewSet(ModelViewSet): serializer_classes = { "list": AuthorListSerializer, "retrieve": AuthorDetailSerializer} - serializer_class = AuthorSerializer # fallback + serializer_class = AuthorSerializer # fallback def get_serializer_class(self): try: