[1.11.x] Fixed #27998, #28543 -- Restored logging of ManyToManyField changes in admin's object history.

And prevented ManyToManyField initial data in model forms from being affected
by subsequent model changes.

Regression in 56a55566a791a11420fe96f745b7489e756fc931.

Partial backport of e5bd585c6e and
15b465c584 from master

AUTHORS

@@ -459,6 +459,7 @@ answer newbie questions, and generally made Django that much better:
     Lex Berezhny <lex@damoti.com>
     Liang Feng <hutuworm@gmail.com>
     limodou
+    Lincoln Smith <lincoln.smith@anu.edu.au>
     Loek van Gent <loek@barakken.nl>
     Loïc Bistuer <loic.bistuer@sixmedia.com>
     Lowe Thiderman <lowe.thiderman@gmail.com>

django/forms/models.py

@@ -84,6 +84,7 @@ def model_to_dict(instance, fields=None, exclude=None):
     fields will be excluded from the returned dict, even if they are listed in
     the ``fields`` argument.
     """
+    from django.db import models
     opts = instance._meta
     data = {}
     for f in chain(opts.concrete_fields, opts.private_fields, opts.many_to_many):
@@ -94,6 +95,10 @@ def model_to_dict(instance, fields=None, exclude=None):
         if exclude and f.name in exclude:
             continue
         data[f.name] = f.value_from_object(instance)
+        # Evaluate ManyToManyField QuerySets to prevent subsequent model
+        # alteration of that field from being reflected in the data.
+        if isinstance(f, models.ManyToManyField):
+            data[f.name] = list(data[f.name])
     return data
 
 

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`).

tests/admin_views/admin.py

@@ -38,14 +38,14 @@ from .models import (
     OtherStory, Paper, Parent, ParentWithDependentChildren, ParentWithUUIDPK,
     Person, Persona, Picture, Pizza, Plot, PlotDetails, PlotProxy,
     PluggableSearchPerson, Podcast, Post, PrePopulatedPost,
-    PrePopulatedPostLargeSlug, PrePopulatedSubPost, Promo, Question, Recipe,
-    Recommendation, Recommender, ReferencedByGenRel, ReferencedByInline,
-    ReferencedByParent, RelatedPrepopulated, RelatedWithUUIDPKModel, Report,
-    Reservation, Restaurant, RowLevelChangePermissionModel, Section,
-    ShortMessage, Simple, Sketch, State, Story, StumpJoke, Subscriber,
-    SuperVillain, Telegram, Thing, Topping, UnchangeableObject,
-    UndeletableObject, UnorderedObject, UserMessenger, Villain, Vodcast,
-    Whatsit, Widget, Worker, WorkHour,
+    PrePopulatedPostLargeSlug, PrePopulatedSubPost, Promo, Question,
+    ReadablePizza, Recipe, Recommendation, Recommender, ReferencedByGenRel,
+    ReferencedByInline, ReferencedByParent, RelatedPrepopulated,
+    RelatedWithUUIDPKModel, Report, Reservation, Restaurant,
+    RowLevelChangePermissionModel, Section, ShortMessage, Simple, Sketch,
+    State, Story, StumpJoke, Subscriber, SuperVillain, Telegram, Thing,
+    Topping, UnchangeableObject, UndeletableObject, UnorderedObject,
+    UserMessenger, Villain, Vodcast, Whatsit, Widget, Worker, WorkHour,
 )
 
 
@@ -988,6 +988,7 @@ site.register(Book, inlines=[ChapterInline])
 site.register(Promo)
 site.register(ChapterXtra1, ChapterXtra1Admin)
 site.register(Pizza, PizzaAdmin)
+site.register(ReadablePizza)
 site.register(Topping, ToppingAdmin)
 site.register(Album, AlbumAdmin)
 site.register(Question)

tests/admin_views/models.py

@@ -611,6 +611,13 @@ class Pizza(models.Model):
     toppings = models.ManyToManyField('Topping', related_name='pizzas')
 
 
+# Pizza's ModelAdmin has readonly_fields = ['toppings'].
+# toppings is editable for this model's admin.
+class ReadablePizza(Pizza):
+    class Meta:
+        proxy = True
+
+
 class Album(models.Model):
     owner = models.ForeignKey(User, models.SET_NULL, null=True, blank=True)
     title = models.CharField(max_length=30)

tests/admin_views/tests.py

@@ -57,12 +57,13 @@ from .models import (
     MainPrepopulated, Media, ModelWithStringPrimaryKey, OtherStory, Paper,
     Parent, ParentWithDependentChildren, ParentWithUUIDPK, Person, Persona,
     Picture, Pizza, Plot, PlotDetails, PluggableSearchPerson, Podcast, Post,
-    PrePopulatedPost, Promo, Question, Recommendation, Recommender,
-    RelatedPrepopulated, RelatedWithUUIDPKModel, Report, Restaurant,
-    RowLevelChangePermissionModel, SecretHideout, Section, ShortMessage,
-    Simple, State, Story, Subscriber, SuperSecretHideout, SuperVillain,
-    Telegram, TitleTranslation, Topping, UnchangeableObject, UndeletableObject,
-    UnorderedObject, Villain, Vodcast, Whatsit, Widget, Worker, WorkHour,
+    PrePopulatedPost, Promo, Question, ReadablePizza, Recommendation,
+    Recommender, RelatedPrepopulated, RelatedWithUUIDPKModel, Report,
+    Restaurant, RowLevelChangePermissionModel, SecretHideout, Section,
+    ShortMessage, Simple, State, Story, Subscriber, SuperSecretHideout,
+    SuperVillain, Telegram, TitleTranslation, Topping, UnchangeableObject,
+    UndeletableObject, UnorderedObject, Villain, Vodcast, Whatsit, Widget,
+    Worker, WorkHour,
 )
 
 
@@ -879,6 +880,17 @@ class AdminViewBasicTest(AdminViewBasicTestCase):
         response = self.client.get(reverse('admin:admin_views_undeletableobject_change', args=(instance.pk,)))
         self.assertNotContains(response, 'deletelink')
 
+    def test_change_view_logs_m2m_field_changes(self):
+        """Changes to ManyToManyFields are included in the object's history."""
+        pizza = ReadablePizza.objects.create(name='Cheese')
+        cheese = Topping.objects.create(name='cheese')
+        post_data = {'name': pizza.name, 'toppings': [cheese.pk]}
+        response = self.client.post(reverse('admin:admin_views_readablepizza_change', args=(pizza.pk,)), post_data)
+        self.assertRedirects(response, reverse('admin:admin_views_readablepizza_changelist'))
+        pizza_ctype = ContentType.objects.get_for_model(ReadablePizza, for_concrete_model=False)
+        log = LogEntry.objects.filter(content_type=pizza_ctype, object_id=pizza.pk).first()
+        self.assertEqual(log.get_change_message(), 'Changed toppings.')
+
     def test_allows_attributeerror_to_bubble_up(self):
         """
         AttributeErrors are allowed to bubble when raised inside a change list

tests/model_forms/tests.py

@@ -3142,3 +3142,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])