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.

6 files changed, 41 insertions, 18 deletions

diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index a88c867d85..808f4709ac 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -1468,10 +1468,6 @@ class ModelAdmin(BaseModelAdmin):
                 new_object = form.instance
             formsets, inline_instances = self._create_formsets(request, new_object, change=not add)
             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_related(request, form, formsets, not add)
                 change_message = self.construct_change_message(request, form, formsets, add)
diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index f6b87b7de8..0e0910277f 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -1591,9 +1591,7 @@ class ManyToManyField(RelatedField):
         pass
 
     def value_from_object(self, obj):
-        if obj.pk is None:
-            return self.related_model.objects.none()
-        return getattr(obj, self.attname).all()
+        return [] if obj.pk is None else list(getattr(obj, self.attname).all())
 
     def save_form_data(self, instance, data):
         getattr(instance, self.attname).set(data)
diff --git a/docs/releases/1.11.5.txt b/docs/releases/1.11.5.txt
index 91620eb740..cb9e466248 100644
--- a/docs/releases/1.11.5.txt
+++ b/docs/releases/1.11.5.txt
@@ -35,3 +35,8 @@ Bugfixes
 
 * Fixed a regression in 1.11.4 where ``runserver`` crashed with non-Unicode
   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`).
diff --git a/tests/model_fields/models.py b/tests/model_fields/models.py
index 2208a8d4b8..1d18e78869 100644
--- a/tests/model_fields/models.py
+++ b/tests/model_fields/models.py
@@ -360,6 +360,10 @@ class AllFieldsModel(models.Model):
     gr = GenericRelation(DataModel)
 
 
+class ManyToMany(models.Model):
+    m2m = models.ManyToManyField('self')
+
+
 ###############################################################################
 
 
diff --git a/tests/model_fields/test_manytomanyfield.py b/tests/model_fields/test_manytomanyfield.py
index b270048bd8..5724fe9384 100644
--- a/tests/model_fields/test_manytomanyfield.py
+++ b/tests/model_fields/test_manytomanyfield.py
@@ -1,20 +1,12 @@
 from django.apps import apps
 from django.db import models
-from django.test import SimpleTestCase
+from django.test import SimpleTestCase, TestCase
 from django.test.utils import isolate_apps
 
+from .models import ManyToMany
 
-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), [])
+class ManyToManyFieldTests(SimpleTestCase):
 
     def test_abstract_model_pending_operations(self):
         """
@@ -66,3 +58,16 @@ class ManyToManyFieldTests(SimpleTestCase):
 
         assert_app_model_resolved('model_fields')
         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])
diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py
index 0cfc18659f..7af2aff825 100644
--- a/tests/model_forms/tests.py
+++ b/tests/model_forms/tests.py
@@ -3113,3 +3113,18 @@ class StrictAssignmentTests(TestCase):
             '__all__': ['Cannot set attribute'],
             '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])