From 52d9a590737e1739d4fa79c04efdfa899607fd14 Mon Sep 17 00:00:00 2001 From: Muneeb Shahid Date: Tue, 23 Jan 2024 22:33:17 -0800 Subject: [PATCH 1/2] fix(admin): Fix URLs and object ID handling in simple_history module - Used quote method to convert a primary key value into a string that can be used in a URL for history view - Used unquote method to convert quoted primary key back into its original form for history form view #1295 --- simple_history/admin.py | 1 + simple_history/models.py | 3 ++- .../simple_history/_object_history_list.html | 2 +- simple_history/tests/tests/test_admin.py | 18 +++++++++++++----- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/simple_history/admin.py b/simple_history/admin.py index 34e3033fe..7c5e74e58 100644 --- a/simple_history/admin.py +++ b/simple_history/admin.py @@ -126,6 +126,7 @@ def history_form_view(self, request, object_id, version_id, extra_context=None): model = getattr( self.model, self.model._meta.simple_history_manager_attribute ).model + object_id = unquote(str(object_id)) obj = get_object_or_404( model, **{original_opts.pk.attname: object_id, "history_id": version_id} ).instance diff --git a/simple_history/models.py b/simple_history/models.py index 6dc4db9e8..70a0461c7 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -7,6 +7,7 @@ from django.apps import apps from django.conf import settings from django.contrib import admin +from django.contrib.admin.utils import quote from django.contrib.auth import get_user_model from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist from django.db import models @@ -484,7 +485,7 @@ def revert_url(self): app_label, model_name = opts.app_label, opts.model_name return reverse( f"{admin.site.name}:{app_label}_{model_name}_simple_history", - args=[getattr(self, opts.pk.attname), self.history_id], + args=[quote(getattr(self, opts.pk.attname)), self.history_id], ) def get_instance(self): diff --git a/simple_history/templates/simple_history/_object_history_list.html b/simple_history/templates/simple_history/_object_history_list.html index 0dd575cc5..879e49433 100644 --- a/simple_history/templates/simple_history/_object_history_list.html +++ b/simple_history/templates/simple_history/_object_history_list.html @@ -19,7 +19,7 @@ {% for action in action_list %} - {{ action.history_object }} + {{ action.history_object }} {% for column in history_list_display %} {{ action|getattribute:column }} {% endfor %} diff --git a/simple_history/tests/tests/test_admin.py b/simple_history/tests/tests/test_admin.py index 2b441030b..9f6dbb2f1 100644 --- a/simple_history/tests/tests/test_admin.py +++ b/simple_history/tests/tests/test_admin.py @@ -226,6 +226,14 @@ def test_underscore_in_pk(self): response = self.client.get(get_history_url(book)) self.assertContains(response, book.history.all()[0].revert_url()) + def test_forward_slash_in_pk(self): + self.login() + book = Book(isbn="9780147/513731") + book._history_user = self.user + book.save() + response = self.client.get(get_history_url(book)) + self.assertContains(response, book.history.all()[0].revert_url()) + def test_historical_user_no_setter(self): """Demonstrate admin error without `_historical_user` setter. (Issue #43) @@ -458,7 +466,7 @@ def test_history_form_view_without_getting_history(self): "change_history": False, "title": "Revert %s" % force_str(poll), "adminform": ANY, - "object_id": poll.id, + "object_id": str(poll.id), "is_popup": False, "media": ANY, "errors": ANY, @@ -517,7 +525,7 @@ def test_history_form_view_getting_history(self): "change_history": True, "title": "Revert %s" % force_str(history.instance), "adminform": ANY, - "object_id": poll.id, + "object_id": str(poll.id), "is_popup": False, "media": ANY, "errors": ANY, @@ -576,7 +584,7 @@ def test_history_form_view_getting_history_with_setting_off(self): "change_history": False, "title": "Revert %s" % force_str(poll), "adminform": ANY, - "object_id": poll.id, + "object_id": str(poll.id), "is_popup": False, "media": ANY, "errors": ANY, @@ -635,7 +643,7 @@ def test_history_form_view_getting_history_abstract_external(self): "change_history": True, "title": "Revert %s" % force_str(history.instance), "adminform": ANY, - "object_id": obj.id, + "object_id": str(obj.id), "is_popup": False, "media": ANY, "errors": ANY, @@ -700,7 +708,7 @@ def test_history_form_view_accepts_additional_context(self): "change_history": False, "title": "Revert %s" % force_str(poll), "adminform": ANY, - "object_id": poll.id, + "object_id": str(poll.id), "is_popup": False, "media": ANY, "errors": ANY, From 043c43ec6d20ff80d8f3a42799b1862439df6a64 Mon Sep 17 00:00:00 2001 From: Muneeb Shahid Date: Tue, 23 Jan 2024 22:45:01 -0800 Subject: [PATCH 2/2] docs(Changes): Change log added #1295 --- CHANGES.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.rst b/CHANGES.rst index 0279c0dca..43acb7890 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -15,6 +15,7 @@ Unreleased - Added temporary requirement on ``asgiref>=3.6`` while the minimum required Django version is lower than 4.2 (gh-1261) - Small performance optimization of the ``clean-duplicate_history`` command (gh-1015) +- Fixed error in history view of object with forward slash in primary key (gh-1295) 3.4.0 (2023-08-18) ------------------