Merge pull request #36 from bequest/master

Allow child one-to-one instances to be updated without providing PK (updated)
beda-software · Apr 20, 2018 · 29d1368 · 29d1368
2 parents 8b4ee4b + 49be7bd
commit 29d1368
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 1 deletion.
diff --git a/drf_writable_nested/mixins.py b/drf_writable_nested/mixins.py
@@ -125,8 +125,17 @@ def update_or_create_reverse_relations(self, instance, reverse_relations):
             if related_data is None:
                 continue
 
-            # Expand to array of one item for one-to-one for uniformity
             if related_field.one_to_one:
+                # If an object already exists, fill in the pk so we don't try to duplicate it
+                pk_name = field.Meta.model._meta.pk.attname
+                if pk_name not in related_data and 'pk' in related_data:
+                    pk_name = 'pk'
+                if pk_name not in related_data:
+                    related_instance = getattr(instance, field_source, None)
+                    if related_instance:
+                        related_data[pk_name] = related_instance.pk
+
+                # Expand to array of one item for one-to-one for uniformity
                 related_data = [related_data]
 
             instances = self.prefetch_related_instances(field, related_data)

diff --git a/tests/test_writable_nested_model_serializer.py b/tests/test_writable_nested_model_serializer.py
@@ -221,6 +221,112 @@ def test_update(self):
         # Sites shouldn't be deleted either as it is M2M
         self.assertEqual(models.Site.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)
+        user = serializer.save()
+
+        # Check instances count
+        self.assertEqual(models.User.objects.count(), 1)
+        self.assertEqual(models.Profile.objects.count(), 1)
+        self.assertEqual(models.Site.objects.count(), 2)
+        self.assertEqual(models.Avatar.objects.count(), 2)
+        self.assertEqual(models.Message.objects.count(), 3)
+
+        # Update
+        user_pk = user.pk
+        profile_pk = user.profile.pk
+
+        message_to_update_str_pk = str(user.profile.message_set.first().pk)
+        message_to_update_pk = user.profile.message_set.last().pk
+        serializer = serializers.UserSerializer(
+            instance=user,
+            data={
+                'pk': user_pk,
+                'username': 'new',
+                'profile': {
+                    # omit pk
+                    'access_key': None,
+                    'sites': [
+                        {
+                            'url': 'http://new-site.com',
+                        },
+                    ],
+                    'avatars': [
+                        {
+                            'pk': user.profile.avatars.earliest('pk').pk,
+                            'image': 'old-image-1.png',
+                        },
+                        {
+                            'image': 'new-image-1.png',
+                        },
+                        {
+                            'image': 'new-image-2.png',
+                        },
+                    ],
+                    'message_set': [
+                        {
+                            'pk': message_to_update_str_pk,
+                            'message': 'Old message 1'
+                        },
+                        {
+                            'pk': message_to_update_pk,
+                            'message': 'Old message 2'
+                        },
+                        {
+                            'message': 'New message 1'
+                        }
+                    ],
+                },
+            },
+        )
+
+        serializer.is_valid(raise_exception=True)
+        user = serializer.save()
+        user.refresh_from_db()
+        self.assertIsNotNone(user)
+        self.assertEqual(user.pk, user_pk)
+        self.assertEqual(user.username, 'new')
+
+        profile = user.profile
+        self.assertIsNotNone(profile)
+        self.assertIsNone(profile.access_key)
+        self.assertEqual(profile.pk, profile_pk)
+        self.assertEqual(profile.sites.count(), 1)
+        self.assertSetEqual(
+            set(profile.sites.values_list('url', flat=True)),
+            {'http://new-site.com'}
+        )
+        self.assertEqual(profile.avatars.count(), 3)
+        self.assertSetEqual(
+            set(profile.avatars.values_list('image', flat=True)),
+            {'old-image-1.png', 'new-image-1.png', 'new-image-2.png'}
+        )
+        self.assertSetEqual(
+            set(profile.message_set.values_list('message', flat=True)),
+            {'Old message 1', 'Old message 2', 'New message 1'}
+        )
+        # Check that message which supposed to be updated still in profile
+        # messages (new message wasn't created instead of update)
+        self.assertIn(
+            message_to_update_pk,
+            profile.message_set.values_list('id', flat=True)
+        )
+        self.assertIn(
+            uuid.UUID(message_to_update_str_pk),
+            profile.message_set.values_list('id', flat=True)
+        )
+
+        # Check instances count
+        self.assertEqual(models.User.objects.count(), 1)
+        self.assertEqual(models.Profile.objects.count(), 1)
+        self.assertEqual(models.Avatar.objects.count(), 3)
+        self.assertEqual(models.Message.objects.count(), 3)
+        # Access key shouldn't be removed because it is FK
+        self.assertEqual(models.AccessKey.objects.count(), 1)
+        # Sites shouldn't be deleted either as it is M2M
+        self.assertEqual(models.Site.objects.count(), 3)
+
     def test_update_raise_protected_error(self):
         serializer = serializers.UserSerializer(data=self.get_initial_data())
         serializer.is_valid(raise_exception=True)