Fixed #21113 -- Made LogEntry.change_message language independent

Thanks Tim Graham for the review.

django/contrib/admin/models.py

@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+import json
+
 from django.conf import settings
 from django.contrib.admin.utils import quote
 from django.contrib.contenttypes.models import ContentType
@@ -7,6 +9,7 @@ from django.db import models
 from django.urls import NoReverseMatch, reverse
 from django.utils import timezone
 from django.utils.encoding import python_2_unicode_compatible, smart_text
+from django.utils.text import get_text_list
 from django.utils.translation import ugettext, ugettext_lazy as _
 
 ADDITION = 1
@@ -18,6 +21,8 @@ class LogEntryManager(models.Manager):
     use_in_migrations = True
 
     def log_action(self, user_id, content_type_id, object_id, object_repr, action_flag, change_message=''):
+        if isinstance(change_message, list):
+            change_message = json.dumps(change_message)
         self.model.objects.create(
             user_id=user_id,
             content_type_id=content_type_id,
@@ -50,6 +55,7 @@ class LogEntry(models.Model):
     # Translators: 'repr' means representation (https://docs.python.org/3/library/functions.html#repr)
     object_repr = models.CharField(_('object repr'), max_length=200)
     action_flag = models.PositiveSmallIntegerField(_('action flag'))
+    # change_message is either a string or a JSON structure
     change_message = models.TextField(_('change message'), blank=True)
 
     objects = LogEntryManager()
@@ -69,7 +75,7 @@ class LogEntry(models.Model):
         elif self.is_change():
             return ugettext('Changed "%(object)s" - %(changes)s') % {
                 'object': self.object_repr,
-                'changes': self.change_message,
+                'changes': self.get_change_message(),
             }
         elif self.is_deletion():
             return ugettext('Deleted "%(object)s."') % {'object': self.object_repr}
@@ -85,6 +91,46 @@ class LogEntry(models.Model):
     def is_deletion(self):
         return self.action_flag == DELETION
 
+    def get_change_message(self):
+        """
+        If self.change_message is a JSON structure, interpret it as a change
+        string, properly translated.
+        """
+        if self.change_message and self.change_message[0] == '[':
+            try:
+                change_message = json.loads(self.change_message)
+            except ValueError:
+                return self.change_message
+            messages = []
+            for sub_message in change_message:
+                if 'added' in sub_message:
+                    if sub_message['added']:
+                        sub_message['added']['name'] = ugettext(sub_message['added']['name'])
+                        messages.append(ugettext('Added {name} "{object}".').format(**sub_message['added']))
+                    else:
+                        messages.append(ugettext('Added.'))
+
+                elif 'changed' in sub_message:
+                    sub_message['changed']['fields'] = get_text_list(
+                        sub_message['changed']['fields'], ugettext('and')
+                    )
+                    if 'name' in sub_message['changed']:
+                        sub_message['changed']['name'] = ugettext(sub_message['changed']['name'])
+                        messages.append(ugettext('Changed {fields} for {name} "{object}".').format(
+                            **sub_message['changed']
+                        ))
+                    else:
+                        messages.append(ugettext('Changed {fields}.').format(**sub_message['changed']))
+
+                elif 'deleted' in sub_message:
+                    sub_message['deleted']['name'] = ugettext(sub_message['deleted']['name'])
+                    messages.append(ugettext('Deleted {name} "{object}".').format(**sub_message['deleted']))
+
+            change_message = ' '.join(msg[0].upper() + msg[1:] for msg in messages)
+            return change_message or ugettext('No fields changed.')
+        else:
+            return self.change_message
+
     def get_edited_object(self):
         "Returns the edited object represented by this log entry"
         return self.content_type.get_object_for_this_type(pk=self.object_id)

django/contrib/admin/options.py

@@ -44,7 +44,9 @@ from django.utils.html import escape, format_html
 from django.utils.http import urlencode, urlquote
 from django.utils.safestring import mark_safe
 from django.utils.text import capfirst, get_text_list
-from django.utils.translation import string_concat, ugettext as _, ungettext
+from django.utils.translation import (
+    override as translation_override, string_concat, ugettext as _, ungettext,
+)
 from django.views.decorators.csrf import csrf_protect
 from django.views.generic import RedirectView
 
@@ -924,33 +926,44 @@ class ModelAdmin(BaseModelAdmin):
                 return urlencode({'_changelist_filters': preserved_filters})
         return ''
 
+    @translation_override(None)
     def construct_change_message(self, request, form, formsets, add=False):
         """
-        Construct a change message from a changed object.
+        Construct a JSON structure describing changes from a changed object.
+        Translations are deactivated so that strings are stored untranslated.
+        Translation happens later on LogEntry access.
         """
         change_message = []
         if add:
-            change_message.append(_('Added.'))
+            change_message.append({'added': {}})
         elif form.changed_data:
-            change_message.append(_('Changed %s.') % get_text_list(form.changed_data, _('and')))
+            change_message.append({'changed': {'fields': form.changed_data}})
 
         if formsets:
             for formset in formsets:
                 for added_object in formset.new_objects:
-                    change_message.append(_('Added %(name)s "%(object)s".')
+                    change_message.append({
-                                          % {'name': force_text(added_object._meta.verbose_name),
+                        'added': {
-                                             'object': force_text(added_object)})
+                            'name': force_text(added_object._meta.verbose_name),
+                            'object': force_text(added_object),
+                        }
+                    })
                 for changed_object, changed_fields in formset.changed_objects:
-                    change_message.append(_('Changed %(list)s for %(name)s "%(object)s".')
+                    change_message.append({
-                                          % {'list': get_text_list(changed_fields, _('and')),
+                        'changed': {
                             'name': force_text(changed_object._meta.verbose_name),
-                                             'object': force_text(changed_object)})
+                            'object': force_text(changed_object),
+                            'fields': changed_fields,
+                        }
+                    })
                 for deleted_object in formset.deleted_objects:
-                    change_message.append(_('Deleted %(name)s "%(object)s".')
+                    change_message.append({
-                                          % {'name': force_text(deleted_object._meta.verbose_name),
+                        'deleted': {
-                                             'object': force_text(deleted_object)})
+                            'name': force_text(deleted_object._meta.verbose_name),
-        change_message = ' '.join(change_message)
+                            'object': force_text(deleted_object),
-        return change_message or _('No fields changed.')
+                        }
+                    })
+        return change_message
 
     def message_user(self, request, message, level=messages.INFO, extra_tags='',
                      fail_silently=False):

django/contrib/admin/templates/admin/object_history.html

@@ -29,7 +29,7 @@
         <tr>
             <th scope="row">{{ action.action_time|date:"DATETIME_FORMAT" }}</th>
             <td>{{ action.user.get_username }}{% if action.user.get_full_name %} ({{ action.user.get_full_name }}){% endif %}</td>
-            <td>{{ action.change_message }}</td>
+            <td>{{ action.get_change_message }}</td>
         </tr>
         {% endfor %}
         </tbody>

docs/ref/contrib/admin/index.txt

@@ -2823,7 +2823,18 @@ password box.
 .. attribute:: LogEntry.change_message
 
     The detailed description of the modification. In the case of an edit, for
-    example, the message contains a list of the edited fields.
+    example, the message contains a list of the edited fields. The Django admin
+    site formats this content as a JSON structure, so that
+    :meth:`get_change_message` can recompose a message translated in the current
+    user language. Custom code might set this as a plain string though. You are
+    advised to use the :meth:`get_change_message` method to retrieve this value
+    instead of accessing it directly.
+
+    .. versionchanged:: 1.10
+
+        Previously, this attribute was always a plain string. It is
+        now JSON-structured so that the message can be translated in the current
+        user language. Old messages are untouched.
 
 ``LogEntry`` methods
 --------------------
@@ -2832,6 +2843,14 @@ password box.
 
     A shortcut that returns the referenced object.
 
+.. method:: LogEntry.get_change_message()
+
+    .. versionadded:: 1.10
+
+    Formats and translates :attr:`change_message` into the current user
+    language. Messages created before Django 1.10 will always be displayed in
+    the language in which they were logged.
+
 .. currentmodule:: django.contrib.admin
 
 .. _admin-reverse-urls:

docs/releases/1.10.txt

@@ -51,6 +51,11 @@ Minor features
   model's changelist will now be rendered (without the add button, of course).
   This makes it easier to add custom tools in this case.
 
+* The :class:`~django.contrib.admin.models.LogEntry` model now stores change
+  messages in a JSON structure so that the message can be dynamically translated
+  using the current active language. A new ``LogEntry.get_change_message()``
+  method is now the preferred way of retrieving the change message.
+
 :mod:`django.contrib.admindocs`
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -417,6 +422,10 @@ Miscellaneous
 * :djadmin:`loaddata` now raises a ``CommandError`` instead of showing a
   warning when the specified fixture file is not found.
 
+* Instead of directly accessing the ``LogEntry.change_message`` attribute, it's
+  now better to call the ``LogEntry.get_change_message()`` method which will
+  provide the message in the current language.
+
 .. _deprecated-features-1.10:
 
 Features deprecated in 1.10

tests/admin_utils/admin.py

@@ -1,7 +1,18 @@
 from django.contrib import admin
 
-from .models import Article, ArticleProxy
+from .models import Article, ArticleProxy, Site
+
+
+class ArticleInline(admin.TabularInline):
+    model = Article
+    fields = ['title']
+
+
+class SiteAdmin(admin.ModelAdmin):
+    inlines = [ArticleInline]
+
 
 site = admin.AdminSite(name='admin')
 site.register(Article)
 site.register(ArticleProxy)
+site.register(Site, SiteAdmin)

tests/admin_utils/models.py

@@ -1,6 +1,7 @@
 from django.db import models
 from django.utils import six
 from django.utils.encoding import python_2_unicode_compatible
+from django.utils.translation import ugettext_lazy as _
 
 
 @python_2_unicode_compatible
@@ -17,8 +18,8 @@ class Article(models.Model):
     """
     site = models.ForeignKey(Site, models.CASCADE, related_name="admin_articles")
     title = models.CharField(max_length=100)
-    title2 = models.CharField(max_length=100, verbose_name="another name")
+    hist = models.CharField(max_length=100, verbose_name=_("History"))
-    created = models.DateTimeField()
+    created = models.DateTimeField(null=True)
 
     def test_from_model(self):
         return "nothing"

tests/admin_utils/test_logentry.py

@@ -1,5 +1,7 @@
+# -*- coding: utf-8 -*-
 from __future__ import unicode_literals
 
+import json
 from datetime import datetime
 
 from django.contrib.admin.models import ADDITION, CHANGE, DELETION, LogEntry
@@ -8,7 +10,7 @@ from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.test import TestCase, override_settings
 from django.urls import reverse
-from django.utils import six
+from django.utils import six, translation
 from django.utils.encoding import force_bytes
 from django.utils.html import escape
 
@@ -28,7 +30,7 @@ class LogEntryTests(TestCase):
         self.a1 = Article.objects.create(
             site=self.site,
             title="Title",
-            created=datetime(2008, 3, 18, 11, 54, 58),
+            created=datetime(2008, 3, 18, 11, 54),
         )
         content_type_pk = ContentType.objects.get_for_model(Article).pk
         LogEntry.objects.log_action(
@@ -47,6 +49,86 @@ class LogEntryTests(TestCase):
         logentry.save()
         self.assertEqual(logentry.action_time, action_time)
 
+    def test_logentry_change_message(self):
+        """
+        LogEntry.change_message is stored as a dumped JSON structure to be able
+        to get the message dynamically translated at display time.
+        """
+        post_data = {
+            'site': self.site.pk, 'title': 'Changed', 'hist': 'Some content',
+            'created_0': '2008-03-18', 'created_1': '11:54',
+        }
+        change_url = reverse('admin:admin_utils_article_change', args=[quote(self.a1.pk)])
+        response = self.client.post(change_url, post_data)
+        self.assertRedirects(response, reverse('admin:admin_utils_article_changelist'))
+        logentry = LogEntry.objects.filter(content_type__model__iexact='article').latest('action_time')
+        self.assertEqual(logentry.get_change_message(), 'Changed title and hist.')
+        with translation.override('fr'):
+            self.assertEqual(logentry.get_change_message(), 'Modification de title et hist.')
+
+        add_url = reverse('admin:admin_utils_article_add')
+        post_data['title'] = 'New'
+        response = self.client.post(add_url, post_data)
+        self.assertRedirects(response, reverse('admin:admin_utils_article_changelist'))
+        logentry = LogEntry.objects.filter(content_type__model__iexact='article').latest('action_time')
+        self.assertEqual(logentry.get_change_message(), 'Added.')
+        with translation.override('fr'):
+            self.assertEqual(logentry.get_change_message(), 'Ajout.')
+
+    def test_logentry_change_message_formsets(self):
+        """
+        All messages for changed formsets are logged in a change message.
+        """
+        a2 = Article.objects.create(
+            site=self.site,
+            title="Title second article",
+            created=datetime(2012, 3, 18, 11, 54),
+        )
+        post_data = {
+            'domain': 'example.com',  # domain changed
+            'admin_articles-TOTAL_FORMS': '5',
+            'admin_articles-INITIAL_FORMS': '2',
+            'admin_articles-MIN_NUM_FORMS': '0',
+            'admin_articles-MAX_NUM_FORMS': '1000',
+            # Changed title for 1st article
+            'admin_articles-0-id': str(self.a1.pk),
+            'admin_articles-0-site': str(self.site.pk),
+            'admin_articles-0-title': 'Changed Title',
+            # Second article is deleted
+            'admin_articles-1-id': str(a2.pk),
+            'admin_articles-1-site': str(self.site.pk),
+            'admin_articles-1-title': 'Title second article',
+            'admin_articles-1-DELETE': 'on',
+            # A new article is added
+            'admin_articles-2-site': str(self.site.pk),
+            'admin_articles-2-title': 'Added article',
+        }
+        change_url = reverse('admin:admin_utils_site_change', args=[quote(self.site.pk)])
+        response = self.client.post(change_url, post_data)
+        self.assertRedirects(response, reverse('admin:admin_utils_site_changelist'))
+        self.assertQuerysetEqual(Article.objects.filter(pk=a2.pk), [])
+        logentry = LogEntry.objects.filter(content_type__model__iexact='site').latest('action_time')
+        self.assertEqual(
+            json.loads(logentry.change_message),
+            [
+                {"changed": {"fields": ["domain"]}},
+                {"added": {"object": "Article object", "name": "article"}},
+                {"changed": {"fields": ["title"], "object": "Article object", "name": "article"}},
+                {"deleted": {"object": "Article object", "name": "article"}},
+            ]
+        )
+        self.assertEqual(
+            logentry.get_change_message(),
+            'Changed domain. Added article "Article object". '
+            'Changed title for article "Article object". Deleted article "Article object".'
+        )
+        with translation.override('fr'):
+            self.assertEqual(
+                logentry.get_change_message(),
+                'Modification de domain. Article « Article object » ajouté. '
+                'Modification de title pour l\'objet article « Article object ». Article « Article object » supprimé.'
+            )
+
     def test_logentry_get_edited_object(self):
         """
         LogEntry.get_edited_object() returns the edited object of a LogEntry
@@ -114,7 +196,7 @@ class LogEntryTests(TestCase):
         """
         proxy_content_type = ContentType.objects.get_for_model(ArticleProxy, for_concrete_model=False)
         post_data = {
-            'site': self.site.pk, 'title': "Foo", 'title2': "Bar",
+            'site': self.site.pk, 'title': "Foo", 'hist': "Bar",
             'created_0': '2015-12-25', 'created_1': '00:00',
         }
         changelist_url = reverse('admin:admin_utils_articleproxy_changelist')

tests/admin_utils/tests.py

@@ -203,12 +203,12 @@ class UtilsTests(SimpleTestCase):
             "title"
         )
         self.assertEqual(
-            label_for_field("title2", Article),
+            label_for_field("hist", Article),
-            "another name"
+            "History"
         )
         self.assertEqual(
-            label_for_field("title2", Article, return_attr=True),
+            label_for_field("hist", Article, return_attr=True),
-            ("another name", None)
+            ("History", None)
         )
 
         self.assertEqual(

tests/admin_views/tests.py

@@ -1542,7 +1542,7 @@ class AdminViewPermissionsTest(TestCase):
         self.assertEqual(addition_log.object_id, str(new_article.pk))
         self.assertEqual(addition_log.object_repr, "Døm ikke")
         self.assertEqual(addition_log.action_flag, ADDITION)
-        self.assertEqual(addition_log.change_message, "Added.")
+        self.assertEqual(addition_log.get_change_message(), "Added.")
 
         # Super can add too, but is redirected to the change list view
         self.client.force_login(self.superuser)

tests/auth_tests/test_views.py

@@ -931,7 +931,7 @@ class ChangelistTests(AuthViewsTestCase):
         )
         self.assertRedirects(response, reverse('auth_test_admin:auth_user_changelist'))
         row = LogEntry.objects.latest('id')
-        self.assertEqual(row.change_message, 'Changed email.')
+        self.assertEqual(row.get_change_message(), 'Changed email.')
 
     def test_user_not_change(self):
         response = self.client.post(
@@ -940,7 +940,7 @@ class ChangelistTests(AuthViewsTestCase):
         )
         self.assertRedirects(response, reverse('auth_test_admin:auth_user_changelist'))
         row = LogEntry.objects.latest('id')
-        self.assertEqual(row.change_message, 'No fields changed.')
+        self.assertEqual(row.get_change_message(), 'No fields changed.')
 
     def test_user_change_password(self):
         user_change_url = reverse('auth_test_admin:auth_user_change', args=(self.admin.pk,))
@@ -966,7 +966,7 @@ class ChangelistTests(AuthViewsTestCase):
         )
         self.assertRedirects(response, user_change_url)
         row = LogEntry.objects.latest('id')
-        self.assertEqual(row.change_message, 'Changed password.')
+        self.assertEqual(row.get_change_message(), 'Changed password.')
         self.logout()
         self.login(password='password1')
 
@@ -983,7 +983,7 @@ class ChangelistTests(AuthViewsTestCase):
         row = LogEntry.objects.latest('id')
         self.assertEqual(row.user_id, self.admin.pk)
         self.assertEqual(row.object_id, str(u.pk))
-        self.assertEqual(row.change_message, 'Changed password.')
+        self.assertEqual(row.get_change_message(), 'Changed password.')
 
     def test_password_change_bad_url(self):
         response = self.client.get(reverse('auth_test_admin:auth_user_password_change', args=('foobar',)))
@@ -1018,4 +1018,4 @@ class UUIDUserTests(TestCase):
         row = LogEntry.objects.latest('id')
         self.assertEqual(row.user_id, 1)  # hardcoded in CustomUserAdmin.log_change()
         self.assertEqual(row.object_id, str(u.pk))
-        self.assertEqual(row.change_message, 'Changed password.')
+        self.assertEqual(row.get_change_message(), 'Changed password.')