Fixed #34462 -- Made admin log actions in bulk.

This also deprecates ModelAdmin.log_deletion() and
LogEntryManager.log_action().

django/contrib/admin/actions.py

@@ -45,9 +45,7 @@ def delete_selected(modeladmin, request, queryset):
             raise PermissionDenied
         n = len(queryset)
         if n:
-            for obj in queryset:
-                obj_display = str(obj)
-                modeladmin.log_deletion(request, obj, obj_display)
+            modeladmin.log_deletions(request, queryset)
             modeladmin.delete_queryset(request, queryset)
             modeladmin.message_user(
                 request,

django/contrib/admin/models.py

@@ -1,4 +1,5 @@
 import json
+import warnings
 
 from django.conf import settings
 from django.contrib.admin.utils import quote
@@ -6,6 +7,7 @@ from django.contrib.contenttypes.models import ContentType
 from django.db import models
 from django.urls import NoReverseMatch, reverse
 from django.utils import timezone
+from django.utils.deprecation import RemovedInDjango60Warning
 from django.utils.text import get_text_list
 from django.utils.translation import gettext
 from django.utils.translation import gettext_lazy as _
@@ -33,6 +35,11 @@ class LogEntryManager(models.Manager):
         action_flag,
         change_message="",
     ):
+        warnings.warn(
+            "LogEntryManager.log_action() is deprecated. Use log_actions() instead.",
+            RemovedInDjango60Warning,
+            stacklevel=2,
+        )
         if isinstance(change_message, list):
             change_message = json.dumps(change_message)
         return self.model.objects.create(
@@ -44,6 +51,55 @@ class LogEntryManager(models.Manager):
             change_message=change_message,
         )
 
+    def log_actions(
+        self, user_id, queryset, action_flag, change_message="", *, single_object=False
+    ):
+        # RemovedInDjango60Warning.
+        if type(self).log_action != LogEntryManager.log_action:
+            warnings.warn(
+                "The usage of log_action() is deprecated. Implement log_actions() "
+                "instead.",
+                RemovedInDjango60Warning,
+                stacklevel=2,
+            )
+            return [
+                self.log_action(
+                    user_id=user_id,
+                    content_type_id=ContentType.objects.get_for_model(
+                        obj, for_concrete_model=False
+                    ).id,
+                    object_id=obj.pk,
+                    object_repr=str(obj),
+                    action_flag=action_flag,
+                    change_message=change_message,
+                )
+                for obj in queryset
+            ]
+
+        if isinstance(change_message, list):
+            change_message = json.dumps(change_message)
+
+        log_entry_list = [
+            self.model(
+                user_id=user_id,
+                content_type_id=ContentType.objects.get_for_model(
+                    obj, for_concrete_model=False
+                ).id,
+                object_id=obj.pk,
+                object_repr=str(obj)[:200],
+                action_flag=action_flag,
+                change_message=change_message,
+            )
+            for obj in queryset
+        ]
+
+        if single_object and log_entry_list:
+            instance = log_entry_list[0]
+            instance.save()
+            return instance
+
+        return self.model.objects.bulk_create(log_entry_list)
+
 
 class LogEntry(models.Model):
     action_time = models.DateTimeField(

django/contrib/admin/options.py

@@ -2,6 +2,7 @@ import copy
 import enum
 import json
 import re
+import warnings
 from functools import partial, update_wrapper
 from urllib.parse import parse_qsl
 from urllib.parse import quote as urlquote
@@ -54,6 +55,7 @@ from django.http.response import HttpResponseBase
 from django.template.response import SimpleTemplateResponse, TemplateResponse
 from django.urls import reverse
 from django.utils.decorators import method_decorator
+from django.utils.deprecation import RemovedInDjango60Warning
 from django.utils.html import format_html
 from django.utils.http import urlencode
 from django.utils.safestring import mark_safe
@@ -945,13 +947,12 @@ class ModelAdmin(BaseModelAdmin):
         """
         from django.contrib.admin.models import ADDITION, LogEntry
 
-        return LogEntry.objects.log_action(
+        return LogEntry.objects.log_actions(
             user_id=request.user.pk,
-            content_type_id=get_content_type_for_model(obj).pk,
-            object_id=obj.pk,
-            object_repr=str(obj),
+            queryset=[obj],
             action_flag=ADDITION,
             change_message=message,
+            single_object=True,
         )
 
     def log_change(self, request, obj, message):
@@ -962,13 +963,12 @@ class ModelAdmin(BaseModelAdmin):
         """
         from django.contrib.admin.models import CHANGE, LogEntry
 
-        return LogEntry.objects.log_action(
+        return LogEntry.objects.log_actions(
             user_id=request.user.pk,
-            content_type_id=get_content_type_for_model(obj).pk,
-            object_id=obj.pk,
-            object_repr=str(obj),
+            queryset=[obj],
             action_flag=CHANGE,
             change_message=message,
+            single_object=True,
         )
 
     def log_deletion(self, request, obj, object_repr):
@@ -978,6 +978,11 @@ class ModelAdmin(BaseModelAdmin):
 
         The default implementation creates an admin LogEntry object.
         """
+        warnings.warn(
+            "ModelAdmin.log_deletion() is deprecated. Use log_deletions() instead.",
+            RemovedInDjango60Warning,
+            stacklevel=2,
+        )
         from django.contrib.admin.models import DELETION, LogEntry
 
         return LogEntry.objects.log_action(
@@ -988,6 +993,31 @@ class ModelAdmin(BaseModelAdmin):
             action_flag=DELETION,
         )
 
+    def log_deletions(self, request, queryset):
+        """
+        Log that objects will be deleted. Note that this method must be called
+        before the deletion.
+
+        The default implementation creates admin LogEntry objects.
+        """
+        from django.contrib.admin.models import DELETION, LogEntry
+
+        # RemovedInDjango60Warning.
+        if type(self).log_deletion != ModelAdmin.log_deletion:
+            warnings.warn(
+                "The usage of log_deletion() is deprecated. Implement log_deletions() "
+                "instead.",
+                RemovedInDjango60Warning,
+                stacklevel=2,
+            )
+            return [self.log_deletion(request, obj, str(obj)) for obj in queryset]
+
+        return LogEntry.objects.log_actions(
+            user_id=request.user.pk,
+            queryset=queryset,
+            action_flag=DELETION,
+        )
+
     def action_checkbox(self, obj):
         """
         A list_display column containing a checkbox widget.
@@ -2174,7 +2204,7 @@ class ModelAdmin(BaseModelAdmin):
             obj_display = str(obj)
             attr = str(to_field) if to_field else self.opts.pk.attname
             obj_id = obj.serializable_value(attr)
-            self.log_deletion(request, obj, obj_display)
+            self.log_deletions(request, [obj])
             self.delete_model(request, obj)
 
             return self.response_delete(request, obj_display, obj_id)

docs/internals/deprecation.txt

@@ -56,7 +56,8 @@ details on these changes.
 See the :ref:`Django 5.1 release notes <deprecated-features-5.1>` for more
 details on these changes.
 
-* ...
+* The ``ModelAdmin.log_deletion()`` and ``LogEntryManager.log_action()``
+  methods will be removed.
 
 .. _deprecation-removed-in-5.1:
 

docs/releases/5.1.txt

@@ -285,7 +285,10 @@ Features deprecated in 5.1
 Miscellaneous
 -------------
 
-* ...
+* The ``ModelAdmin.log_deletion()`` and ``LogEntryManager.log_action()``
+  methods are deprecated. Subclasses should implement
+  ``ModelAdmin.log_deletions()`` and  ``LogEntryManager.log_actions()``
+  instead.
 
 Features removed in 5.1
 =======================

tests/admin_changelist/tests.py

@@ -16,7 +16,6 @@ from django.contrib.admin.views.main import (
     TO_FIELD_VAR,
 )
 from django.contrib.auth.models import User
-from django.contrib.contenttypes.models import ContentType
 from django.contrib.messages.storage.cookie import CookieStorage
 from django.db import DatabaseError, connection, models
 from django.db.models import F, Field, IntegerField
@@ -1636,8 +1635,7 @@ class GetAdminLogTests(TestCase):
         """{% get_admin_log %} works without specifying a user."""
         user = User(username="jondoe", password="secret", email="super@example.com")
         user.save()
-        ct = ContentType.objects.get_for_model(User)
-        LogEntry.objects.log_action(user.pk, ct.pk, user.pk, repr(user), 1)
+        LogEntry.objects.log_actions(user.pk, [user], 1, single_object=True)
         context = Context({"log_entries": LogEntry.objects.all()})
         t = Template(
             "{% load log %}"
@@ -1646,7 +1644,7 @@ class GetAdminLogTests(TestCase):
             "{{ entry|safe }}"
             "{% endfor %}"
         )
-        self.assertEqual(t.render(context), "Added “<User: jondoe>”.")
+        self.assertEqual(t.render(context), "Added “jondoe”.")
 
     def test_missing_args(self):
         msg = "'get_admin_log' statements require two arguments"

tests/admin_utils/models.py

@@ -1,4 +1,5 @@
 from django.contrib import admin
+from django.contrib.admin.models import LogEntry, LogEntryManager
 from django.db import models
 from django.utils.translation import gettext_lazy as _
 
@@ -86,3 +87,26 @@ class VehicleMixin(Vehicle):
 
 class Car(VehicleMixin):
     pass
+
+
+class InheritedLogEntryManager(LogEntryManager):
+    model = LogEntry
+
+    def log_action(
+        self,
+        user_id,
+        content_type_id,
+        object_id,
+        object_repr,
+        action_flag,
+        change_message="",
+    ):
+        return LogEntry.objects.create(
+            user_id=user_id,
+            content_type_id=content_type_id,
+            object_id=str(object_id),
+            # Changing actual repr to test repr
+            object_repr="Test Repr",
+            action_flag=action_flag,
+            change_message=change_message,
+        )

tests/admin_utils/test_logentry.py

@@ -8,9 +8,10 @@ from django.contrib.contenttypes.models import ContentType
 from django.test import TestCase, override_settings
 from django.urls import reverse
 from django.utils import translation
+from django.utils.deprecation import RemovedInDjango60Warning
 from django.utils.html import escape
 
-from .models import Article, ArticleProxy, Car, Site
+from .models import Article, ArticleProxy, Car, InheritedLogEntryManager, Site
 
 
 @override_settings(ROOT_URLCONF="admin_utils.urls")
@@ -26,14 +27,22 @@ class LogEntryTests(TestCase):
             title="Title",
             created=datetime(2008, 3, 12, 11, 54),
         )
-        content_type_pk = ContentType.objects.get_for_model(Article).pk
-        LogEntry.objects.log_action(
+        cls.a2 = Article.objects.create(
+            site=cls.site,
+            title="Title 2",
+            created=datetime(2009, 3, 12, 11, 54),
+        )
+        cls.a3 = Article.objects.create(
+            site=cls.site,
+            title="Title 3",
+            created=datetime(2010, 3, 12, 11, 54),
+        )
+        LogEntry.objects.log_actions(
             cls.user.pk,
-            content_type_pk,
-            cls.a1.pk,
-            repr(cls.a1),
+            [cls.a1],
             CHANGE,
             change_message="Changed something",
+            single_object=True,
         )
 
     def setUp(self):
@@ -227,18 +236,95 @@ class LogEntryTests(TestCase):
         logentry = LogEntry.objects.first()
         self.assertEqual(repr(logentry), str(logentry.action_time))
 
+    # RemovedInDjango60Warning.
     def test_log_action(self):
-        content_type_pk = ContentType.objects.get_for_model(Article).pk
-        log_entry = LogEntry.objects.log_action(
-            self.user.pk,
-            content_type_pk,
-            self.a1.pk,
-            repr(self.a1),
-            CHANGE,
-            change_message="Changed something else",
-        )
+        msg = "LogEntryManager.log_action() is deprecated. Use log_actions() instead."
+        content_type_val = ContentType.objects.get_for_model(Article).pk
+        with self.assertWarnsMessage(RemovedInDjango60Warning, msg):
+            log_entry = LogEntry.objects.log_action(
+                self.user.pk,
+                content_type_val,
+                self.a1.pk,
+                repr(self.a1),
+                CHANGE,
+                change_message="Changed something else",
+            )
         self.assertEqual(log_entry, LogEntry.objects.latest("id"))
 
+    def test_log_actions(self):
+        queryset = Article.objects.all().order_by("-id")
+        msg = "Deleted Something"
+        content_type = ContentType.objects.get_for_model(self.a1)
+        self.assertEqual(len(queryset), 3)
+        with self.assertNumQueries(1):
+            LogEntry.objects.log_actions(
+                self.user.pk,
+                queryset,
+                DELETION,
+                change_message=msg,
+            )
+        logs = (
+            LogEntry.objects.filter(action_flag=DELETION)
+            .order_by("id")
+            .values_list(
+                "user",
+                "content_type",
+                "object_id",
+                "object_repr",
+                "action_flag",
+                "change_message",
+            )
+        )
+        expected_log_values = [
+            (
+                self.user.pk,
+                content_type.id,
+                str(obj.pk),
+                str(obj),
+                DELETION,
+                msg,
+            )
+            for obj in queryset
+        ]
+        self.assertSequenceEqual(logs, expected_log_values)
+
+    # RemovedInDjango60Warning.
+    def test_log_action_fallback(self):
+        LogEntry.objects2 = InheritedLogEntryManager()
+        queryset = Article.objects.all().order_by("-id")
+        content_type = ContentType.objects.get_for_model(self.a1)
+        self.assertEqual(len(queryset), 3)
+        msg = (
+            "The usage of log_action() is deprecated. Implement log_actions() instead."
+        )
+        with self.assertNumQueries(3):
+            with self.assertWarnsMessage(RemovedInDjango60Warning, msg):
+                LogEntry.objects2.log_actions(self.user.pk, queryset, DELETION)
+        log_values = (
+            LogEntry.objects.filter(action_flag=DELETION)
+            .order_by("id")
+            .values_list(
+                "user",
+                "content_type",
+                "object_id",
+                "object_repr",
+                "action_flag",
+                "change_message",
+            )
+        )
+        expected_log_values = [
+            (
+                self.user.pk,
+                content_type.id,
+                str(obj.pk),
+                "Test Repr",
+                DELETION,
+                "",
+            )
+            for obj in queryset
+        ]
+        self.assertSequenceEqual(log_values, expected_log_values)
+
     def test_recentactions_without_content_type(self):
         """
         If a LogEntry is missing content_type it will not display it in span
@@ -248,7 +334,7 @@ class LogEntryTests(TestCase):
         link = reverse("admin:admin_utils_article_change", args=(quote(self.a1.pk),))
         should_contain = """<a href="%s">%s</a>""" % (
             escape(link),
-            escape(repr(self.a1)),
+            escape(str(self.a1)),
         )
         self.assertContains(response, should_contain)
         should_contain = "Article"
@@ -320,28 +406,29 @@ class LogEntryTests(TestCase):
                 self.assertEqual(log.get_action_flag_display(), display_name)
 
     def test_hook_get_log_entries(self):
-        LogEntry.objects.log_action(
+        LogEntry.objects.log_actions(
             self.user.pk,
-            ContentType.objects.get_for_model(Article).pk,
-            self.a1.pk,
-            "Article changed",
+            [self.a1],
             CHANGE,
             change_message="Article changed message",
+            single_object=True,
         )
         c1 = Car.objects.create()
-        LogEntry.objects.log_action(
+        LogEntry.objects.log_actions(
             self.user.pk,
-            ContentType.objects.get_for_model(Car).pk,
-            c1.pk,
-            "Car created",
+            [c1],
             ADDITION,
             change_message="Car created message",
+            single_object=True,
         )
+        exp_str_article = escape(str(self.a1))
+        exp_str_car = escape(str(c1))
+
         response = self.client.get(reverse("admin:index"))
-        self.assertContains(response, "Article changed")
-        self.assertContains(response, "Car created")
+        self.assertContains(response, exp_str_article)
+        self.assertContains(response, exp_str_car)
 
         # site "custom_admin" only renders log entries of registered models
         response = self.client.get(reverse("custom_admin:index"))
-        self.assertContains(response, "Article changed")
-        self.assertNotContains(response, "Car created")
+        self.assertContains(response, exp_str_article)
+        self.assertNotContains(response, exp_str_car)

tests/admin_views/test_actions.py

@@ -4,9 +4,11 @@ from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME
 from django.contrib.admin.views.main import IS_POPUP_VAR
 from django.contrib.auth.models import Permission, User
 from django.core import mail
+from django.db import connection
 from django.template.loader import render_to_string
 from django.template.response import TemplateResponse
 from django.test import TestCase, override_settings
+from django.test.utils import CaptureQueriesContext
 from django.urls import reverse
 
 from .admin import SubscriberAdmin
@@ -74,8 +76,21 @@ class AdminActionsTest(TestCase):
         self.assertContains(confirmation, "<li>Subscribers: 2</li>")
         self.assertContains(confirmation, "<li>External subscribers: 1</li>")
         self.assertContains(confirmation, ACTION_CHECKBOX_NAME, count=2)
-        self.client.post(
-            reverse("admin:admin_views_subscriber_changelist"), delete_confirmation_data
+        with CaptureQueriesContext(connection) as ctx:
+            self.client.post(
+                reverse("admin:admin_views_subscriber_changelist"),
+                delete_confirmation_data,
+            )
+        # Log entries are inserted in bulk.
+        self.assertEqual(
+            len(
+                [
+                    q["sql"]
+                    for q in ctx.captured_queries
+                    if q["sql"].startswith("INSERT")
+                ]
+            ),
+            1,
         )
         self.assertEqual(Subscriber.objects.count(), 0)
 

tests/admin_views/test_history_view.py

@@ -1,7 +1,6 @@
 from django.contrib.admin.models import CHANGE, LogEntry
 from django.contrib.admin.tests import AdminSeleniumTestCase
 from django.contrib.auth.models import User
-from django.contrib.contenttypes.models import ContentType
 from django.core.paginator import Paginator
 from django.test import TestCase, override_settings
 from django.urls import reverse
@@ -61,15 +60,13 @@ class SeleniumTests(AdminSeleniumTestCase):
             password="secret",
             email="super@example.com",
         )
-        content_type_pk = ContentType.objects.get_for_model(User).pk
         for i in range(1, 1101):
-            LogEntry.objects.log_action(
+            LogEntry.objects.log_actions(
                 self.superuser.pk,
-                content_type_pk,
-                self.superuser.pk,
-                repr(self.superuser),
+                [self.superuser],
                 CHANGE,
                 change_message=f"Changed something {i}",
+                single_object=True,
             )
         self.admin_login(
             username="super",

tests/admin_views/tests.py

@@ -3808,33 +3808,27 @@ class AdminViewStringPrimaryKeyTest(TestCase):
             r"""-_.!~*'() ;/?:@&=+$, <>#%" {}|\^[]`"""
         )
         cls.m1 = ModelWithStringPrimaryKey.objects.create(string_pk=cls.pk)
-        content_type_pk = ContentType.objects.get_for_model(
-            ModelWithStringPrimaryKey
-        ).pk
         user_pk = cls.superuser.pk
-        LogEntry.objects.log_action(
+        LogEntry.objects.log_actions(
             user_pk,
-            content_type_pk,
-            cls.pk,
-            cls.pk,
+            [cls.m1],
             2,
             change_message="Changed something",
+            single_object=True,
         )
-        LogEntry.objects.log_action(
+        LogEntry.objects.log_actions(
             user_pk,
-            content_type_pk,
-            cls.pk,
-            cls.pk,
+            [cls.m1],
             1,
             change_message="Added something",
+            single_object=True,
         )
-        LogEntry.objects.log_action(
+        LogEntry.objects.log_actions(
             user_pk,
-            content_type_pk,
-            cls.pk,
-            cls.pk,
+            [cls.m1],
             3,
             change_message="Deleted something",
+            single_object=True,
         )
 
     def setUp(self):

tests/modeladmin/tests.py

@@ -833,12 +833,59 @@ class ModelAdminTests(TestCase):
                 self.assertEqual(fetched.change_message, str(message))
                 self.assertEqual(fetched.object_repr, str(self.band))
 
+    def test_log_deletions(self):
+        ma = ModelAdmin(Band, self.site)
+        mock_request = MockRequest()
+        mock_request.user = User.objects.create(username="akash")
+        content_type = get_content_type_for_model(self.band)
+        Band.objects.create(
+            name="The Beatles",
+            bio="A legendary rock band from Liverpool.",
+            sign_date=date(1962, 1, 1),
+        )
+        Band.objects.create(
+            name="Mohiner Ghoraguli",
+            bio="A progressive rock band from Calcutta.",
+            sign_date=date(1975, 1, 1),
+        )
+        queryset = Band.objects.all().order_by("-id")[:3]
+        self.assertEqual(len(queryset), 3)
+        with self.assertNumQueries(1):
+            ma.log_deletions(mock_request, queryset)
+        logs = (
+            LogEntry.objects.filter(action_flag=DELETION)
+            .order_by("id")
+            .values_list(
+                "user_id",
+                "content_type",
+                "object_id",
+                "object_repr",
+                "action_flag",
+                "change_message",
+            )
+        )
+        expected_log_values = [
+            (
+                mock_request.user.id,
+                content_type.id,
+                str(obj.pk),
+                str(obj),
+                DELETION,
+                "",
+            )
+            for obj in queryset
+        ]
+        self.assertSequenceEqual(logs, expected_log_values)
+
+    # RemovedInDjango60Warning.
     def test_log_deletion(self):
         ma = ModelAdmin(Band, self.site)
         mock_request = MockRequest()
         mock_request.user = User.objects.create(username="bill")
         content_type = get_content_type_for_model(self.band)
-        created = ma.log_deletion(mock_request, self.band, str(self.band))
+        msg = "ModelAdmin.log_deletion() is deprecated. Use log_deletions() instead."
+        with self.assertWarnsMessage(RemovedInDjango60Warning, msg):
+            created = ma.log_deletion(mock_request, self.band, str(self.band))
         fetched = LogEntry.objects.filter(action_flag=DELETION).latest("id")
         self.assertEqual(created, fetched)
         self.assertEqual(fetched.action_flag, DELETION)
@@ -848,6 +895,60 @@ class ModelAdminTests(TestCase):
         self.assertEqual(fetched.change_message, "")
         self.assertEqual(fetched.object_repr, str(self.band))
 
+    # RemovedInDjango60Warning.
+    def test_log_deletion_fallback(self):
+        class InheritedModelAdmin(ModelAdmin):
+            def log_deletion(self, request, obj, object_repr):
+                return super().log_deletion(request, obj, object_repr)
+
+        ima = InheritedModelAdmin(Band, self.site)
+        mock_request = MockRequest()
+        mock_request.user = User.objects.create(username="akash")
+        content_type = get_content_type_for_model(self.band)
+        Band.objects.create(
+            name="The Beatles",
+            bio="A legendary rock band from Liverpool.",
+            sign_date=date(1962, 1, 1),
+        )
+        Band.objects.create(
+            name="Mohiner Ghoraguli",
+            bio="A progressive rock band from Calcutta.",
+            sign_date=date(1975, 1, 1),
+        )
+        queryset = Band.objects.all().order_by("-id")[:3]
+        self.assertEqual(len(queryset), 3)
+        msg = (
+            "The usage of log_deletion() is deprecated. Implement log_deletions() "
+            "instead."
+        )
+        with self.assertNumQueries(3):
+            with self.assertWarnsMessage(RemovedInDjango60Warning, msg):
+                ima.log_deletions(mock_request, queryset)
+        logs = (
+            LogEntry.objects.filter(action_flag=DELETION)
+            .order_by("id")
+            .values_list(
+                "user_id",
+                "content_type",
+                "object_id",
+                "object_repr",
+                "action_flag",
+                "change_message",
+            )
+        )
+        expected_log_values = [
+            (
+                mock_request.user.id,
+                content_type.id,
+                str(obj.pk),
+                str(obj),
+                DELETION,
+                "",
+            )
+            for obj in queryset
+        ]
+        self.assertSequenceEqual(logs, expected_log_values)
+
     def test_get_autocomplete_fields(self):
         class NameAdmin(ModelAdmin):
             search_fields = ["name"]