Fixed #12952 -- Adjusted admin log change messages to use form labels instead of field names.

django/contrib/admin/models.py

@@ -114,7 +114,7 @@ class LogEntry(models.Model):
 
                 elif 'changed' in sub_message:
                     sub_message['changed']['fields'] = get_text_list(
-                        sub_message['changed']['fields'], gettext('and')
+                        [gettext(field_name) for field_name in sub_message['changed']['fields']], gettext('and')
                     )
                     if 'name' in sub_message['changed']:
                         sub_message['changed']['name'] = gettext(sub_message['changed']['name'])

django/contrib/admin/utils.py

@@ -489,12 +489,21 @@ def construct_change_message(form, formsets, add):
     Translations are deactivated so that strings are stored untranslated.
     Translation happens later on LogEntry access.
     """
+    # Evaluating `form.changed_data` prior to disabling translations is required
+    # to avoid fields affected by localization from being included incorrectly,
+    # e.g. where date formats differ such as MM/DD/YYYY vs DD/MM/YYYY.
+    changed_data = form.changed_data
+    with translation_override(None):
+        # Deactivate translations while fetching verbose_name for form
+        # field labels and using `field_name`, if verbose_name is not provided.
+        # Translations will happen later on LogEntry access.
+        changed_field_labels = _get_changed_field_labels_from_form(form, changed_data)
+
     change_message = []
     if add:
         change_message.append({'added': {}})
     elif form.changed_data:
-        change_message.append({'changed': {'fields': form.changed_data}})
+        change_message.append({'changed': {'fields': changed_field_labels}})
-
     if formsets:
         with translation_override(None):
             for formset in formsets:
@@ -510,7 +519,7 @@ def construct_change_message(form, formsets, add):
                         'changed': {
                             'name': str(changed_object._meta.verbose_name),
                             'object': str(changed_object),
-                            'fields': changed_fields,
+                            'fields': _get_changed_field_labels_from_form(formset.forms[0], changed_fields),
                         }
                     })
                 for deleted_object in formset.deleted_objects:
@@ -521,3 +530,14 @@ def construct_change_message(form, formsets, add):
                         }
                     })
     return change_message
+
+
+def _get_changed_field_labels_from_form(form, changed_data):
+    changed_field_labels = []
+    for field_name in changed_data:
+        try:
+            verbose_field_name = form.fields[field_name].label or field_name
+        except KeyError:
+            verbose_field_name = field_name
+        changed_field_labels.append(str(verbose_field_name))
+    return changed_field_labels

docs/releases/3.0.txt

@@ -322,6 +322,12 @@ backends.
   :class:`~django.db.models.DateTimeField` in ``datetime_cast_date_sql()``,
   ``datetime_extract_sql()``, etc.
 
+:mod:`django.contrib.admin`
+---------------------------
+
+* Admin's model history change messages now prefers more readable field labels
+  instead of field names.
+
 :mod:`django.contrib.gis`
 -------------------------
 

tests/admin_utils/admin.py

@@ -1,11 +1,30 @@
+from django import forms
 from django.contrib import admin
 
 from .models import Article, ArticleProxy, Site
 
 
+class ArticleAdminForm(forms.ModelForm):
+    nolabel_form_field = forms.BooleanField(required=False)
+
+    class Meta:
+        model = Article
+        fields = ['title']
+
+    @property
+    def changed_data(self):
+        data = super().changed_data
+        if data:
+            # Add arbitrary name to changed_data to test
+            # change message construction.
+            return data + ['not_a_form_field']
+        return data
+
+
 class ArticleInline(admin.TabularInline):
     model = Article
     fields = ['title']
+    form = ArticleAdminForm
 
 
 class SiteAdmin(admin.ModelAdmin):

tests/admin_utils/test_logentry.py

@@ -53,9 +53,9 @@ class LogEntryTests(TestCase):
         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('id')
-        self.assertEqual(logentry.get_change_message(), 'Changed title and hist.')
+        self.assertEqual(logentry.get_change_message(), 'Changed Title and History.')
         with translation.override('fr'):
-            self.assertEqual(logentry.get_change_message(), 'Modification de title et hist.')
+            self.assertEqual(logentry.get_change_message(), 'Modification de Title et Historique.')
 
         add_url = reverse('admin:admin_utils_article_add')
         post_data['title'] = 'New'
@@ -85,7 +85,7 @@ class LogEntryTests(TestCase):
             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('id')
-        self.assertEqual(logentry.get_change_message(), 'Changed title and hist.')
+        self.assertEqual(logentry.get_change_message(), 'Changed Title and History.')
 
     def test_logentry_change_message_formsets(self):
         """
@@ -123,23 +123,25 @@ class LogEntryTests(TestCase):
         self.assertEqual(
             json.loads(logentry.change_message),
             [
-                {"changed": {"fields": ["domain"]}},
+                {"changed": {"fields": ["Domain"]}},
                 {"added": {"object": "Added article", "name": "article"}},
-                {"changed": {"fields": ["title"], "object": "Changed Title", "name": "article"}},
+                {"changed": {"fields": ["Title", "not_a_form_field"], "object": "Changed Title", "name": "article"}},
                 {"deleted": {"object": "Title second article", "name": "article"}},
             ]
         )
         self.assertEqual(
             logentry.get_change_message(),
-            'Changed domain. Added article "Added article". '
+            'Changed Domain. Added article "Added article". '
-            'Changed title for article "Changed Title". Deleted article "Title second article".'
+            'Changed Title and not_a_form_field for article "Changed Title". '
+            'Deleted article "Title second article".'
         )
 
         with translation.override('fr'):
             self.assertEqual(
                 logentry.get_change_message(),
-                "Modification de domain. Ajout de article « Added article ». "
+                "Modification de Domain. Ajout de article « Added article ». "
-                "Modification de title pour l'objet article « Changed Title ». "
+                "Modification de Title et not_a_form_field pour l'objet "
+                "article « Changed Title ». "
                 "Suppression de article « Title second article »."
             )
 

tests/admin_views/admin.py

@@ -893,8 +893,27 @@ class CityInlineAdmin(admin.TabularInline):
     view_on_site = False
 
 
+class StateAdminForm(forms.ModelForm):
+    nolabel_form_field = forms.BooleanField(required=False)
+
+    class Meta:
+        model = State
+        fields = '__all__'
+        labels = {"name": "State name (from form's Meta.labels)"}
+
+    @property
+    def changed_data(self):
+        data = super().changed_data
+        if data:
+            # Add arbitrary name to changed_data to test
+            # change message construction.
+            return data + ['not_a_form_field']
+        return data
+
+
 class StateAdmin(admin.ModelAdmin):
     inlines = [CityInlineAdmin]
+    form = StateAdminForm
 
 
 class RestaurantInlineAdmin(admin.TabularInline):

tests/admin_views/models.py

@@ -864,12 +864,12 @@ class EmptyModelMixin(models.Model):
 
 
 class State(models.Model):
-    name = models.CharField(max_length=100)
+    name = models.CharField(max_length=100, verbose_name='State verbose_name')
 
 
 class City(models.Model):
     state = models.ForeignKey(State, models.CASCADE)
-    name = models.CharField(max_length=100)
+    name = models.CharField(max_length=100, verbose_name='City verbose_name')
 
     def get_absolute_url(self):
         return '/dummy/%s/' % self.pk

tests/admin_views/test_history_view.py

@@ -0,0 +1,45 @@
+from django.contrib.admin.models import LogEntry
+from django.contrib.auth.models import User
+from django.test import TestCase, override_settings
+from django.urls import reverse
+
+from .models import City, State
+
+
+@override_settings(ROOT_URLCONF='admin_views.urls')
+class AdminHistoryViewTests(TestCase):
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.superuser = User.objects.create_superuser(
+            username='super', password='secret', email='super@example.com',
+        )
+
+    def setUp(self):
+        self.client.force_login(self.superuser)
+
+    def test_changed_message_uses_form_lables(self):
+        """
+        Admin's model history change messages use form labels instead of
+        field names.
+        """
+        state = State.objects.create(name='My State Name')
+        city = City.objects.create(name='My City Name', state=state)
+        change_dict = {
+            'name': 'My State Name 2',
+            'nolabel_form_field': True,
+            'city_set-0-name': 'My City name 2',
+            'city_set-0-id': city.pk,
+            'city_set-TOTAL_FORMS': '3',
+            'city_set-INITIAL_FORMS': '1',
+            'city_set-MAX_NUM_FORMS': '0',
+        }
+        state_change_url = reverse('admin:admin_views_state_change', args=(state.pk,))
+        self.client.post(state_change_url, change_dict)
+        logentry = LogEntry.objects.filter(content_type__model__iexact='state').latest('id')
+        self.assertEqual(
+            logentry.get_change_message(),
+            'Changed State name (from form\'s Meta.labels), '
+            'nolabel_form_field and not_a_form_field. '
+            'Changed City verbose_name for city "%s".' % city
+        )

tests/admin_views/tests.py

@@ -900,7 +900,7 @@ class AdminViewBasicTest(AdminViewBasicTestCase):
         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.')
+        self.assertEqual(log.get_change_message(), 'Changed Toppings.')
 
     def test_allows_attributeerror_to_bubble_up(self):
         """

tests/auth_tests/test_views.py

@@ -1173,7 +1173,7 @@ class ChangelistTests(AuthViewsTestCase):
         )
         self.assertRedirects(response, reverse('auth_test_admin:auth_user_changelist'))
         row = LogEntry.objects.latest('id')
-        self.assertEqual(row.get_change_message(), 'Changed email.')
+        self.assertEqual(row.get_change_message(), 'Changed Email address.')
 
     def test_user_not_change(self):
         response = self.client.post(