Skip to content

Commit

Permalink
Fix SimpleHistoryAdmin to consistently use AUTH_USER_MODEL in history…
Browse files Browse the repository at this point in the history
…_view
  • Loading branch information
epou committed Jan 25, 2024
1 parent f48b53d commit 6d15191
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 8 deletions.
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions simple_history/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 3 additions & 6 deletions simple_history/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
}

Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions simple_history/tests/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from .models import (
Book,
BucketData,
Choice,
ConcreteExternal,
Document,
Expand Down Expand Up @@ -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)
128 changes: 128 additions & 0 deletions simple_history/tests/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
23 changes: 23 additions & 0 deletions simple_history/tests/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", "[email protected]")
user2 = User.objects.create_user("user2", "[email protected]")
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 6d15191

Please sign in to comment.