From dfaa30dc6541f37d04e3f31b598b22a92a8d6cbe Mon Sep 17 00:00:00 2001 From: ali_cramstack Date: Mon, 22 Oct 2018 20:01:12 +0600 Subject: [PATCH 1/2] Added test to check for relationship view relationship types and patch accordingly and updated relationship functionality for related fields --- example/tests/test_views.py | 38 +++++++++++++++++++++++++++++++- rest_framework_json_api/views.py | 24 +++++++++++++++++++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/example/tests/test_views.py b/example/tests/test_views.py index 48e1bfa6..e4bf700f 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -11,7 +11,7 @@ from . import TestBase from .. import views -from example.factories import AuthorFactory, EntryFactory +from example.factories import AuthorFactory, EntryFactory, CommentFactory from example.models import Author, Blog, Comment, Entry from example.serializers import AuthorBioSerializer, AuthorTypeSerializer, EntrySerializer from example.views import AuthorViewSet @@ -229,6 +229,42 @@ def test_delete_to_many_relationship_with_change(self): response = self.client.delete(url, data=request_data) assert response.status_code == 200, response.content.decode() + def test_new_comment_data_patch_to_many_relationship(self): + entry = EntryFactory(blog=self.blog, authors=(self.author,)) + comment = CommentFactory(entry=entry) + + url = '/authors/{}/relationships/comment_set'.format(self.author.id) + request_data = { + 'data': [{'type': format_resource_type('Comment'), 'id': str(comment.id)}, ] + } + previous_response = { + 'data': [ + {'type': 'comments', + 'id': f'{self.second_comment.id}' + } + ], + 'links': {'self': f'http://testserver/authors/{self.author.id}/relationships/comment_set'} + } + + response = self.client.get(url) + assert response.status_code == 200 + assert response.json() == previous_response + + new_patched_response = { + 'data': [ + {'type': 'comments', + 'id': f'{comment.id}' + } + ], + 'links': {'self': f'http://testserver/authors/{self.author.id}/relationships/comment_set'} + } + + response = self.client.patch(url, data=request_data) + assert response.status_code == 200 + assert response.json() == new_patched_response + + assert Comment.objects.filter(id=self.second_comment.id).exists + class TestRelatedMixin(APITestCase): diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index f8766bc4..224e4d8b 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -251,6 +251,18 @@ def get(self, request, *args, **kwargs): serializer_instance = self._instantiate_serializer(related_instance) return Response(serializer_instance.data) + def remove_relationships(self, instance_manager, field): + field_object = getattr(instance_manager, field) + + if getattr(field_object, "null"): + for obj in instance_manager.all(): + setattr(obj, field_object.name, None) + obj.save() + else: + instance_manager.all().delete() + + return instance_manager + def patch(self, request, *args, **kwargs): parent_obj = self.get_object() related_instance_or_manager = self.get_related_instance() @@ -261,7 +273,17 @@ def patch(self, request, *args, **kwargs): data=request.data, model_class=related_model_class, many=True ) serializer.is_valid(raise_exception=True) - related_instance_or_manager.all().delete() + # related_instance_or_manager.all().delete() + + # for to one + if hasattr(related_instance_or_manager, "field"): + related_instance_or_manager = self.remove_relationships(instance_manager=related_instance_or_manager, + field="field") + # for to many + else: + related_instance_or_manager = self.remove_relationships(instance_manager=related_instance_or_manager, + field="target_field") + # have to set bulk to False since data isn't saved yet class_name = related_instance_or_manager.__class__.__name__ if class_name != 'ManyRelatedManager': From 46a386ecb172f16683ca10a0d44174009d9aac0c Mon Sep 17 00:00:00 2001 From: Oliver Sauder Date: Thu, 25 Oct 2018 16:27:25 +0200 Subject: [PATCH 2/2] Add changelog entry for patch RelationshipView patch --- CHANGELOG.md | 1 + example/tests/test_views.py | 20 ++++++++++++++------ rest_framework_json_api/views.py | 11 +++++------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49114a36..c60329ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ any parts of the framework not mentioned in the documentation should generally b ### Fixed * Pass context from `PolymorphicModelSerializer` to child serializers to support fields which require a `request` context such as `url`. +* Avoid patch on `RelationshipView` deleting relationship instance when constraint would allow null ([#242](https://github.com/django-json-api/django-rest-framework-json-api/issues/242)) ## [2.6.0] - 2018-09-20 diff --git a/example/tests/test_views.py b/example/tests/test_views.py index e4bf700f..5b778b90 100644 --- a/example/tests/test_views.py +++ b/example/tests/test_views.py @@ -11,7 +11,7 @@ from . import TestBase from .. import views -from example.factories import AuthorFactory, EntryFactory, CommentFactory +from example.factories import AuthorFactory, CommentFactory, EntryFactory from example.models import Author, Blog, Comment, Entry from example.serializers import AuthorBioSerializer, AuthorTypeSerializer, EntrySerializer from example.views import AuthorViewSet @@ -240,10 +240,14 @@ def test_new_comment_data_patch_to_many_relationship(self): previous_response = { 'data': [ {'type': 'comments', - 'id': f'{self.second_comment.id}' + 'id': str(self.second_comment.id) } ], - 'links': {'self': f'http://testserver/authors/{self.author.id}/relationships/comment_set'} + 'links': { + 'self': 'http://testserver/authors/{}/relationships/comment_set'.format( + self.author.id + ) + } } response = self.client.get(url) @@ -253,17 +257,21 @@ def test_new_comment_data_patch_to_many_relationship(self): new_patched_response = { 'data': [ {'type': 'comments', - 'id': f'{comment.id}' + 'id': str(comment.id) } ], - 'links': {'self': f'http://testserver/authors/{self.author.id}/relationships/comment_set'} + 'links': { + 'self': 'http://testserver/authors/{}/relationships/comment_set'.format( + self.author.id + ) + } } response = self.client.patch(url, data=request_data) assert response.status_code == 200 assert response.json() == new_patched_response - assert Comment.objects.filter(id=self.second_comment.id).exists + assert Comment.objects.filter(id=self.second_comment.id).exists() class TestRelatedMixin(APITestCase): diff --git a/rest_framework_json_api/views.py b/rest_framework_json_api/views.py index 224e4d8b..9f3178a1 100644 --- a/rest_framework_json_api/views.py +++ b/rest_framework_json_api/views.py @@ -254,7 +254,7 @@ def get(self, request, *args, **kwargs): def remove_relationships(self, instance_manager, field): field_object = getattr(instance_manager, field) - if getattr(field_object, "null"): + if field_object.null: for obj in instance_manager.all(): setattr(obj, field_object.name, None) obj.save() @@ -273,16 +273,15 @@ def patch(self, request, *args, **kwargs): data=request.data, model_class=related_model_class, many=True ) serializer.is_valid(raise_exception=True) - # related_instance_or_manager.all().delete() # for to one if hasattr(related_instance_or_manager, "field"): - related_instance_or_manager = self.remove_relationships(instance_manager=related_instance_or_manager, - field="field") + related_instance_or_manager = self.remove_relationships( + instance_manager=related_instance_or_manager, field="field") # for to many else: - related_instance_or_manager = self.remove_relationships(instance_manager=related_instance_or_manager, - field="target_field") + related_instance_or_manager = self.remove_relationships( + instance_manager=related_instance_or_manager, field="target_field") # have to set bulk to False since data isn't saved yet class_name = related_instance_or_manager.__class__.__name__