From 20c03399d8fd03484f3ed33d93691c29c2ff5aaf Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Wed, 30 Aug 2017 10:06:10 -0400 Subject: [PATCH] [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 e5bd585c6eb1e13e2f8aac030b33c077b0b70c05 and 15b465c584f49a1d43b6c18796f83521ee4ffc22 from master --- AUTHORS | 1 + django/forms/models.py | 5 +++++ docs/releases/1.11.5.txt | 5 +++++ tests/admin_views/admin.py | 17 +++++++++-------- tests/admin_views/models.py | 7 +++++++ tests/admin_views/tests.py | 24 ++++++++++++++++++------ tests/model_forms/tests.py | 15 +++++++++++++++ 7 files changed, 60 insertions(+), 14 deletions(-) diff --git a/AUTHORS b/AUTHORS index cf4a559427..d1280eb6c9 100644 --- a/AUTHORS +++ b/AUTHORS @@ -459,6 +459,7 @@ answer newbie questions, and generally made Django that much better: Lex Berezhny Liang Feng limodou + Lincoln Smith Loek van Gent Loïc Bistuer Lowe Thiderman diff --git a/django/forms/models.py b/django/forms/models.py index db710a2ed2..0e80e19042 100644 --- a/django/forms/models.py +++ b/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 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/admin_views/admin.py b/tests/admin_views/admin.py index b5e0ff4814..5d02f0f37d 100644 --- a/tests/admin_views/admin.py +++ b/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) diff --git a/tests/admin_views/models.py b/tests/admin_views/models.py index 9c7eee7547..29d96474c6 100644 --- a/tests/admin_views/models.py +++ b/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) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index febb20914c..efddc49de7 100644 --- a/tests/admin_views/tests.py +++ b/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 diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index f3317f59de..b14e0e2389 100644 --- a/tests/model_forms/tests.py +++ b/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])