From 6d151919368269f2867da844076ca9e2e8a53f54 Mon Sep 17 00:00:00 2001
From: Enric Pou <enricpou@gmail.com>
Date: Thu, 25 Jan 2024 13:32:43 +0100
Subject: [PATCH] Fix SimpleHistoryAdmin to consistently use AUTH_USER_MODEL in
 history_view

---
 AUTHORS.rst                               |   1 +
 simple_history/admin.py                   |   4 +-
 simple_history/models.py                  |   9 +-
 simple_history/tests/admin.py             |   2 +
 simple_history/tests/tests/test_admin.py  | 128 ++++++++++++++++++++++
 simple_history/tests/tests/test_models.py |  23 ++++
 6 files changed, 159 insertions(+), 8 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index fcf285f71..b865d19fc 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -43,6 +43,7 @@ Authors
 - Dmytro Shyshov (`xahgmah <https://github.com/xahgmah>`_)
 - Edouard Richard (`vied12 <https://github.com/vied12>` _)
 - Eduardo Cuducos
+- Enric Pou (`epou <https://github.com/epou>`_)
 - Erik van Widenfelt (`erikvw <https://github.com/erikvw>`_)
 - Fábio Capuano (`fabiocapsouza <https://github.com/fabiocapsouza`_)
 - Filipe Pina (@fopina)
diff --git a/simple_history/admin.py b/simple_history/admin.py
index 34e3033fe..2c15961e7 100644
--- a/simple_history/admin.py
+++ b/simple_history/admin.py
@@ -4,7 +4,7 @@
 from django.contrib import admin
 from django.contrib.admin import helpers
 from django.contrib.admin.utils import unquote
-from django.contrib.auth import get_permission_codename, get_user_model
+from django.contrib.auth import get_permission_codename
 from django.core.exceptions import PermissionDenied
 from django.shortcuts import get_object_or_404, render
 from django.urls import re_path, reverse
@@ -71,7 +71,7 @@ def history_view(self, request, object_id, extra_context=None):
                     setattr(list_entry, history_list_entry, value_for_entry(list_entry))
 
         content_type = self.content_type_model_cls.objects.get_for_model(
-            get_user_model()
+            history.model._history_user_model
         )
 
         admin_user_view = "admin:{}_{}_change".format(
diff --git a/simple_history/models.py b/simple_history/models.py
index 6dc4db9e8..f59f1529c 100644
--- a/simple_history/models.py
+++ b/simple_history/models.py
@@ -112,7 +112,7 @@ def __init__(
         self.inherit = inherit
         self.history_id_field = history_id_field
         self.history_change_reason_field = history_change_reason_field
-        self.user_model = user_model
+        self.user_model = user_model or get_user_model()
         self.get_user = get_user
         self.cascade_delete_history = cascade_delete_history
         self.custom_model_name = custom_model_name
@@ -276,6 +276,7 @@ def create_history_model(self, model, inherited):
             "__module__": self.module,
             "_history_excluded_fields": self.excluded_fields,
             "_history_m2m_fields": self.get_m2m_fields_from_model(model),
+            "_history_user_model": self.user_model,
             "tracked_fields": self.fields_included(model),
         }
 
@@ -423,13 +424,9 @@ def _get_history_user_fields(self):
                 "history_user_id": self.user_id_field,
             }
         else:
-            user_model = self.user_model or getattr(
-                settings, "AUTH_USER_MODEL", "auth.User"
-            )
-
             history_user_fields = {
                 "history_user": models.ForeignKey(
-                    user_model,
+                    self.user_model,
                     null=True,
                     related_name=self.user_related_name,
                     on_delete=models.SET_NULL,
diff --git a/simple_history/tests/admin.py b/simple_history/tests/admin.py
index 24cf5f7ac..e064c7a4c 100644
--- a/simple_history/tests/admin.py
+++ b/simple_history/tests/admin.py
@@ -5,6 +5,7 @@
 
 from .models import (
     Book,
+    BucketData,
     Choice,
     ConcreteExternal,
     Document,
@@ -54,3 +55,4 @@ def test_method(self, obj):
 admin.site.register(ExternalModelWithCustomUserIdField, SimpleHistoryAdmin)
 admin.site.register(FileModel, FileModelAdmin)
 admin.site.register(Planet, PlanetAdmin)
+admin.site.register(BucketData, SimpleHistoryAdmin)
diff --git a/simple_history/tests/tests/test_admin.py b/simple_history/tests/tests/test_admin.py
index 2b441030b..4b707706e 100644
--- a/simple_history/tests/tests/test_admin.py
+++ b/simple_history/tests/tests/test_admin.py
@@ -772,6 +772,134 @@ def test_history_view__title_suggests_view_only(self):
             title_prefix=PermissionAction.VIEW, choose_date=False
         )
 
+    def test_history_view_sets_admin_user_view(self):
+        poll = Poll.objects.create(question="why?", pub_date=today)
+        poll.question = "how?"
+        poll.save()
+
+        request = RequestFactory().get(get_history_url(poll))
+        request.session = "session"
+        request._messages = FallbackStorage(request)
+        request.user = self.user
+
+        admin_site = AdminSite()
+        admin = SimpleHistoryAdmin(Poll, admin_site)
+
+        with patch("simple_history.admin.render") as mock_render:
+            admin.history_view(request, str(poll.id))
+
+        user_model = get_user_model()
+        admin_user_view = "admin:{}_{}_change".format(
+            user_model._meta.app_label,
+            user_model._meta.model_name
+        )
+        context = {
+            "title": admin.history_view_title(request, poll),
+            "action_list": ANY,
+            "module_name": "Polls",
+            "object": poll,
+            "root_path": getattr(admin_site, "root_path", None),
+            "app_label": "tests",
+            "opts": ANY,
+            "admin_user_view": admin_user_view,
+            "history_list_display": getattr(admin_site, "history_list_display", []),
+            "revert_disabled": admin.revert_disabled(request, poll),
+            **admin_site.each_context(request),
+        }
+
+        # This key didn't exist prior to Django 4.2
+        if "log_entries" in context:
+            context["log_entries"] = ANY
+
+        mock_render.assert_called_once_with(
+            request, admin.object_history_template, context
+        )
+
+    def test_history_view_sets_admin_user_view_on_user_model_override(self):
+        member = BucketMember.objects.create(name="member1", user=self.user)
+        bucket_data = BucketData(changed_by=member)
+        bucket_data.save()
+
+        request = RequestFactory().get(get_history_url(bucket_data))
+        request.session = "session"
+        request._messages = FallbackStorage(request)
+        request.user = self.user
+
+        admin_site = AdminSite()
+        admin = SimpleHistoryAdmin(BucketData, admin_site)
+
+        with patch("simple_history.admin.render") as mock_render:
+            admin.history_view(request, str(bucket_data.id))
+
+        user_model = BucketMember
+        admin_user_view = "admin:{}_{}_change".format(
+            user_model._meta.app_label,
+            user_model._meta.model_name
+        )
+        context = {
+            "title": admin.history_view_title(request, bucket_data),
+            "action_list": ANY,
+            "module_name": "Bucket datas",
+            "object": bucket_data,
+            "root_path": getattr(admin_site, "root_path", None),
+            "app_label": "tests",
+            "opts": ANY,
+            "admin_user_view": admin_user_view,
+            "history_list_display": getattr(admin_site, "history_list_display", []),
+            "revert_disabled": admin.revert_disabled(request, bucket_data),
+            **admin_site.each_context(request),
+        }
+
+        # This key didn't exist prior to Django 4.2
+        if "log_entries" in context:
+            context["log_entries"] = ANY
+
+        mock_render.assert_called_once_with(
+            request, admin.object_history_template, context
+        )
+
+    def test_history_view_sets_admin_user_view_on_custom_user_id_field(self):
+        instance = ExternalModelWithCustomUserIdField(name="random_name")
+        instance.save()
+
+        request = RequestFactory().get(get_history_url(instance))
+        request.session = "session"
+        request._messages = FallbackStorage(request)
+        request.user = self.user
+
+        admin_site = AdminSite()
+        admin = SimpleHistoryAdmin(ExternalModelWithCustomUserIdField, admin_site)
+
+        with patch("simple_history.admin.render") as mock_render:
+            admin.history_view(request, str(instance.id))
+
+        user_model = get_user_model()
+        admin_user_view = "admin:{}_{}_change".format(
+            user_model._meta.app_label,
+            user_model._meta.model_name
+        )
+        context = {
+            "title": admin.history_view_title(request, instance),
+            "action_list": ANY,
+            "module_name": "External model with custom user id fields",
+            "object": instance,
+            "root_path": getattr(admin_site, "root_path", None),
+            "app_label": "external",
+            "opts": ANY,
+            "admin_user_view": admin_user_view,
+            "history_list_display": getattr(admin_site, "history_list_display", []),
+            "revert_disabled": admin.revert_disabled(request, instance),
+            **admin_site.each_context(request),
+        }
+
+        # This key didn't exist prior to Django 4.2
+        if "log_entries" in context:
+            context["log_entries"] = ANY
+
+        mock_render.assert_called_once_with(
+            request, admin.object_history_template, context
+        )
+
     def test_history_form_view__shows_revert_button_by_default(self):
         self.login()
         planet = Planet.objects.create(star="Sun")
diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py
index 484df73f9..5d2ad4681 100644
--- a/simple_history/tests/tests/test_models.py
+++ b/simple_history/tests/tests/test_models.py
@@ -598,6 +598,24 @@ def test_model_with_excluded_fields(self):
         self.assertEqual(most_recent.question, p.question)
         self.assertEqual(most_recent.place, p.place)
 
+    def test_history_user_model_is_auth_user_as_default(self):
+        self.assertEqual(
+            get_user_model(),
+            get_history_model_for_model(Poll)._history_user_model
+        )
+
+    def test_history_user_model_is_user_model_on_override(self):
+        self.assertEqual(
+            BucketMember,
+            get_history_model_for_model(BucketData)._history_user_model
+        )
+
+    def test_history_user_model_is_user_model_on_override_register(self):
+        self.assertEqual(
+            BucketMember,
+            get_history_model_for_model(BucketDataRegisterChangedBy)._history_user_model
+        )
+
     def test_user_model_override(self):
         user1 = User.objects.create_user("user1", "1@example.com")
         user2 = User.objects.create_user("user2", "1@example.com")
@@ -2292,6 +2310,11 @@ def test_history_user_does_not_exist(self):
         self.assertEqual(user_id, instance.history.first().history_user_id)
         self.assertIsNone(instance.history.first().history_user)
 
+    def test_history_user_model_is_auth_user(self):
+        self.assertEqual(
+            get_user_model(),
+            get_history_model_for_model(ExternalModelWithCustomUserIdField)._history_user_model
+        )
 
 class RelatedNameTest(TestCase):
     def setUp(self):