From 775013864380c1443813e1ffdfdd7e8c49ed061e Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Thu, 6 Mar 2014 07:47:11 +0100 Subject: [PATCH 1/7] Fixed the many to many behavior. --- rest_framework/serializers.py | 1 + .../tests/test_serializer_nested.py | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 10256d479..c788dec16 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -992,6 +992,7 @@ class ModelSerializer(Serializer): if getattr(obj, '_m2m_data', None): for accessor_name, object_list in obj._m2m_data.items(): + [self.save_object(o) for o in object_list] setattr(obj, accessor_name, object_list) del(obj._m2m_data) diff --git a/rest_framework/tests/test_serializer_nested.py b/rest_framework/tests/test_serializer_nested.py index 6d69ffbd0..fcd379bc0 100644 --- a/rest_framework/tests/test_serializer_nested.py +++ b/rest_framework/tests/test_serializer_nested.py @@ -345,3 +345,31 @@ class NestedModelSerializerUpdateTests(TestCase): result = deserialize.object result.save() self.assertEqual(result.id, john.id) + + def test_creation_with_nested_many_to_many_relation(self): + class ManyToManyTargetSerializer(serializers.ModelSerializer): + class Meta: + model = models.ManyToManyTarget + + class ManyToManySourceSerializer(serializers.ModelSerializer): + targets = ManyToManyTargetSerializer(many=True, allow_add_remove=True) + class Meta: + model = models.ManyToManySource + + data = { + 'name': 'source', + 'targets': [{ + 'name': 'target1' + }, { + 'name': 'another target' + }] + } + + source_count = models.ManyToManySource.objects.count() + target_count = models.ManyToManyTarget.objects.count() + + deserialize = ManyToManySourceSerializer(data=data) + self.assertTrue(deserialize.is_valid(), deserialize.errors) + deserialize.save() + self.assertEqual(models.ManyToManySource.objects.count(), source_count + 1) + self.assertEqual(models.ManyToManyTarget.objects.count(), target_count + 2) From 4160bbcf72947164b1757dd6a83f514d56422dd9 Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Fri, 11 Apr 2014 23:13:43 +0200 Subject: [PATCH 2/7] Fixed the reversed relation for virtual fields (generic foreign keys). --- rest_framework/serializers.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index c788dec16..6a3432240 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -919,6 +919,7 @@ class ModelSerializer(Serializer): Restore the model instance. """ m2m_data = {} + virtual_m2m_data = {} related_data = {} nested_forward_relations = {} meta = self.opts.model._meta @@ -936,10 +937,16 @@ class ModelSerializer(Serializer): m2m_data[field_name] = attrs.pop(field_name) # Forward m2m relations - for field in meta.many_to_many + meta.virtual_fields: + for field in meta.many_to_many: if field.name in attrs: m2m_data[field.name] = attrs.pop(field.name) + # Virtual m2m relations + # This is mostly for GenericRelations + for field in meta.virtual_fields: + if field.name in attrs: + virtual_m2m_data[field.name] = attrs.pop(field.name) + # Nested forward relations - These need to be marked so we can save # them before saving the parent model instance. for field_name in attrs.keys(): @@ -964,6 +971,7 @@ class ModelSerializer(Serializer): # at the point of save. instance._related_data = related_data instance._m2m_data = m2m_data + instance._virtual_m2m_data = virtual_m2m_data instance._nested_forward_relations = nested_forward_relations return instance @@ -992,10 +1000,18 @@ class ModelSerializer(Serializer): if getattr(obj, '_m2m_data', None): for accessor_name, object_list in obj._m2m_data.items(): + # We need to save m2m data before linking the objects together [self.save_object(o) for o in object_list] setattr(obj, accessor_name, object_list) del(obj._m2m_data) + if getattr(obj, '_virtual_m2m_data', None): + for accessor_name, object_list in obj._virtual_m2m_data.items(): + # In case of GenericRelation, we have a FK as constraint + setattr(obj, accessor_name, object_list) + [self.save_object(o) for o in object_list] + del(obj._m2m_data) + if getattr(obj, '_related_data', None): related_fields = dict([ (field.get_accessor_name(), field) From 9f9ad964cd5c6999f365cc56267fe607feb9386a Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Wed, 30 Apr 2014 22:59:43 +0200 Subject: [PATCH 3/7] Removed the former virtual field list. --- rest_framework/serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 03ac78bea..131aeed31 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -930,7 +930,6 @@ class ModelSerializer(Serializer): Restore the model instance. """ m2m_data = {} - virtual_m2m_data = {} related_data = {} nested_forward_relations = {} meta = self.opts.model._meta From 5dab5e4dcf0920093377ca06544386ea598de84b Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Wed, 30 Apr 2014 23:51:07 +0200 Subject: [PATCH 4/7] Deal with reversed GenericFK. --- rest_framework/serializers.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 131aeed31..6146864f7 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -16,7 +16,7 @@ import datetime import inspect import types from decimal import Decimal -from django.contrib.contenttypes.generic import GenericForeignKey +from django.contrib.contenttypes.generic import GenericForeignKey, GenericRelation from django.core.paginator import Page from django.db import models from django.forms import widgets @@ -932,6 +932,7 @@ class ModelSerializer(Serializer): m2m_data = {} related_data = {} nested_forward_relations = {} + generic_fk = [] meta = self.opts.model._meta # Reverse fk or one-to-one relations @@ -950,6 +951,8 @@ class ModelSerializer(Serializer): for field in meta.many_to_many + meta.virtual_fields: if isinstance(field, GenericForeignKey): continue + if isinstance(field, GenericRelation): + generic_fk.append(field.name) if field.name in attrs: m2m_data[field.name] = attrs.pop(field.name) @@ -973,6 +976,7 @@ class ModelSerializer(Serializer): # saved the model get hidden away on these # private attributes, so we can deal with them # at the point of save. + instance._generic_fk = generic_fk instance._related_data = related_data instance._m2m_data = m2m_data instance._nested_forward_relations = nested_forward_relations @@ -1003,9 +1007,13 @@ class ModelSerializer(Serializer): if getattr(obj, '_m2m_data', None): for accessor_name, object_list in obj._m2m_data.items(): - # We need to save m2m data before linking the objects together - [self.save_object(o) for o in object_list] - setattr(obj, accessor_name, object_list) + if accessor_name in getattr(obj, '_generic_fk', []): + # We are dealing with a reversed generic FK + setattr(obj, accessor_name, object_list) + [self.save_object(o) for o in object_list if not isinstance(o, GenericRelation)] + if accessor_name not in getattr(obj, '_generic_fk', []): + # We need to save m2m data before linking the objects together + setattr(obj, accessor_name, object_list) del(obj._m2m_data) if getattr(obj, '_related_data', None): From 4bdb6787c1fa3e49fcb586491816e8c151329921 Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Tue, 6 May 2014 11:33:13 +0200 Subject: [PATCH 5/7] Added an test on updates through many to many field. --- .../tests/test_serializer_nested.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/rest_framework/tests/test_serializer_nested.py b/rest_framework/tests/test_serializer_nested.py index fcd379bc0..5525ceffe 100644 --- a/rest_framework/tests/test_serializer_nested.py +++ b/rest_framework/tests/test_serializer_nested.py @@ -373,3 +373,46 @@ class NestedModelSerializerUpdateTests(TestCase): deserialize.save() self.assertEqual(models.ManyToManySource.objects.count(), source_count + 1) self.assertEqual(models.ManyToManyTarget.objects.count(), target_count + 2) + + def test_update_with_nested_many_to_many_relation(self): + class ManyToManyTargetSerializer(serializers.ModelSerializer): + class Meta: + model = models.ManyToManyTarget + + class ManyToManySourceSerializer(serializers.ModelSerializer): + targets = ManyToManyTargetSerializer(many=True, allow_add_remove=True) + class Meta: + model = models.ManyToManySource + + source = models.ManyToManySource.objects.create(name='source') + target1 = models.ManyToManyTarget.objects.create(name='target1') + target2 = models.ManyToManyTarget.objects.create(name='target2') + source.targets = [target1, target2] + + data = { + 'id': source.id, + 'name': source.name + '0', + 'targets': [{ + 'id': target1.id, + 'name': target1.name + '1', + }, { + 'id': target2.id, + 'name': target2.name + '2', + }] + } + + source_count = models.ManyToManySource.objects.count() + target_count = models.ManyToManyTarget.objects.count() + + deserialize = ManyToManySourceSerializer(data=data, instance=source) + self.assertTrue(deserialize.is_valid(), deserialize.errors) + deserialize.save() + self.assertEqual(models.ManyToManySource.objects.count(), source_count) + self.assertEqual(models.ManyToManyTarget.objects.count(), target_count) + + # Were the models updated ? + self.assertEqual(source.name, 'source0') + alt_target1 = models.ManyToManyTarget.objects.get(id=target1.id) + self.assertEqual(alt_target1.name, target1.name + '1') + alt_target2 = models.ManyToManyTarget.objects.get(id=target2.id) + self.assertEqual(alt_target2.name, target2.name + '2') From b90ebce00bdc8f3a0232da7e123f5e0d60f38acc Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Tue, 6 May 2014 12:23:26 +0200 Subject: [PATCH 6/7] Failing test case: an item that wasn't in the relation before is created although it already exist. --- rest_framework/tests/test_serializer_nested.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/tests/test_serializer_nested.py b/rest_framework/tests/test_serializer_nested.py index 5525ceffe..65a6d5d46 100644 --- a/rest_framework/tests/test_serializer_nested.py +++ b/rest_framework/tests/test_serializer_nested.py @@ -387,7 +387,7 @@ class NestedModelSerializerUpdateTests(TestCase): source = models.ManyToManySource.objects.create(name='source') target1 = models.ManyToManyTarget.objects.create(name='target1') target2 = models.ManyToManyTarget.objects.create(name='target2') - source.targets = [target1, target2] + source.targets = [target1] data = { 'id': source.id, From 19dcdc5442728451540d764f674f299cc2a80d48 Mon Sep 17 00:00:00 2001 From: Xavier Ordoquy Date: Fri, 16 May 2014 00:06:14 +0200 Subject: [PATCH 7/7] Try to recover an object by its ID before duplicating it. --- rest_framework/serializers.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index def223f83..7659c444a 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -526,6 +526,11 @@ class BaseSerializer(WritableField): # Determine which object we're updating identity = self.get_identity(item) self.object = identity_to_objects.pop(identity, None) + if not self.object and getattr(self.opts, 'model', None): + try: + self.object = self.opts.model.objects.get(id=self.get_identity(item)) + except ObjectDoesNotExist: + pass if self.object is None and not self.allow_add_remove: ret.append(None) errors.append({'non_field_errors': ['Cannot create a new item, only existing items may be updated.']})