diff --git a/constance/admin.py b/constance/admin.py index 188f32e..712f8a9 100644 --- a/constance/admin.py +++ b/constance/admin.py @@ -9,7 +9,10 @@ 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.contenttypes.models import ContentType from django.core.exceptions import PermissionDenied from django.http import HttpResponseRedirect from django.template.response import TemplateResponse @@ -38,6 +41,14 @@ 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 /change/ to the changelist so that "Recent actions" links in the admin index point + # somewhere useful. + path( + "/change/", + 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 +105,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,6 +168,65 @@ 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.""" + from django.contrib.admin.views.main import PAGE_VAR + + 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 diff --git a/constance/forms.py b/constance/forms.py index 321ab47..4975b99 100644 --- a/constance/forms.py +++ b/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"] diff --git a/constance/templates/admin/constance/change_list.html b/constance/templates/admin/constance/change_list.html index 72b834a..fce2bfb 100644 --- a/constance/templates/admin/constance/change_list.html +++ b/constance/templates/admin/constance/change_list.html @@ -23,6 +23,9 @@ {% block bodyclass %}{{ block.super }} change-list{% endblock %} {% block content %} +
{% csrf_token %} diff --git a/constance/templates/admin/constance/config_history.html b/constance/templates/admin/constance/config_history.html new file mode 100644 index 0000000..078fff9 --- /dev/null +++ b/constance/templates/admin/constance/config_history.html @@ -0,0 +1,55 @@ +{% extends "admin/base_site.html" %} +{% load i18n %} + +{% block breadcrumbs %} + +{% endblock %} + +{% block content %} +
+
+ +{% if action_list %} + + + + + + + + + + {% for action in action_list %} + + + + + + {% endfor %} + +
{% translate 'Date/time' %}{% translate 'User' %}{% translate 'Action' %}
{{ action.action_time|date:"DATETIME_FORMAT" }}{{ action.user.get_username }}{% if action.user.get_full_name %} ({{ action.user.get_full_name }}){% endif %}{{ action.get_change_message }}
+

+ {% if pagination_required %} + {% for i in page_range %} + {% if i == action_list.paginator.ELLIPSIS %} + {{ action_list.paginator.ELLIPSIS }} + {% elif i == action_list.number %} + {{ i }} + {% else %} + {{ i }} + {% endif %} + {% endfor %} + {% endif %} + {{ action_list.paginator.count }} {% blocktranslate count counter=action_list.paginator.count %}entry{% plural %}entries{% endblocktranslate %} +

+{% else %} +

{% translate "This object doesn't have a change history." %}

+{% endif %} +
+
+{% endblock %} diff --git a/tests/test_admin.py b/tests/test_admin.py index 4f25339..05d35ac 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -1,3 +1,4 @@ +import json from datetime import datetime from unittest import mock @@ -28,6 +29,11 @@ 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. + from django.contrib.contenttypes.models import ContentType + + ContentType.objects.clear_cache() def test_changelist(self): self.client.login(username="admin", password="nimda") @@ -126,7 +132,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 +328,151 @@ 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.""" + from django.contrib.admin.models import CHANGE + from django.contrib.admin.models import LogEntry + + 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.""" + from django.contrib.admin.models import LogEntry + + 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.""" + 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, + 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("0", 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 + 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() + self.assertIn('href="history/"', 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.""" + 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) + 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)