From 6a95451d72c2e1a87e0c5c1d70b2c21e7daa70ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20=C5=98eh=C3=A1=C4=8Dek?= Date: Sat, 22 Jun 2019 10:14:15 +0200 Subject: [PATCH] Fixes #6751 - ModelSerializer fields does not get updated correctly when signals are connected to some fields (#6752) * fixes #6751 * reverted condition * save instance before setting m2m fields * added comment why m2m fields are saved after instance * removed blank line * added test for the issue 6751 --- rest_framework/serializers.py | 12 ++++++++-- tests/test_model_serializer.py | 43 +++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index 857d3ed94..891b250f4 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -973,14 +973,22 @@ class ModelSerializer(Serializer): # Note that unlike `.create()` we don't need to treat many-to-many # relationships as being a special case. During updates we already # have an instance pk for the relationships to be associated with. + m2m_fields = [] for attr, value in validated_data.items(): if attr in info.relations and info.relations[attr].to_many: - field = getattr(instance, attr) - field.set(value) + m2m_fields.append((attr, value)) else: setattr(instance, attr, value) + instance.save() + # Note that many-to-many fields are set after updating instance. + # Setting m2m fields triggers signals which could potentialy change + # updated instance and we do not want it to collide with .update() + for attr, value in m2m_fields: + field = getattr(instance, attr) + field.set(value) + return instance # Determine the fields to apply... diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 88c6785b2..ae320927c 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -18,6 +18,8 @@ from django.core.validators import ( MaxValueValidator, MinLengthValidator, MinValueValidator ) from django.db import models +from django.db.models.signals import m2m_changed +from django.dispatch import receiver from django.test import TestCase from rest_framework import serializers @@ -1251,7 +1253,6 @@ class Issue6110ModelSerializer(serializers.ModelSerializer): class Issue6110Test(TestCase): - def test_model_serializer_custom_manager(self): instance = Issue6110ModelSerializer().create({'name': 'test_name'}) self.assertEqual(instance.name, 'test_name') @@ -1260,3 +1261,43 @@ class Issue6110Test(TestCase): msginitial = ('Got a `TypeError` when calling `Issue6110TestModel.all_objects.create()`.') with self.assertRaisesMessage(TypeError, msginitial): Issue6110ModelSerializer().create({'wrong_param': 'wrong_param'}) + + +class Issue6751Model(models.Model): + many_to_many = models.ManyToManyField(ManyToManyTargetModel, related_name='+') + char_field = models.CharField(max_length=100) + char_field2 = models.CharField(max_length=100) + + +@receiver(m2m_changed, sender=Issue6751Model.many_to_many.through) +def process_issue6751model_m2m_changed(action, instance, **_): + if action == 'post_add': + instance.char_field = 'value changed by signal' + instance.save() + + +class Issue6751Test(TestCase): + def test_model_serializer_save_m2m_after_instance(self): + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = Issue6751Model + fields = ( + 'many_to_many', + 'char_field', + ) + + instance = Issue6751Model.objects.create(char_field='initial value') + m2m_target = ManyToManyTargetModel.objects.create(name='target') + + serializer = TestSerializer( + instance=instance, + data={ + 'many_to_many': (m2m_target.id,), + 'char_field': 'will be changed by signal', + } + ) + + serializer.is_valid() + serializer.save() + + self.assertEqual(instance.char_field, 'value changed by signal')