From b655fdaa44a3dadd303da214b3ff66eab4de53ab Mon Sep 17 00:00:00 2001 From: Bailey Lind-Trefts Date: Mon, 23 Dec 2019 17:32:43 -0800 Subject: [PATCH 1/2] When updating an instance with foreign key relations, the objects linking to the instance should not be deleted. Instead, the foreign key should simply be removed from the objects to the instance. This commit checks if the related field is a foreign key, and, if so, it unlinks the object. If the field cannot be null, the object gets deleted. --- drf_writable_nested/mixins.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drf_writable_nested/mixins.py b/drf_writable_nested/mixins.py index 34d26f3..26e0e65 100644 --- a/drf_writable_nested/mixins.py +++ b/drf_writable_nested/mixins.py @@ -333,6 +333,12 @@ def delete_reverse_relations_if_need(self, instance, reverse_relations): # Remove relations from m2m table m2m_manager = getattr(instance, field_source) m2m_manager.remove(*pks_to_delete) + elif related_field.many_to_one and related_field.null: + # The instance should not be deleted if the object is foreign-key related. + unlink_foreign_key = { + related_field.name: None, + } + model_class.objects.filter(pk__in=pks_to_delete).update(**unlink_foreign_key) else: model_class.objects.filter(pk__in=pks_to_delete).delete() From 771a4f768e624222f1942cf96b10f53410951103 Mon Sep 17 00:00:00 2001 From: Bailey Lind-Trefts Date: Wed, 1 Jan 2020 11:57:52 -0800 Subject: [PATCH 2/2] Added test to demonstrate that messages with a nullable profile foreign key, i.e. s, are not deleted when they are removed from a profile. --- tests/models.py | 14 +++++- tests/serializers.py | 21 +++++++++ .../test_writable_nested_model_serializer.py | 43 +++++++++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/tests/models.py b/tests/models.py index 4455d97..948d9b8 100644 --- a/tests/models.py +++ b/tests/models.py @@ -65,15 +65,25 @@ class CustomPK(models.Model): ) -class Message(models.Model): +class AbstractMessage(models.Model): id = models.UUIDField( primary_key=True, default=uuid.uuid4, editable=False ) - profile = models.ForeignKey(Profile, on_delete=models.CASCADE) message = models.CharField(max_length=100) + class Meta: + abstract = True + + +class Message(AbstractMessage): + profile = models.ForeignKey(Profile, on_delete=models.CASCADE) + + +class PersistentMessage(AbstractMessage): + profile = models.ForeignKey(Profile, on_delete=models.CASCADE, null=True) + class AnotherProfile(models.Model): sites = models.ManyToManyField(Site) diff --git a/tests/serializers.py b/tests/serializers.py index fdb68b2..0110954 100644 --- a/tests/serializers.py +++ b/tests/serializers.py @@ -21,6 +21,13 @@ class Meta: fields = ('pk', 'message',) +class PersistentMessageSerializer(serializers.ModelSerializer): + + class Meta: + model = models.PersistentMessage + fields = ('pk', 'message',) + + class SiteSerializer(serializers.ModelSerializer): url = serializers.CharField() @@ -54,6 +61,13 @@ class Meta: fields = ('pk', 'sites', 'avatars', 'access_key', 'message_set',) +class PersistentProfileSerializer(ProfileSerializer): + message_set = PersistentMessageSerializer(many=True) + + class Meta(ProfileSerializer.Meta): + pass + + class UserSerializer(WritableNestedModelSerializer): # Reverse OneToOne relation profile = ProfileSerializer(required=False, allow_null=True) @@ -64,6 +78,13 @@ class Meta: fields = ('pk', 'profile', 'username', 'user_avatar') +class PersistentUserSerializer(WritableNestedModelSerializer): + profile = PersistentProfileSerializer(required=False, allow_null=True) + + class Meta(UserSerializer.Meta): + pass + + class CustomSerializer(UserSerializer): # Simulate having non-modelfield information on the serializer custom_field = serializers.CharField() diff --git a/tests/test_writable_nested_model_serializer.py b/tests/test_writable_nested_model_serializer.py index d75bf74..ffe8e4b 100644 --- a/tests/test_writable_nested_model_serializer.py +++ b/tests/test_writable_nested_model_serializer.py @@ -224,6 +224,49 @@ def test_update(self): # Sites shouldn't be deleted either as it is M2M self.assertEqual(models.Site.objects.count(), 3) + def test_update_no_delete_if_null_is_false(self): + serializer = serializers.UserSerializer(data=self.get_initial_data()) + serializer.is_valid(raise_exception=True) + user = serializer.save() + + # Check instances count + self.assertEqual(models.User.objects.count(), 1) + self.assertEqual(models.Message.objects.count(), 3) + + # Update + user_pk = user.pk + profile_pk = user.profile.pk + + serializer = serializers.PersistentUserSerializer( + data={ + 'pk': user_pk, + 'username': 'new', + 'profile': { + 'pk': profile_pk, + 'access_key': None, + 'sites': [ + { + 'url': 'http://new-site.com', + }, + ], + 'avatars': [ + ], + 'message_set': [ + ], + }, + }, + instance=user, + ) + + serializer.is_valid(raise_exception=True) + user = serializer.save() + user.refresh_from_db() + + # Check instances count + self.assertEqual(models.User.objects.count(), 1) + self.assertEqual(models.Avatar.objects.count(), 0) + self.assertEqual(models.Message.objects.count(), 3) + def test_update_reverse_one_to_one_without_pk(self): serializer = serializers.UserSerializer(data=self.get_initial_data()) serializer.is_valid(raise_exception=True)