Added admin logentry (#662)

* Added Logentry creation

* Added history view

* Used url template tag for history

* Minor cleanup

constance/admin.py

@@ -9,7 +9,11 @@ from django import get_version
 from django.apps import apps
 from django.contrib import admin
 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
 from django.template.response import TemplateResponse
@@ -38,6 +42,15 @@ 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.  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/",
+                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"),
         ]
 
     def get_config_value(self, name, options, form, initial):
@@ -94,7 +107,9 @@ class ConstanceAdmin(admin.ModelAdmin):
         if request.method == "POST" and request.user.has_perm("constance.change_config"):
             form = form_cls(data=request.POST, files=request.FILES, initial=initial, request=request)
             if form.is_valid():
-                form.save()
+                changed_fields = form.save()
+                if changed_fields:
+                    self._log_config_change(request, changed_fields)
                 messages.add_message(request, messages.SUCCESS, _("Live settings updated successfully."))
                 return HttpResponseRedirect(".")
             messages.add_message(request, messages.ERROR, _("Failed to update live settings."))
@@ -155,12 +170,74 @@ class ConstanceAdmin(admin.ModelAdmin):
         request.current_app = self.admin_site.name
         return TemplateResponse(request, self.change_list_template, context)
 
+    def history_view(self, request, object_id=None, extra_context=None):
+        """Display the change history for constance config values."""
+        if not self.has_view_or_change_permission(request):
+            raise PermissionDenied
+
+        ct = ContentType.objects.get_for_model(self.model)
+        action_list = (
+            LogEntry.objects.filter(
+                content_type=ct,
+                object_id="Config",
+            )
+            .select_related()
+            .order_by("-action_time")
+        )
+
+        paginator = self.get_paginator(request, action_list, 100)
+        page_number = request.GET.get(PAGE_VAR, 1)
+        page_obj = paginator.get_page(page_number)
+        page_range = paginator.get_elided_page_range(page_obj.number)
+
+        context = {
+            **self.admin_site.each_context(request),
+            "title": _("Change history: %s") % self.model._meta.verbose_name_plural.capitalize(),
+            "action_list": page_obj,
+            "page_range": page_range,
+            "page_var": PAGE_VAR,
+            "pagination_required": paginator.count > 100,
+            "opts": self.model._meta,
+            "app_label": "constance",
+            **(extra_context or {}),
+        }
+
+        request.current_app = self.admin_site.name
+        return TemplateResponse(
+            request,
+            "admin/constance/config_history.html",
+            context,
+        )
+
+    def _log_config_change(self, request, changed_fields):
+        """
+        Create a Django admin LogEntry recording which config fields were changed.
+
+        Uses the standard Django JSON change_message format so that
+        LogEntry.get_change_message() can interpret it correctly.
+        """
+        ct = ContentType.objects.get_for_model(self.model)
+        change_message = json.dumps([{"changed": {"fields": changed_fields}}])
+        LogEntry.objects.create(
+            user_id=request.user.pk,
+            content_type_id=ct.pk,
+            object_id="Config",
+            object_repr="Config",
+            action_flag=CHANGE,
+            change_message=change_message,
+        )
+
     def has_add_permission(self, *args, **kwargs):
         return False
 
     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/forms.py

@@ -124,10 +124,16 @@ class ConstanceForm(forms.Form):
         self.initial["version"] = version_hash.hexdigest()
 
     def save(self):
+        """
+        Save changed config values to the backend.
+
+        Returns a list of config field names that were actually modified.
+        """
         for file_field in self.files:
             file = self.cleaned_data[file_field]
             self.cleaned_data[file_field] = default_storage.save(join(settings.FILE_ROOT, file.name), file)
 
+        changed_fields = []
         for name in settings.CONFIG:
             current = getattr(config, name)
             new = self.cleaned_data[name]
@@ -140,6 +146,9 @@ class ConstanceForm(forms.Form):
 
             if current != new:
                 setattr(config, name, new)
+                changed_fields.append(name)
+
+        return changed_fields
 
     def clean_version(self):
         value = self.cleaned_data["version"]

constance/templates/admin/constance/change_list.html

@@ -23,6 +23,9 @@
 {% block bodyclass %}{{ block.super }} change-list{% endblock %}
 
 {% block content %}
+  <ul class="object-tools">
+    <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">
         <form id="changelist-form" action="" method="post" enctype="multipart/form-data">{% csrf_token %}

constance/templates/admin/constance/config_history.html

@@ -0,0 +1,55 @@
+{% extends "admin/base_site.html" %}
+{% load i18n %}
+
+{% block breadcrumbs %}
+<div class="breadcrumbs">
+  <a href="{% url 'admin:index' %}">{% translate 'Home' %}</a>
+  &rsaquo; <a href="{% url 'admin:app_list' app_label=opts.app_label %}">{{ opts.app_config.verbose_name }}</a>
+  &rsaquo; <a href="{% url 'admin:constance_config_changelist' %}">{{ opts.verbose_name_plural|capfirst }}</a>
+  &rsaquo; {% translate 'History' %}
+</div>
+{% endblock %}
+
+{% block content %}
+<div id="content-main">
+<div id="change-history" class="module">
+
+{% if action_list %}
+    <table>
+        <thead>
+        <tr>
+            <th scope="col">{% translate 'Date/time' %}</th>
+            <th scope="col">{% translate 'User' %}</th>
+            <th scope="col">{% translate 'Action' %}</th>
+        </tr>
+        </thead>
+        <tbody>
+        {% for action in action_list %}
+        <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.get_change_message }}</td>
+        </tr>
+        {% endfor %}
+        </tbody>
+    </table>
+    <p class="paginator">
+      {% if pagination_required %}
+        {% for i in page_range %}
+          {% if i == action_list.paginator.ELLIPSIS %}
+            {{ action_list.paginator.ELLIPSIS }}
+          {% elif i == action_list.number %}
+            <span class="this-page">{{ i }}</span>
+          {% else %}
+            <a href="?{{ page_var }}={{ i }}" {% if i == action_list.paginator.num_pages %} class="end" {% endif %}>{{ i }}</a>
+          {% endif %}
+        {% endfor %}
+      {% endif %}
+      {{ action_list.paginator.count }} {% blocktranslate count counter=action_list.paginator.count %}entry{% plural %}entries{% endblocktranslate %}
+    </p>
+{% else %}
+    <p>{% translate "This object doesn't have a change history." %}</p>
+{% endif %}
+</div>
+</div>
+{% endblock %}

tests/test_admin.py

@@ -1,14 +1,20 @@
+import json
 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
@@ -28,6 +34,9 @@ class TestAdmin(TestCase):
         self.normaluser.is_staff = True
         self.normaluser.save()
         self.options = admin.site._registry[self.model]
+        # Clear ContentType cache to avoid stale content_type_id references
+        # across tests wrapped in transactions.
+        ContentType.objects.clear_cache()
 
     def test_changelist(self):
         self.client.login(username="admin", password="nimda")
@@ -126,7 +135,7 @@ class TestAdmin(TestCase):
         },
     )
     @mock.patch("constance.settings.IGNORE_ADMIN_VERSION_CHECK", True)
-    @mock.patch("constance.forms.ConstanceForm.save", lambda _: None)
+    @mock.patch("constance.forms.ConstanceForm.save", lambda _: [])
     @mock.patch("constance.forms.ConstanceForm.is_valid", lambda _: True)
     def test_submit(self):
         """
@@ -322,6 +331,137 @@ class TestAdmin(TestCase):
             # Clean up FIELDS to avoid leaking into other tests
             FIELDS.pop("language_select", None)
 
+    @mock.patch("constance.settings.CONFIG_FIELDSETS", {"FieldSetOne": ("INT_VALUE", "STRING_VALUE")})
+    @mock.patch(
+        "constance.settings.CONFIG",
+        {
+            "INT_VALUE": (1, "some int"),
+            "STRING_VALUE": ("Hello world", "greetings"),
+        },
+    )
+    @mock.patch("constance.settings.IGNORE_ADMIN_VERSION_CHECK", True)
+    @mock.patch("constance.forms.ConstanceForm.save", lambda _: ["INT_VALUE"])
+    @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."""
+        request = self.rf.post(
+            "/admin/constance/config/",
+            data={
+                "INT_VALUE": "42",
+                "STRING_VALUE": "Hello world",
+                "version": "123",
+            },
+        )
+        request.user = self.superuser
+        request._dont_enforce_csrf_checks = True
+
+        with mock.patch("django.contrib.messages.add_message"):
+            response = self.options.changelist_view(request, {})
+
+        self.assertIsInstance(response, HttpResponseRedirect)
+        log_entry = LogEntry.objects.latest("pk")
+        self.assertEqual(log_entry.user, self.superuser)
+        self.assertEqual(log_entry.action_flag, CHANGE)
+        self.assertEqual(log_entry.object_repr, "Config")
+        # Verify change_message uses Django's standard JSON format
+        # so that get_change_message() can render it correctly.
+        self.assertEqual(
+            log_entry.get_change_message(),
+            "Changed INT_VALUE.",
+        )
+
+    @mock.patch("constance.settings.CONFIG_FIELDSETS", {"FieldSetOne": ("INT_VALUE",)})
+    @mock.patch(
+        "constance.settings.CONFIG",
+        {
+            "INT_VALUE": (1, "some int"),
+        },
+    )
+    @mock.patch("constance.settings.IGNORE_ADMIN_VERSION_CHECK", True)
+    @mock.patch("constance.forms.ConstanceForm.save", lambda _: [])
+    @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."""
+        initial_count = LogEntry.objects.count()
+        request = self.rf.post(
+            "/admin/constance/config/",
+            data={
+                "INT_VALUE": "1",
+                "version": "123",
+            },
+        )
+        request.user = self.superuser
+        request._dont_enforce_csrf_checks = True
+
+        with mock.patch("django.contrib.messages.add_message"):
+            response = self.options.changelist_view(request, {})
+
+        self.assertIsInstance(response, HttpResponseRedirect)
+        self.assertEqual(LogEntry.objects.count(), initial_count)
+
+    def test_history_view(self):
+        """Test that the history view renders and shows LogEntry records."""
+        ct = ContentType.objects.get_for_model(self.model)
+        LogEntry.objects.create(
+            user_id=self.superuser.pk,
+            content_type_id=ct.pk,
+            object_id="Config",
+            object_repr="Config",
+            action_flag=CHANGE,
+            change_message=json.dumps([{"changed": {"fields": ["INT_VALUE"]}}]),
+        )
+
+        request = self.rf.get("/admin/constance/config/history/")
+        request.user = self.superuser
+        response = self.options.history_view(request)
+        self.assertEqual(response.status_code, 200)
+        response.render()
+        content = response.content.decode()
+        self.assertIn("INT_VALUE", content)
+        self.assertIn("History", content)
+
+    def test_history_view_empty(self):
+        """Test that the history view renders correctly with no entries."""
+        request = self.rf.get("/admin/constance/config/history/")
+        request.user = self.superuser
+        response = self.options.history_view(request)
+        self.assertEqual(response.status_code, 200)
+        response.render()
+        content = response.content.decode()
+        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."""
+        unprivileged = User.objects.create_user("noperm", "noperm", "c@c.cz")
+        request = self.rf.get("/admin/constance/config/history/")
+        request.user = unprivileged
+        with self.assertRaises(PermissionDenied):
+            self.options.history_view(request)
+
+    def test_changelist_has_history_link(self):
+        """Test that the changelist page contains a link to the history view."""
+        request = self.rf.get("/admin/constance/config/")
+        request.user = self.superuser
+        response = self.options.changelist_view(request)
+        response.render()
+        content = response.content.decode()
+        history_url = reverse("admin:constance_config_history")
+        self.assertIn(f'href="{history_url}"', content)
+        self.assertIn("History", content)
+
+    def test_change_url_redirects_to_changelist(self):
+        """Test that the change URL (used by 'Recent actions') redirects to the changelist."""
+        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.
+        match = resolve(url)
+        response = match.func(request, object_id="Config")
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, "../../")
+
     def test_labels(self):
         self.assertEqual(type(self.model._meta.label), str)
         self.assertEqual(type(self.model._meta.label_lower), str)