From a617a3758f635bf1ebe3464555e397d09f4dfc6d Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Mon, 31 Dec 2012 14:33:24 +0100 Subject: [PATCH 1/3] Don't persist relation changes in ModelSerializer#restore_object() --- rest_framework/serializers.py | 34 ++++++++++----------- rest_framework/tests/relations_hyperlink.py | 6 ++-- rest_framework/tests/relations_pk.py | 20 +++++++++--- 3 files changed, 35 insertions(+), 25 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index ed173d85c..24674f2a6 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -498,28 +498,28 @@ class ModelSerializer(Serializer): self.m2m_data = {} self.related_data = {} + # Reverse fk relations + for (obj, model) in self.opts.model._meta.get_all_related_objects_with_model(): + field_name = obj.field.related_query_name() + if field_name in attrs: + self.related_data[field_name] = attrs.pop(field_name) + + # Reverse m2m relations + for (obj, model) in self.opts.model._meta.get_all_related_m2m_objects_with_model(): + field_name = obj.field.related_query_name() + if field_name in attrs: + self.m2m_data[field_name] = attrs.pop(field_name) + + # Forward m2m relations + for field in self.opts.model._meta.many_to_many: + if field.name in attrs: + self.m2m_data[field.name] = attrs.pop(field.name) + if instance is not None: for key, val in attrs.items(): setattr(instance, key, val) else: - # Reverse fk relations - for (obj, model) in self.opts.model._meta.get_all_related_objects_with_model(): - field_name = obj.field.related_query_name() - if field_name in attrs: - self.related_data[field_name] = attrs.pop(field_name) - - # Reverse m2m relations - for (obj, model) in self.opts.model._meta.get_all_related_m2m_objects_with_model(): - field_name = obj.field.related_query_name() - if field_name in attrs: - self.m2m_data[field_name] = attrs.pop(field_name) - - # Forward m2m relations - for field in self.opts.model._meta.many_to_many: - if field.name in attrs: - self.m2m_data[field.name] = attrs.pop(field.name) - instance = self.opts.model(**attrs) try: diff --git a/rest_framework/tests/relations_hyperlink.py b/rest_framework/tests/relations_hyperlink.py index 240394105..4179ec680 100644 --- a/rest_framework/tests/relations_hyperlink.py +++ b/rest_framework/tests/relations_hyperlink.py @@ -114,8 +114,8 @@ class HyperlinkedManyToManyTests(TestCase): instance = ManyToManySource.objects.get(pk=1) serializer = ManyToManySourceSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) - self.assertEquals(serializer.data, data) serializer.save() + self.assertEquals(serializer.data, data) # Ensure source 1 is updated, and everything else is as expected queryset = ManyToManySource.objects.all() @@ -132,8 +132,8 @@ class HyperlinkedManyToManyTests(TestCase): instance = ManyToManyTarget.objects.get(pk=1) serializer = ManyToManyTargetSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) - self.assertEquals(serializer.data, data) serializer.save() + self.assertEquals(serializer.data, data) # Ensure target 1 is updated, and everything else is as expected queryset = ManyToManyTarget.objects.all() @@ -239,8 +239,8 @@ class HyperlinkedForeignKeyTests(TestCase): instance = ForeignKeyTarget.objects.get(pk=2) serializer = ForeignKeyTargetSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) - self.assertEquals(serializer.data, data) serializer.save() + self.assertEquals(serializer.data, data) # Ensure target 2 is update, and everything else is as expected queryset = ForeignKeyTarget.objects.all() diff --git a/rest_framework/tests/relations_pk.py b/rest_framework/tests/relations_pk.py index 01109ef95..289670999 100644 --- a/rest_framework/tests/relations_pk.py +++ b/rest_framework/tests/relations_pk.py @@ -99,8 +99,8 @@ class PKManyToManyTests(TestCase): instance = ManyToManySource.objects.get(pk=1) serializer = ManyToManySourceSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) - self.assertEquals(serializer.data, data) serializer.save() + self.assertEquals(serializer.data, data) # Ensure source 1 is updated, and everything else is as expected queryset = ManyToManySource.objects.all() @@ -117,8 +117,8 @@ class PKManyToManyTests(TestCase): instance = ManyToManyTarget.objects.get(pk=1) serializer = ManyToManyTargetSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) - self.assertEquals(serializer.data, data) serializer.save() + self.assertEquals(serializer.data, data) # Ensure target 1 is updated, and everything else is as expected queryset = ManyToManyTarget.objects.all() @@ -221,8 +221,18 @@ class PKForeignKeyTests(TestCase): instance = ForeignKeyTarget.objects.get(pk=2) serializer = ForeignKeyTargetSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) - self.assertEquals(serializer.data, data) + # We shouldn't have saved anything to the db yet since save + # hasn't been called. + queryset = ForeignKeyTarget.objects.all() + new_serializer = ForeignKeyTargetSerializer(queryset) + expected = [ + {'id': 1, 'name': u'target-1', 'sources': [1, 2, 3]}, + {'id': 2, 'name': u'target-2', 'sources': []}, + ] + self.assertEquals(new_serializer.data, expected) + serializer.save() + self.assertEquals(serializer.data, data) # Ensure target 2 is update, and everything else is as expected queryset = ForeignKeyTarget.objects.all() @@ -241,7 +251,7 @@ class PKForeignKeyTests(TestCase): self.assertEquals(serializer.data, data) self.assertEqual(obj.name, u'source-4') - # Ensure source 1 is updated, and everything else is as expected + # Ensure source 4 is added, and everything else is as expected queryset = ForeignKeySource.objects.all() serializer = ForeignKeySourceSerializer(queryset) expected = [ @@ -260,7 +270,7 @@ class PKForeignKeyTests(TestCase): self.assertEquals(serializer.data, data) self.assertEqual(obj.name, u'target-3') - # Ensure target 4 is added, and everything else is as expected + # Ensure target 3 is added, and everything else is as expected queryset = ForeignKeyTarget.objects.all() serializer = ForeignKeyTargetSerializer(queryset) expected = [ From 44771e81b23ca8ef982e4bb0d0ac5a435b684b22 Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Tue, 1 Jan 2013 17:51:39 +0100 Subject: [PATCH 2/3] Update HyperlinkedForeignKeyTests to match PKForeignKeyTests --- rest_framework/tests/relations_hyperlink.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/rest_framework/tests/relations_hyperlink.py b/rest_framework/tests/relations_hyperlink.py index 4179ec680..0a7ea0f48 100644 --- a/rest_framework/tests/relations_hyperlink.py +++ b/rest_framework/tests/relations_hyperlink.py @@ -239,6 +239,16 @@ class HyperlinkedForeignKeyTests(TestCase): instance = ForeignKeyTarget.objects.get(pk=2) serializer = ForeignKeyTargetSerializer(instance, data=data) self.assertTrue(serializer.is_valid()) + # We shouldn't have saved anything to the db yet since save + # hasn't been called. + queryset = ForeignKeyTarget.objects.all() + new_serializer = ForeignKeyTargetSerializer(queryset) + expected = [ + {'url': '/foreignkeytarget/1/', 'name': u'target-1', 'sources': ['/foreignkeysource/1/', '/foreignkeysource/2/', '/foreignkeysource/3/']}, + {'url': '/foreignkeytarget/2/', 'name': u'target-2', 'sources': []}, + ] + self.assertEquals(new_serializer.data, expected) + serializer.save() self.assertEquals(serializer.data, data) From f62acf025e938d14b1e74d3cecb4f42975793e9a Mon Sep 17 00:00:00 2001 From: Mark Aaron Shirley Date: Tue, 1 Jan 2013 18:06:22 +0100 Subject: [PATCH 3/3] Update release notes --- docs/topics/release-notes.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/topics/release-notes.md b/docs/topics/release-notes.md index c93eebac8..5b34bf3de 100644 --- a/docs/topics/release-notes.md +++ b/docs/topics/release-notes.md @@ -16,6 +16,10 @@ Major version numbers (x.0.0) are reserved for project milestones. No major poi ## 2.1.x series +### Master + +* Relation changes are no longer persisted in `.restore_object` + ### 2.1.14 **Date**: 31st Dec 2012