Fix: Error in history view of object with forward slash in primary key

CHANGES.rst

@@ -16,6 +16,7 @@ Unreleased
   version is lower than 4.2 (gh-1261)
 - Small performance optimization of the ``clean-duplicate_history`` command (gh-1015)
 - Support Simplified Chinese translation (gh-1281)
+- Fixed error in history view of object with forward slash in primary key (gh-1295)
 
 3.4.0 (2023-08-18)
 ------------------

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

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):

simple_history/templates/simple_history/_object_history_list.html

@@ -19,7 +19,7 @@
   <tbody>
     {% for action in action_list %}
       <tr>
-        <td><a href="{% url opts|admin_urlname:'simple_history' object.pk action.pk %}">{{ action.history_object }}</a></td>
+        <td><a href="{% url opts|admin_urlname:'simple_history' object.pk|admin_urlquote action.pk %}">{{ action.history_object }}</a></td>
         {% for column in history_list_display %}
         <td scope="col">{{ action|getattribute:column }}</th>
         {% endfor %}

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,