Minor cleanup

constance/admin.py

@@ -12,6 +12,7 @@ from django.contrib import messages
 from django.contrib.admin.models import CHANGE
 from django.contrib.admin.models import LogEntry
 from django.contrib.admin.options import csrf_protect_m
+from django.contrib.admin.views.main import PAGE_VAR
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponseRedirect
@@ -41,11 +42,12 @@ class ConstanceAdmin(admin.ModelAdmin):
         return [
             path("", self.admin_site.admin_view(self.changelist_view), name=f"{info}_changelist"),
             path("", self.admin_site.admin_view(self.changelist_view), name=f"{info}_add"),
-            # Redirect <object_id>/change/ to the changelist so that "Recent actions" links in the admin index point
-            # somewhere useful.
+            # Redirect <object_id>/change/ to the changelist so that "Recent actions" links in the admin index
+            # point somewhere useful.  The relative "../../" resolves to the constance changelist because the
+            # full path is <app>/<model>/<object_id>/change/ and two levels up lands on <app>/<model>/.
             path(
                 "<path:object_id>/change/",
-                lambda request, object_id: HttpResponseRedirect("../../"),
+                self.admin_site.admin_view(lambda request, object_id: HttpResponseRedirect("../../")),
                 name=f"{info}_change",
             ),
             path("history/", self.admin_site.admin_view(self.history_view), name=f"{info}_history"),
@@ -170,8 +172,6 @@ class ConstanceAdmin(admin.ModelAdmin):
 
     def history_view(self, request, object_id=None, extra_context=None):
         """Display the change history for constance config values."""
-        from django.contrib.admin.views.main import PAGE_VAR
-
         if not self.has_view_or_change_permission(request):
             raise PermissionDenied
 
@@ -233,6 +233,11 @@ class ConstanceAdmin(admin.ModelAdmin):
     def has_delete_permission(self, *args, **kwargs):
         return False
 
+    def has_view_permission(self, request, obj=None):
+        if settings.SUPERUSER_ONLY:
+            return request.user.is_superuser
+        return super().has_view_permission(request, obj)
+
     def has_change_permission(self, request, obj=None):
         if settings.SUPERUSER_ONLY:
             return request.user.is_superuser

constance/templates/admin/constance/change_list.html

@@ -24,7 +24,7 @@
 
 {% block content %}
   <ul class="object-tools">
-    <li><a href="{% url 'admin:constance_config_history' %}" class="historylink">{% trans 'History' %}</a></li>
+    <li><a href="{% url 'admin:constance_config_history' %}" class="historylink">{% translate 'History' %}</a></li>
   </ul>
   <div id="content-main" class="constance">
     <div class="module" id="changelist">

tests/test_admin.py

@@ -3,13 +3,18 @@ from datetime import datetime
 from unittest import mock
 
 from django.contrib import admin
+from django.contrib.admin.models import CHANGE
+from django.contrib.admin.models import LogEntry
 from django.contrib.auth.models import Permission
 from django.contrib.auth.models import User
+from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
 from django.http import HttpResponseRedirect
 from django.template.defaultfilters import linebreaksbr
 from django.test import RequestFactory
 from django.test import TestCase
+from django.urls import resolve
+from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
 from constance import settings
@@ -31,8 +36,6 @@ class TestAdmin(TestCase):
         self.options = admin.site._registry[self.model]
         # Clear ContentType cache to avoid stale content_type_id references
         # across tests wrapped in transactions.
-        from django.contrib.contenttypes.models import ContentType
-
         ContentType.objects.clear_cache()
 
     def test_changelist(self):
@@ -341,9 +344,6 @@ class TestAdmin(TestCase):
     @mock.patch("constance.forms.ConstanceForm.is_valid", lambda _: True)
     def test_log_entry_created_on_change(self):
         """Test that a valid LogEntry is created when config values are changed."""
-        from django.contrib.admin.models import CHANGE
-        from django.contrib.admin.models import LogEntry
-
         request = self.rf.post(
             "/admin/constance/config/",
             data={
@@ -382,8 +382,6 @@ class TestAdmin(TestCase):
     @mock.patch("constance.forms.ConstanceForm.is_valid", lambda _: True)
     def test_no_log_entry_when_no_changes(self):
         """Test that no LogEntry is created when the form is saved without any changes."""
-        from django.contrib.admin.models import LogEntry
-
         initial_count = LogEntry.objects.count()
         request = self.rf.post(
             "/admin/constance/config/",
@@ -403,10 +401,6 @@ class TestAdmin(TestCase):
 
     def test_history_view(self):
         """Test that the history view renders and shows LogEntry records."""
-        from django.contrib.admin.models import CHANGE
-        from django.contrib.admin.models import LogEntry
-        from django.contrib.contenttypes.models import ContentType
-
         ct = ContentType.objects.get_for_model(self.model)
         LogEntry.objects.create(
             user_id=self.superuser.pk,
@@ -434,12 +428,10 @@ class TestAdmin(TestCase):
         self.assertEqual(response.status_code, 200)
         response.render()
         content = response.content.decode()
-        self.assertIn("0", content)
+        self.assertIn("doesn't have a change history", content)
 
     def test_history_view_permission_denied(self):
         """Test that the history view denies access to users without permission."""
-        from django.contrib.auth.models import User
-
         unprivileged = User.objects.create_user("noperm", "noperm", "c@c.cz")
         request = self.rf.get("/admin/constance/config/history/")
         request.user = unprivileged
@@ -448,8 +440,6 @@ class TestAdmin(TestCase):
 
     def test_changelist_has_history_link(self):
         """Test that the changelist page contains a link to the history view."""
-        from django.urls import reverse
-
         request = self.rf.get("/admin/constance/config/")
         request.user = self.superuser
         response = self.options.changelist_view(request)
@@ -461,16 +451,12 @@ class TestAdmin(TestCase):
 
     def test_change_url_redirects_to_changelist(self):
         """Test that the change URL (used by 'Recent actions') redirects to the changelist."""
-        from django.urls import reverse
-
         url = reverse("admin:constance_config_change", args=["Config"])
         self.assertIn("Config/change/", url)
         request = self.rf.get(url)
         request.user = self.superuser
 
         # The change URL is a simple lambda redirect, so invoke it via URL resolution.
-        from django.urls import resolve
-
         match = resolve(url)
         response = match.func(request, object_id="Config")
         self.assertEqual(response.status_code, 302)