Skip to content

Commit

Permalink
History view pagination (#1277)
Browse files Browse the repository at this point in the history
* Implement history_view pagination

Borrowed from Django's object_history.html

* Move pagination logic below the permission check

This avoids performing any database queries before we know that the user should be able to view data.

* Reworded the docs around changing the page size

A developer is more likely to look for the term page size.

---------
  • Loading branch information
jurrian authored Nov 25, 2024
1 parent 98462de commit 5dfeefe
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 6 deletions.
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ Authors
- Jordon Wing (`jordonwii <https://github.com/jordonwii>`_)
- Josh Fyne
- Josh Thomas (`joshuadavidthomas <https://github.com/joshuadavidthomas>`_)
- Jurrian Tromp (`jurrian <https://github.com/jurrian>`_)
- Keith Hackbarth
- Kevin Foster
- Kira (`kiraware <https://github.com/kiraware>`_)
Expand Down
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Unreleased
- Updated all djangoproject.com links to reference the stable version (gh-1420)
- Dropped support for Python 3.8, which reached end-of-life on 2024-10-07 (gh-1421)
- Added support for Django 5.1 (gh-1388)
- Added pagination to ``SimpleHistoryAdmin`` (gh-1277)

3.7.0 (2024-05-29)
------------------
Expand Down
24 changes: 23 additions & 1 deletion docs/admin.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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 +62,7 @@ admin class
list_display = ["id", "name", "status"]
history_list_display = ["status"]
search_fields = ['name', 'user__username']
history_list_per_page = 100
admin.site.register(Poll, PollHistoryAdmin)
admin.site.register(Choice, SimpleHistoryAdmin)
Expand All @@ -70,6 +71,27 @@ admin class
.. image:: screens/5_history_list_display.png


Changing the page size in the admin history list view
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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


.. code-block:: python
from django.contrib import admin
from simple_history.admin import SimpleHistoryAdmin
from .models import Poll
class PollHistoryAdmin(SimpleHistoryAdmin):
# history_list_per_page defaults to 100
history_list_per_page = 200
admin.site.register(Poll, PollHistoryAdmin)
Customizing the History Admin Templates
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
17 changes: 14 additions & 3 deletions simple_history/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
from django.contrib import admin
from django.contrib.admin import helpers
from django.contrib.admin.utils import unquote
from django.contrib.admin.views.main import PAGE_VAR
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.db.models import QuerySet
from django.shortcuts import get_object_or_404, render
from django.urls import re_path, reverse
Expand All @@ -31,6 +33,7 @@ class SimpleHistoryAdmin(admin.ModelAdmin):
object_history_template = "simple_history/object_history.html"
object_history_list_template = "simple_history/object_history_list.html"
object_history_form_template = "simple_history/object_history_form.html"
history_list_per_page = 100

def get_urls(self):
"""Returns the additional urls used by the Reversion admin."""
Expand Down Expand Up @@ -72,14 +75,19 @@ def history_view(self, request, object_id, extra_context=None):
if not self.has_view_history_or_change_history_permission(request, obj):
raise PermissionDenied

# Use the same pagination as in Django admin, with history_list_per_page items
paginator = Paginator(historical_records, self.history_list_per_page)
page_obj = paginator.get_page(request.GET.get(PAGE_VAR))
page_range = paginator.get_elided_page_range(page_obj.number)

# Set attribute on each historical record 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 record in historical_records:
for record in page_obj.object_list:
setattr(record, history_list_entry, value_for_entry(record))

self.set_history_delta_changes(request, historical_records)
self.set_history_delta_changes(request, page_obj)

content_type = self.content_type_model_cls.objects.get_for_model(
get_user_model()
Expand All @@ -92,7 +100,10 @@ def history_view(self, request, object_id, extra_context=None):
context = {
"title": self.history_view_title(request, obj),
"object_history_list_template": self.object_history_list_template,
"historical_records": historical_records,
"page_obj": page_obj,
"page_range": page_range,
"page_var": PAGE_VAR,
"pagination_required": paginator.count > self.history_list_per_page,
"module_name": capfirst(force_str(opts.verbose_name_plural)),
"object": obj,
"root_path": getattr(self.admin_site, "root_path", None),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
{% if not revert_disabled %}<p>
{% blocktrans %}Choose a date from the list below to revert to a previous version of this object.{% endblocktrans %}</p>{% endif %}
<div class="module">
{% if historical_records %}
{% if page_obj.object_list %}
{% include object_history_list_template %}
{% else %}
<p>{% trans "This object doesn't have a change history." %}</p>
Expand Down
17 changes: 16 additions & 1 deletion simple_history/templates/simple_history/object_history_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
</tr>
</thead>
<tbody>
{% for record in historical_records %}
{% for record in page_obj %}
<tr>
<td>
<a href="{% url opts|admin_urlname:'simple_history' object.pk record.pk %}">
Expand Down Expand Up @@ -65,3 +65,18 @@
{% endfor %}
</tbody>
</table>

<p class="paginator" style="border-top: 0">
{% if pagination_required %}
{% for i in page_range %}
{% if i == page_obj.paginator.ELLIPSIS %}
{{ page_obj.paginator.ELLIPSIS }}
{% elif i == page_obj.number %}
<span class="this-page">{{ i }}</span>
{% else %}
<a href="?{{ page_var }}={{ i }}" {% if i == page_obj.paginator.num_pages %} class="end" {% endif %}>{{ i }}</a>
{% endif %}
{% endfor %}
{% endif %}
{{ page_obj.paginator.count }} {% blocktranslate count counter=page_obj.paginator.count %}entry{% plural %}entries{% endblocktranslate %}
</p>
120 changes: 120 additions & 0 deletions simple_history/tests/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import django
from django.contrib.admin import AdminSite
from django.contrib.admin.utils import quote
from django.contrib.admin.views.main import PAGE_VAR
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Permission
from django.contrib.messages.storage.fallback import FallbackStorage
Expand Down Expand Up @@ -556,6 +557,125 @@ def test_response_change(self):

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

def test_history_view_pagination(self):
"""
Ensure the history_view handles pagination correctly.
The default history_list_per_page is 100 so page 2 should have 1 record.
"""
# Create a Poll object and make more than 100 changes to ensure pagination
poll = Poll.objects.create(question="what?", pub_date=today)
for i in range(100):
poll.question = f"change_{i}"
poll.save()

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

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

self.login(superuser=True)

# Simulate a request to the second page
request = RequestFactory().get("/", {PAGE_VAR: "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 = len(mock_render.call_args[0][2]["page_obj"].object_list)

# Check if only 1 (101 - 100 from the first page)
# objects are present in the context
self.assertEqual(action_list_count, 1)

def test_history_view_pagination_no_pagination(self):
"""
When all records fit on one page because the history_list_per_page is
higher than the number of records, ensure that the pagination is not set.
But it should show the number of entries.
"""
# 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)

# Create an admin with more per page than the number of records
class CustomSimpleHistoryAdmin(SimpleHistoryAdmin):
history_list_per_page = 200

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

self.login(superuser=True)

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

response = admin.history_view(request, str(poll.id))

expected = '<p class="paginator" style="border-top: 0">61 entries</p>'
self.assertInHTML(expected, response.content.decode())

def test_history_view_pagination_last_page(self):
"""
With 31 records, the last page should have 1 record. Non-existing pages
also end up on the last page.
"""
# Create a Poll object and make more than 30 changes to ensure pagination
poll = Poll.objects.create(question="what?", pub_date=today)
for i in range(30):
poll.question = f"change_{i}"
poll.save()

expected_entry_count = 31

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

# Create an admin with less per page than the number of records
class CustomSimpleHistoryAdmin(SimpleHistoryAdmin):
history_list_per_page = 10

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

self.login(superuser=True)

# Simulate a request to the 4th and last page
request = RequestFactory().get("/", {PAGE_VAR: "4"})
request.user = self.user

response = admin.history_view(request, str(poll.id))

expected = (
'<p class="paginator" style="border-top: 0">'
'<a href="?p=1" >1</a>'
'<a href="?p=2" >2</a>'
'<a href="?p=3" >3</a>'
'<span class="this-page">4</span>'
f"{expected_entry_count} entries"
"</p>"
)
self.assertInHTML(expected, response.content.decode())

# Also a non-existent page should return the last page
request = RequestFactory().get("/", {PAGE_VAR: "5"})
request.user = self.user

response = admin.history_view(request, str(poll.id))
self.assertInHTML(expected, response.content.decode())

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

0 comments on commit 5dfeefe

Please sign in to comment.