Implement history_order_by_date and get_version_ordering() hook

micmarc · micmarc · commit 7ea93c5ea643 · 2025-11-23T12:56:54.000-05:00
diff --git a/docs/admin.rst b/docs/admin.rst
@@ -90,6 +90,11 @@ A subclass of ``django.contrib.ModelAdmin`` providing rollback and recovery.
     If ``True``, revisions will be displayed with the most recent revision first.
 
 
+``history_order_by_date = False``
+
+    If ``True``, revisions will be ordered by ``date_created`` instead of the numeric version ID.
+
+
 .. _VersionAdmin_register:
 
 ``reversion_register(model, **options)``
@@ -107,3 +112,21 @@ A subclass of ``django.contrib.ModelAdmin`` providing rollback and recovery.
 
     ``options``
         Registration options, see :ref:`reversion.register() <register>`.
+
+.. _VersionAdmin_get_version_ordering:
+
+``get_version_ordering(model, **options)``
+
+    Hook for custom or dynamic ordering of the version queryset, with similar semantics to the built-in ``get_ordering`` method in Django's ``ModelAdmin``. Implementations are expected to return a tuple specifying the field names (relative to the ``Version`` model) for ordering.
+
+    The values of ``history_latest_first`` and ``history_order_by_date`` are ignored when overriding this method unless explicitly used in the override logic.
+
+    .. code:: python
+
+        def get_version_ordering(self, request):
+            if request.user.is_superuser():
+                return ("-revision__date_created", "pk")
+            return ("-pk",)
+
+    ``request``
+        The current request.
diff --git a/reversion/admin.py b/reversion/admin.py
@@ -40,10 +40,16 @@ class VersionAdmin(admin.ModelAdmin):
 
     history_latest_first = False
 
+    history_order_by_date = False
+
     def reversion_register(self, model, **kwargs):
         """Registers the model with reversion."""
         register(model, **kwargs)
 
+    def get_version_ordering(self, request):
+        """Hook for specifying custom field ordering for the version queryset."""
+        pass
+
     @contextmanager
     def create_revision(self, request):
         with create_revision():
@@ -60,11 +66,20 @@ def _reversion_get_template_list(self, template_name):
             "reversion/%s" % template_name,
         )
 
-    def _reversion_order_version_queryset(self, queryset):
+    def _reversion_order_version_queryset(self, request, queryset):
         """Applies the correct ordering to the given version queryset."""
-        if not self.history_latest_first:
-            queryset = queryset.order_by("pk")
-        return queryset
+        ordering = self.get_version_ordering(request) or ()
+        # Default ordering logic should remain here to prevent accidental side effects
+        # from calling super().get_version_ordering() in subclass implementations
+        if not ordering:
+            order_fields = ["pk"]
+            if self.history_order_by_date:
+                order_fields.insert(0, "revision__date_created")
+            # history_latest_first causes order to be reversed on all fields
+            if self.history_latest_first:
+                order_fields = [f"-{field}" for field in order_fields]
+            ordering = tuple(order_fields)
+        return queryset.order_by(*ordering)
 
     # Messages.
 
@@ -258,6 +273,7 @@ def recoverlist_view(self, request, extra_context=None):
         model = self.model
         opts = model._meta
         deleted = self._reversion_order_version_queryset(
+            request,
             Version.objects.get_deleted(self.model).select_related("revision")
         )
         # Set the app name.
@@ -298,10 +314,10 @@ def history_view(self, request, object_id, extra_context=None):
                 ),
             }
             for version
-            in self._reversion_order_version_queryset(Version.objects.get_for_object_reference(
+            in self._reversion_order_version_queryset(request, Version.objects.get_for_object_reference(
                 self.model,
                 unquote(object_id),  # Underscores in primary key get quoted to "_5F"
-            ).select_related("revision__user"))
+            ).select_related("revision", "revision__user"))
         ]
         # Compile the context.
         context = {"action_list": action_list}
diff --git a/tests/test_app/tests/test_admin.py b/tests/test_app/tests/test_admin.py
@@ -1,8 +1,11 @@
 import re
+from datetime import datetime, timedelta
+
 from django.contrib import admin
 from django.contrib.contenttypes.admin import GenericTabularInline
 from django.db.models.signals import pre_save, post_save, pre_delete, post_delete, m2m_changed
 from django.shortcuts import resolve_url
+
 import reversion
 from reversion.admin import VersionAdmin
 from reversion.models import Version
@@ -248,6 +251,255 @@ def testHistorylistView(self):
             Version.objects.get_for_model(TestModelParent).get().pk,
         ))
 
+    def testHistorylistViewOrderDefault(self):
+        # Create an object and multiple revisions.
+        with reversion.create_revision():
+            obj = TestModelParent.objects.create(name="v1", parent_name="p1")
+        with reversion.create_revision():
+            obj.name = "v2"
+            obj.save()
+        with reversion.create_revision():
+            obj.name = "v3"
+            obj.save()
+
+        # Fetch history page.
+        response = self.client.get(resolve_url("admin:test_app_testmodelparent_history", obj.pk))
+        content = response.content.decode()
+
+        # Compute expected order: default VersionAdmin orders by pk ascending (oldest first).
+        version_ids = list(Version.objects.get_for_object(obj).values_list("pk", flat=True))
+        expected_order = sorted(version_ids)
+
+        # Build the URLs as rendered in the history list and assert their order.
+        urls_in_order = [
+            resolve_url(
+                "admin:test_app_testmodelparent_revision",
+                obj.pk,
+                vid,
+            )
+            for vid in expected_order
+        ]
+
+        # Ensure each subsequent URL appears later in the content than the previous one.
+        last_index = -1
+        for url in urls_in_order:
+            index = content.find(url)
+            self.assertNotEqual(index, -1, f"Expected to find {url} in history page")
+            self.assertGreater(index, last_index, "History list is not ordered by ascending version pk (oldest first)")
+            last_index = index
+
+
+class AdminHistoryViewLatestFirstTest(LoginMixin, TestBase):
+
+    class TestModelParentAdminLatestFirst(VersionAdmin):
+        history_latest_first = True
+
+    def setUp(self):
+        super().setUp()
+        # Register a custom admin with history_latest_first enabled
+        admin.site.register(TestModelParent, self.TestModelParentAdminLatestFirst)
+        self.reloadUrls()
+
+    def tearDown(self):
+        super().tearDown()
+        admin.site.unregister(TestModelParent)
+        self.reloadUrls()
+
+    def testHistorylistViewOrderLatestFirst(self):
+        # Create an object and multiple revisions.
+        with reversion.create_revision():
+            obj = TestModelParent.objects.create(name="v1", parent_name="p1")
+        with reversion.create_revision():
+            obj.name = "v2"
+            obj.save()
+        with reversion.create_revision():
+            obj.name = "v3"
+            obj.save()
+
+        # Fetch history page.
+        response = self.client.get(resolve_url("admin:test_app_testmodelparent_history", obj.pk))
+        content = response.content.decode()
+
+        # Expected order: with history_latest_first=True, versions are ordered by pk descending (newest first).
+        version_ids = list(Version.objects.get_for_object(obj).values_list("pk", flat=True))
+        expected_order = sorted(version_ids, reverse=True)
+
+        urls_in_order = [
+            resolve_url("admin:test_app_testmodelparent_revision", obj.pk, vid)
+            for vid in expected_order
+        ]
+
+        last_index = -1
+        for url in urls_in_order:
+            index = content.find(url)
+            self.assertNotEqual(index, -1, f"Expected to find {url} in history page")
+            self.assertGreater(index, last_index, "History list is not ordered by descending version pk (newest first)")
+            last_index = index
+
+
+class AdminHistoryViewOrderByDateTest(LoginMixin, TestBase):
+
+    class TestModelParentAdminOrderByDate(VersionAdmin):
+        history_order_by_date = True
+
+    def setUp(self):
+        super().setUp()
+        # Register a custom admin with history_order_by_date enabled
+        admin.site.register(TestModelParent, self.TestModelParentAdminOrderByDate)
+        self.reloadUrls()
+
+    def tearDown(self):
+        super().tearDown()
+        admin.site.unregister(TestModelParent)
+        self.reloadUrls()
+
+    def testHistorylistViewOrderByDate(self):
+        # Create an object and multiple revisions with increasing timestamps.
+        with reversion.create_revision():
+            obj = TestModelParent.objects.create(name="v1", parent_name="p1")
+            # Use an out-of-sequence date to verify correct ordering
+            reversion.set_date_created(datetime.now() + timedelta(days=1))
+        with reversion.create_revision():
+            obj.name = "v2"
+            obj.save()
+        with reversion.create_revision():
+            obj.name = "v3"
+            obj.save()
+
+        # Fetch history page.
+        response = self.client.get(resolve_url("admin:test_app_testmodelparent_history", obj.pk))
+        content = response.content.decode()
+
+        # Expected order: ordered by revision creation date ascending (oldest date first).
+        versions = (
+            Version.objects.get_for_object(obj)
+            .select_related("revision")
+            .order_by("revision__date_created", "pk")
+        )
+        expected_order = list(versions.values_list("pk", flat=True))
+
+        urls_in_order = [
+            resolve_url("admin:test_app_testmodelparent_revision", obj.pk, vid)
+            for vid in expected_order
+        ]
+
+        last_index = -1
+        for url in urls_in_order:
+            index = content.find(url)
+            self.assertNotEqual(index, -1, f"Expected to find {url} in history page")
+            self.assertGreater(index, last_index, "History list is not ordered by revision date (oldest first)")
+            last_index = index
+
+
+class AdminHistoryViewLatestFirstOrderByDateTest(LoginMixin, TestBase):
+
+    class TestModelParentAdminLatestFirstOrderByDate(VersionAdmin):
+        history_latest_first = True
+        history_order_by_date = True
+
+    def setUp(self):
+        super().setUp()
+        # Register a custom admin with both flags enabled
+        admin.site.register(TestModelParent, self.TestModelParentAdminLatestFirstOrderByDate)
+        self.reloadUrls()
+
+    def tearDown(self):
+        super().tearDown()
+        admin.site.unregister(TestModelParent)
+        self.reloadUrls()
+
+    def testHistorylistViewOrderLatestFirstByDate(self):
+        # Create an object and multiple revisions with increasing timestamps.
+        with reversion.create_revision():
+            obj = TestModelParent.objects.create(name="v1", parent_name="p1")
+            # Use an out-of-sequence date to verify correct ordering
+            reversion.set_date_created(datetime.now() + timedelta(days=1))
+        with reversion.create_revision():
+            obj.name = "v2"
+            obj.save()
+        with reversion.create_revision():
+            obj.name = "v3"
+            obj.save()
+
+        # Fetch history page.
+        response = self.client.get(resolve_url("admin:test_app_testmodelparent_history", obj.pk))
+        content = response.content.decode()
+
+        # Expected order: ordered by revision creation date descending (newest date first).
+        versions = (
+            Version.objects.get_for_object(obj)
+            .select_related("revision")
+            .order_by("-revision__date_created", "-pk")
+        )
+        expected_order = list(versions.values_list("pk", flat=True))
+
+        urls_in_order = [
+            resolve_url("admin:test_app_testmodelparent_revision", obj.pk, vid)
+            for vid in expected_order
+        ]
+
+        last_index = -1
+        for url in urls_in_order:
+            index = content.find(url)
+            self.assertNotEqual(index, -1, f"Expected to find {url} in history page")
+            self.assertGreater(index, last_index, "History list is not ordered by revision date (newest first)")
+            last_index = index
+
+
+class AdminHistoryViewCustomOrderingTest(LoginMixin, TestBase):
+    class TestModelParentAdminCustomOrdering(VersionAdmin):
+        def get_version_ordering(self, request):
+            return ("revision__comment",)
+
+    def setUp(self):
+        super().setUp()
+        # Register a custom admin with history_order_by_date enabled
+        admin.site.register(TestModelParent, self.TestModelParentAdminCustomOrdering)
+        self.reloadUrls()
+
+    def tearDown(self):
+        super().tearDown()
+        admin.site.unregister(TestModelParent)
+        self.reloadUrls()
+
+    def testHistorylistViewCustomOrdering(self):
+        # Create an object and multiple revisions with increasing timestamps.
+        with reversion.create_revision():
+            obj = TestModelParent.objects.create(name="v1", parent_name="p1")
+            reversion.set_comment("B")
+        with reversion.create_revision():
+            obj.name = "v2"
+            obj.save()
+            reversion.set_comment("A")
+        with reversion.create_revision():
+            obj.name = "v3"
+            obj.save()
+            reversion.set_comment("C")
+
+        # Fetch history page.
+        response = self.client.get(resolve_url("admin:test_app_testmodelparent_history", obj.pk))
+        content = response.content.decode()
+
+        # Expected order: ordered by revision comment ascending.
+        versions = (
+            Version.objects.get_for_object(obj)
+            .select_related("revision")
+            .order_by("revision__comment")
+        )
+        expected_order = list(versions.values_list("pk", flat=True))
+
+        urls_in_order = [
+            resolve_url("admin:test_app_testmodelparent_revision", obj.pk, vid)
+            for vid in expected_order
+        ]
+
+        last_index = -1
+        for url in urls_in_order:
+            index = content.find(url)
+            self.assertNotEqual(index, -1, f"Expected to find {url} in history page")
+            self.assertGreater(index, last_index, "History list is not ordered by comment ascending")
+            last_index = index
+
 
 class AdminQuotingTest(LoginMixin, AdminMixin, TestBase):
 
