Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pagination and filtering #1220

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Unreleased
- Dropped support for Django 4.0, which reached end-of-life on 2023-04-01 (gh-1202)
- Added support for Django 4.2 (gh-1202)
- Made ``bulk_update_with_history()`` return the number of model rows updated (gh-1206)
- Added pagination to ``SimpleHistoryAdmin`` (gh-1220)
- Fixed ``HistoryRequestMiddleware`` not cleaning up after itself (i.e. deleting
``HistoricalRecords.context.request``) under some circumstances (gh-1188)
- Made ``HistoryRequestMiddleware`` async-capable (gh-1209)
Expand Down
9 changes: 8 additions & 1 deletion docs/admin.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ An example of admin integration for the ``Poll`` and ``Choice`` models:

Changing a history-tracked model from the admin interface will automatically record the user who made the change (see :doc:`/user_tracking`).

Changing the number of historical records shown in the admin history list view
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

By default, the history list view of ``SimpleHistoryAdmin`` shows the last 50 records.
You can change this by adding a `history__list_per_page` attribute to the admin class.


Displaying custom columns in the admin history list view
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand All @@ -49,7 +55,7 @@ By default, the history log displays one line per change containing

You can add other columns (for example the object's status to see
how it evolved) by adding a ``history_list_display`` array of fields to the
admin class
admin class.

.. code-block:: python

Expand All @@ -62,6 +68,7 @@ admin class
list_display = ["id", "name", "status"]
history_list_display = ["status"]
search_fields = ['name', 'user__username']
history__list_per_page = 100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not history_list_per_page with one dash as is the pattern with the other naming?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not exactly sure why you're not seeing the pagination because I just copied in what SimpleHistory as it is in my commits here into our own code and it seems to be working just fine with the pagination. I copied in both the object_history.html and SimpleHistoryAdmin class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I look at the files changed in this PR, I see no html file added or changed. And I guess the change as in the link I showed should be in _object_history_list.html for it to work.


admin.site.register(Poll, PollHistoryAdmin)
admin.site.register(Choice, SimpleHistoryAdmin)
Expand Down
9 changes: 7 additions & 2 deletions simple_history/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.contrib.admin.utils import unquote
from django.contrib.auth import get_permission_codename, get_user_model
from django.core.exceptions import PermissionDenied
from django.core.paginator import Paginator
from django.shortcuts import get_object_or_404, render
from django.urls import re_path, reverse
from django.utils.encoding import force_str
Expand All @@ -21,6 +22,7 @@
class SimpleHistoryAdmin(admin.ModelAdmin):
object_history_template = "simple_history/object_history.html"
object_history_form_template = "simple_history/object_history_form.html"
history__list_per_page = 50

def get_urls(self):
"""Returns the additional urls used by the Reversion admin."""
Expand Down Expand Up @@ -60,14 +62,17 @@ def history_view(self, request, object_id, extra_context=None):
except action_list.model.DoesNotExist:
raise http.Http404

paginator = Paginator(action_list, self.history__list_per_page)
action_list_page = paginator.get_page(request.GET.get("page"))

if not self.has_view_history_or_change_history_permission(request, obj):
raise PermissionDenied

# Set attribute on each action_list entry from admin methods
for history_list_entry in history_list_display:
value_for_entry = getattr(self, history_list_entry, None)
if value_for_entry and callable(value_for_entry):
for list_entry in action_list:
for list_entry in action_list_page.object_list:
setattr(list_entry, history_list_entry, value_for_entry(list_entry))

content_type = self.content_type_model_cls.objects.get_for_model(
Expand All @@ -80,7 +85,7 @@ def history_view(self, request, object_id, extra_context=None):
)
context = {
"title": self.history_view_title(request, obj),
"action_list": action_list,
"action_list": action_list_page,
RumitAP marked this conversation as resolved.
Show resolved Hide resolved
"module_name": capfirst(force_str(opts.verbose_name_plural)),
"object": obj,
"root_path": getattr(self.admin_site, "root_path", None),
Expand Down
38 changes: 38 additions & 0 deletions simple_history/tests/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,44 @@ def test_response_change(self):

self.assertEqual(response["Location"], "/awesome/url/")

def test_history_view_pagination(self):
"""
Ensure the history_view handles pagination correctly.
"""
# Create a Poll object and make more than 50 changes to ensure pagination
poll = Poll.objects.create(question="what?", pub_date=today)
for i in range(60):
poll.question = f"change_{i}"
poll.save()

# Verify that there are 60+1 (initial creation) historical records
self.assertEqual(poll.history.count(), 61)

admin_site = AdminSite()
admin = SimpleHistoryAdmin(Poll, admin_site)

self.login(superuser=True)

# Simulate a request to the second page
request = RequestFactory().get("/", {"page": "2"})
request.user = self.user

# Patch the render function
with patch("simple_history.admin.render") as mock_render:
admin.history_view(request, str(poll.id))

# Ensure the render function was called
self.assertTrue(mock_render.called)

# Extract context passed to render function
action_list_count = mock_render.call_args[0][2][
"action_list"
].object_list.count()

# Check if only 10 (61 - 50 from the first page)
# objects are present in the context
self.assertEqual(action_list_count, 11)

def test_response_change_change_history_setting_off(self):
"""
Test the response_change method that it works with a _change_history
Expand Down