From 630e4f6658214c9ac83cc6ef1d5191d7e73ac4b9 Mon Sep 17 00:00:00 2001 From: Marco Sirabella Date: Fri, 18 Aug 2023 18:16:52 -0700 Subject: [PATCH] Expose change reason to admin form Fixes #853 --- CHANGES.rst | 1 + docs/admin.rst | 4 +-- simple_history/admin.py | 25 +++++++++++++- simple_history/tests/tests/test_admin.py | 44 ++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index fb4f6a6ff..2a49cfa10 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,7 @@ Unreleased in ``settings`` (gh-1017). - Fixed ``SimpleHistoryAdmin`` not properly integrating with custom user models (gh-1177) - Fixes self-m2m fields not using the correct through field name (gh-1218) +- Allow setting change reason through ``SimpleHistoryAdmin``'s change form (gh-1232) 3.3.0 (2023-03-08) ------------------ diff --git a/docs/admin.rst b/docs/admin.rst index abd669be6..0c688b30a 100644 --- a/docs/admin.rst +++ b/docs/admin.rst @@ -7,7 +7,7 @@ model with the admin site. This will replace the history object page on the admin site and allow viewing and reverting to previous model versions. Changes made in admin change forms -will also accurately note the user who made the change. +will also accurately note the user who made the change and allow for an optional submitted change reason to be stored with the change. .. image:: screens/1_poll_history.png @@ -34,7 +34,7 @@ An example of admin integration for the ``Poll`` and ``Choice`` models: admin.site.register(Poll, SimpleHistoryAdmin) admin.site.register(Choice, SimpleHistoryAdmin) -Changing a history-tracked model from the admin interface will automatically record the user who made the change (see :doc:`/user_tracking`). +Changing a history-tracked model from the admin interface will automatically record the user who made the change (see :doc:`/user_tracking`) and let the user submit an optional change reason to be stored alongside the change. Displaying custom columns in the admin history list view diff --git a/simple_history/admin.py b/simple_history/admin.py index 40941fd1e..83e4cd4ea 100644 --- a/simple_history/admin.py +++ b/simple_history/admin.py @@ -1,4 +1,4 @@ -from django import http +from django import forms, http from django.apps import apps as django_apps from django.conf import settings from django.contrib import admin @@ -20,6 +20,8 @@ SIMPLE_HISTORY_EDIT = getattr(settings, "SIMPLE_HISTORY_EDIT", False) +class HistoryChangeReasonForm(forms.ModelForm): + history_change_reason = forms.CharField(required=False) class SimpleHistoryAdmin(admin.ModelAdmin): object_history_template = "simple_history/object_history.html" @@ -225,9 +227,30 @@ def render_history_view(self, request, template, context, **kwargs): def save_model(self, request, obj, form, change): """Set special model attribute to user for reference after save""" + obj._change_reason = form.cleaned_data.get("history_change_reason") obj._history_user = request.user super().save_model(request, obj, form, change) + form = HistoryChangeReasonForm + + def get_fields(self, request, obj=None): + return [ + field + for field in super().get_fields(request, obj) + if field != "history_change_reason" + ] + + def get_fieldsets(self, request, obj=None): + return super().get_fieldsets(request, obj) + ([ + ( + "History", + { + "classes": ["collapse"], + "fields": ["history_change_reason"], + }, + ) + ] if 'history_change_reason' in self.form.declared_fields else []) + @property def content_type_model_cls(self): """Returns the ContentType model class.""" diff --git a/simple_history/tests/tests/test_admin.py b/simple_history/tests/tests/test_admin.py index ec99c29ef..ff6424425 100644 --- a/simple_history/tests/tests/test_admin.py +++ b/simple_history/tests/tests/test_admin.py @@ -221,6 +221,50 @@ def test_history_user_on_save_in_admin(self): [p.history_user for p in Poll.history.all()], [self.user, self.user] ) + def test_history_change_reason_on_save_in_admin(self): + self.login() + + # Ensure polls created via admin interface save correct change reason + change_reason = "New change reason" + initial_data = poll_data = { + "question": "new poll?", + "pub_date_0": "2012-01-01", + "pub_date_1": "10:00:00", + "history_change_reason": change_reason, + } + self.client.post(reverse("admin:tests_poll_add"), data=poll_data) + poll = Poll.objects.get() + self.assertEqual(poll.history.get().history_change_reason, change_reason) + + + # Ensure polls modified via admin interface save correct change reason + change_reason = "Edited change reason" + poll_data = { + "question": "new poll?", + "pub_date_0": "2011-01-01", + "pub_date_1": "10:00:00", + "history_change_reason": change_reason, + } + self.client.post(reverse("admin:tests_poll_change", args=[poll.id]), data=poll_data) + poll.refresh_from_db() + self.assertEqual(poll.pub_date.year, 2011) + self.assertEqual(poll.history.count(), 2) + self.assertEqual(poll.history.latest().history_change_reason, change_reason) + + # Let's emulate a revert + change_reason = "Revert to history record 0" + response = self.client.get(get_history_url(poll, 0)) + form = response.context.get("adminform").form + self.assertEqual(form["history_change_reason"].value(), None) # Always starts empty + self.assertEqual(form["pub_date"].value().year, 2012) + + self.client.post(get_history_url(poll, 0), data=initial_data | {"history_change_reason": change_reason}) + + poll.refresh_from_db() + self.assertEqual(poll.pub_date.year, 2012) + self.assertEqual(poll.history.count(), 3) + self.assertEqual(poll.history.latest().history_change_reason, change_reason) + def test_underscore_in_pk(self): self.login() book = Book(isbn="9780147_513731")