Fixed #28543 -- Prevented ManyToManyField.value_from_object() from being lazy.

Previously, it was a QuerySet which could reevaluate to a new value if the
model's data changes. This is inconsistent with other Field.value_from_object()
methods.

This allows reverting the fix in the admin for refs #27998.
This commit is contained in:
Tim Graham 2017-08-31 09:34:44 -04:00 committed by GitHub
parent ec6481246a
commit e5bd585c6e
6 changed files with 41 additions and 18 deletions

View File

@ -1468,10 +1468,6 @@ class ModelAdmin(BaseModelAdmin):
new_object = form.instance new_object = form.instance
formsets, inline_instances = self._create_formsets(request, new_object, change=not add) formsets, inline_instances = self._create_formsets(request, new_object, change=not add)
if all_valid(formsets) and form_validated: if all_valid(formsets) and form_validated:
if not add:
# Evalute querysets in form.initial so that changes to
# ManyToManyFields are reflected in this change's LogEntry.
form.has_changed()
self.save_model(request, new_object, form, not add) self.save_model(request, new_object, form, not add)
self.save_related(request, form, formsets, not add) self.save_related(request, form, formsets, not add)
change_message = self.construct_change_message(request, form, formsets, add) change_message = self.construct_change_message(request, form, formsets, add)

View File

@ -1591,9 +1591,7 @@ class ManyToManyField(RelatedField):
pass pass
def value_from_object(self, obj): def value_from_object(self, obj):
if obj.pk is None: return [] if obj.pk is None else list(getattr(obj, self.attname).all())
return self.related_model.objects.none()
return getattr(obj, self.attname).all()
def save_form_data(self, instance, data): def save_form_data(self, instance, data):
getattr(instance, self.attname).set(data) getattr(instance, self.attname).set(data)

View File

@ -35,3 +35,8 @@ Bugfixes
* Fixed a regression in 1.11.4 where ``runserver`` crashed with non-Unicode * Fixed a regression in 1.11.4 where ``runserver`` crashed with non-Unicode
system encodings on Python 2 + Windows (:ticket:`28487`). system encodings on Python 2 + Windows (:ticket:`28487`).
* Fixed a regression in Django 1.10 where changes to a ``ManyToManyField``
weren't logged in the admin change history (:ticket:`27998`) and prevented
``ManyToManyField`` initial data in model forms from being affected by
subsequent model changes (:ticket:`28543`).

View File

@ -360,6 +360,10 @@ class AllFieldsModel(models.Model):
gr = GenericRelation(DataModel) gr = GenericRelation(DataModel)
class ManyToMany(models.Model):
m2m = models.ManyToManyField('self')
############################################################################### ###############################################################################

View File

@ -1,21 +1,13 @@
from django.apps import apps from django.apps import apps
from django.db import models from django.db import models
from django.test import SimpleTestCase from django.test import SimpleTestCase, TestCase
from django.test.utils import isolate_apps from django.test.utils import isolate_apps
from .models import ManyToMany
class ManyToManyFieldTests(SimpleTestCase): class ManyToManyFieldTests(SimpleTestCase):
@isolate_apps('model_fields')
def test_value_from_object_instance_without_pk(self):
class ManyToManyModel(models.Model):
m2m = models.ManyToManyField('self', models.CASCADE)
instance = ManyToManyModel()
qs = instance._meta.get_field('m2m').value_from_object(instance)
self.assertEqual(qs.model, ManyToManyModel)
self.assertEqual(list(qs), [])
def test_abstract_model_pending_operations(self): def test_abstract_model_pending_operations(self):
""" """
Many-to-many fields declared on abstract models should not add lazy Many-to-many fields declared on abstract models should not add lazy
@ -66,3 +58,16 @@ class ManyToManyFieldTests(SimpleTestCase):
assert_app_model_resolved('model_fields') assert_app_model_resolved('model_fields')
assert_app_model_resolved('tests') assert_app_model_resolved('tests')
class ManyToManyFieldDBTests(TestCase):
def test_value_from_object_instance_without_pk(self):
obj = ManyToMany()
self.assertEqual(obj._meta.get_field('m2m').value_from_object(obj), [])
def test_value_from_object_instance_with_pk(self):
obj = ManyToMany.objects.create()
related_obj = ManyToMany.objects.create()
obj.m2m.add(related_obj)
self.assertEqual(obj._meta.get_field('m2m').value_from_object(obj), [related_obj])

View File

@ -3113,3 +3113,18 @@ class StrictAssignmentTests(TestCase):
'__all__': ['Cannot set attribute'], '__all__': ['Cannot set attribute'],
'title': ['This field cannot be blank.'] 'title': ['This field cannot be blank.']
}) })
class ModelToDictTests(TestCase):
def test_many_to_many(self):
"""Data for a ManyToManyField is a list rather than a lazy QuerySet."""
blue = Colour.objects.create(name='blue')
red = Colour.objects.create(name='red')
item = ColourfulItem.objects.create()
item.colours.set([blue])
data = model_to_dict(item)['colours']
self.assertEqual(data, [blue])
item.colours.set([red])
# If data were a QuerySet, it would be reevaluated here and give "red"
# instead of the original value.
self.assertEqual(data, [blue])