fix: Objects with special symbols in primary key 404-ed (#110)

for case if object in database has any of special symbols
https://github.com/django/django/blob/master/django/contrib/admin/utils.py#L17
clicking on action button causes 404 error, as in SingleObjectMixin there are already parsed kwargs from url, and they are unquoted

made unquoting kwargs

Author: ilyachch

.gitignore

@@ -5,7 +5,8 @@ __pycache__
 .tox/
 *.db
 *.sqlite
-
+.venv
+.idea
 
 # Private files
 .env

django_object_actions/tests/test_admin.py

@@ -1,6 +1,7 @@
 """
 Integration tests that actually try and use the tools setup in admin.py
 """
+from django.contrib.admin.utils import quote
 from django.http import HttpResponse
 from mock import patch
 
@@ -10,7 +11,11 @@
     from django.core.urlresolvers import reverse  # < DJANGO1.10
 
 from .tests import LoggedInTestCase
-from example_project.polls.factories import CommentFactory, PollFactory
+from example_project.polls.factories import (
+    CommentFactory,
+    PollFactory,
+    RelatedDataFactory,
+)
 
 
 class CommentTests(LoggedInTestCase):
@@ -44,6 +49,21 @@ def test_action_on_a_model_with_slash_in_pk_works(self, mock_view):
         self.assertEqual(mock_view.call_args[1]["pk"], "pk/slash")
 
 
+class ExtraTests(LoggedInTestCase):
+    def test_action_on_a_model_with_complex_id(self):
+        related_data = RelatedDataFactory()
+        related_data_url = reverse(
+            "admin:polls_relateddata_change", args=(related_data.pk,)
+        )
+        action_url = "/admin/polls/relateddata/{}/actions/fill_up/".format(
+            quote(related_data.pk)
+        )
+
+        response = self.client.get(action_url)
+        self.assertNotEqual(response.status_code, 404)
+        self.assertRedirects(response, related_data_url)
+
+
 class ChangeTests(LoggedInTestCase):
     def test_buttons_load(self):
         url = "/admin/polls/choice/"

django_object_actions/utils.py

@@ -3,6 +3,7 @@
 
 from django.conf.urls import url
 from django.contrib import messages
+from django.contrib.admin.utils import unquote
 from django.db.models.query import QuerySet
 from django.http import Http404, HttpResponseRedirect
 from django.http.response import HttpResponseBase
@@ -243,6 +244,10 @@ def back_url(self):
         raise NotImplementedError
 
     def get(self, request, tool, **kwargs):
+        # Fix for case if there are special symbols in object pk
+        for k, v in self.kwargs.items():
+            self.kwargs[k] = unquote(v)
+
         try:
             view = self.actions[tool]
         except KeyError:

example_project/polls/admin.py

@@ -10,7 +10,7 @@
 
 from django_object_actions import DjangoObjectActions, takes_instance_or_queryset
 
-from .models import Choice, Poll, Comment
+from .models import Choice, Poll, Comment, RelatedData
 
 
 class ChoiceAdmin(DjangoObjectActions, admin.ModelAdmin):
@@ -160,5 +160,24 @@ def hodor(self, request, obj):
 admin.site.register(Comment, CommentAdmin)
 
 
+class RelatedDataAdmin(DjangoObjectActions, admin.ModelAdmin):
+
+    # Object actions
+    ################
+
+    def fill_up(self, request, obj):
+        if not obj.extra_data:
+            # bail because we need a comment
+            obj.extra_data = "hodor"
+        else:
+            obj.extra_data = ""
+        obj.save()
+
+    change_actions = ("fill_up",)
+
+
+admin.site.register(RelatedData, RelatedDataAdmin)
+
+
 support_admin = AdminSite(name="support")
 support_admin.register(Poll, PollAdmin)

example_project/polls/factories.py

@@ -1,3 +1,6 @@
+import random
+import string
+
 import factory
 from django.contrib.auth import get_user_model
 from django.utils import timezone
@@ -36,3 +39,23 @@ class Meta:
 class CommentFactory(factory.DjangoModelFactory):
     class Meta:
         model = models.Comment
+
+
+def get_random_string(length):
+    letters = string.ascii_lowercase
+    result_str = "".join(random.choice(letters) for i in range(length))
+    return result_str
+
+
+class RelatedDataFactory(factory.DjangoModelFactory):
+    id = factory.lazy_attribute(
+        lambda __: "{}:{}-{}!{}".format(
+            get_random_string(2),
+            get_random_string(2),
+            get_random_string(2),
+            get_random_string(2),
+        )
+    )
+
+    class Meta:
+        model = models.RelatedData

example_project/polls/migrations/0002_auto_20200805_0239.py

@@ -0,0 +1,36 @@
+# Generated by Django 3.1 on 2020-08-05 02:39
+
+from django.db import migrations, models
+import uuid
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("polls", "0001_initial"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="RelatedData",
+            fields=[
+                (
+                    "id",
+                    models.CharField(max_length=32, primary_key=True, serialize=False),
+                ),
+                ("extra_data", models.TextField(blank=True, default="")),
+            ],
+        ),
+        migrations.AlterField(
+            model_name="comment",
+            name="uuid",
+            field=models.UUIDField(
+                default=uuid.uuid4, editable=False, primary_key=True, serialize=False
+            ),
+        ),
+        migrations.AlterField(
+            model_name="poll",
+            name="pub_date",
+            field=models.DateTimeField(verbose_name="date published"),
+        ),
+    ]

example_project/polls/models.py

@@ -35,3 +35,11 @@ class Comment(models.Model):
 
     def __str__(self):
         return self.comment or ""
+
+
+class RelatedData(models.Model):
+    id = models.CharField(primary_key=True, max_length=32)
+    extra_data = models.TextField(blank=True, default="")
+
+    def __str__(self):
+        return self.extra_data or self.id