Fixes string primary keys quotes

.gitignore

@@ -4,3 +4,4 @@ django_mass_edit.egg-info
 .idea
 .coverage
 .tox
+db.sqlite3

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+3.5.1 (unreleased)
+------------------
+
+* Fixed work with string primary keys that have special characters
+
 3.5.0 (22-08-2023)
 ------------------
 

massadmin/massadmin.py

@@ -33,6 +33,7 @@
 
 from django.contrib import admin
 from django.core.exceptions import PermissionDenied, ValidationError
+
 try:
     from django.urls import reverse
 except ImportError:  # Django<2.0
@@ -44,9 +45,9 @@
 except ImportError:
     from django.db.models import get_model
 try:
-    from django.contrib.admin.utils import unquote
+    from django.contrib.admin.utils import quote, unquote
 except ImportError:
-    from django.contrib.admin.util import unquote
+    from django.contrib.admin.util import quote, unquote
 from django.contrib.admin import helpers
 from django.utils.translation import gettext_lazy as _
 try:
@@ -77,7 +78,7 @@ def mass_change_selected(modeladmin, request, queryset):
 
 
 def get_mass_change_redirect_url(model_meta, pk_list, session):
-    object_ids = ",".join(str(s) for s in pk_list)
+    object_ids = ",".join(quote(str(s)) for s in pk_list)
     if len(object_ids) > settings.SESSION_BASED_URL_THRESHOLD:
         hash_id = "session-%s" % hashlib.md5(object_ids.encode('utf-8')).hexdigest()
         session[hash_id] = object_ids
@@ -219,11 +220,11 @@ def mass_change_view(
             "massadmin_queryset",
             self.admin_obj.get_queryset)(request)
 
-        object_ids = comma_separated_object_ids.split(',')
+        object_ids = [unquote(_id) for _id in comma_separated_object_ids.split(',')]
         object_id = object_ids[0]
 
         try:
-            obj = queryset.get(pk=unquote(object_id))
+            obj = queryset.get(pk=object_id)
         except model.DoesNotExist:
             obj = None
 

tests/admin.py

@@ -7,22 +7,20 @@
     CustomAdminModel2,
     InheritedAdminModel,
     FieldsetsAdminModel,
+    StringAdminModel,
 )
 
 
 class CustomAdminForm(forms.ModelForm):
-
     def clean_name(self):
-        """ Fake cleaning for tests
-        """
+        """Fake cleaning for tests"""
         name = self.cleaned_data.get("name")
-        if (self.instance.pk
-                and name == "invalid {}".format(self.instance.pk)):
+        if self.instance.pk and name == "invalid {}".format(self.instance.pk):
             raise forms.ValidationError("Invalid model name")
         return name
 
     class Meta:
-        fields = ("name", )
+        fields = ("name",)
         model = CustomAdminModel
 
 
@@ -31,7 +29,9 @@ class InheritedAdminInline(admin.TabularInline):
 
 
 class CustomAdmin(admin.ModelAdmin):
-    inlines = [InheritedAdminInline, ]
+    inlines = [
+        InheritedAdminInline,
+    ]
     model = CustomAdminModel
     form = CustomAdminForm
 
@@ -45,17 +45,21 @@ class CustomAdminWithCustomTemplate(admin.ModelAdmin):
     change_form_template = "admin/change_form_template.html"
 
 
+class CustomAdminWithStringPK(admin.ModelAdmin):
+    model = StringAdminModel
+
+
 class CustomAdminWithGetFieldsetsAncestor(admin.ModelAdmin):
     """
     Ancestor that defines fieldsets
     which should not be used by the mass_change_view()
     """
+
     model = FieldsetsAdminModel
     fieldsets = ()
 
 
 class CustomAdminWithGetFieldsets(CustomAdminWithGetFieldsetsAncestor):
-
     def get_fieldsets(self, request, obj=None):
         return (
             ("First part of name", {"fields": ("first_name",)}),
@@ -66,18 +70,19 @@ def get_fieldsets(self, request, obj=None):
 admin.site.register(CustomAdminModel, CustomAdmin)
 admin.site.register(CustomAdminModel2, CustomAdminWithCustomTemplate)
 admin.site.register(FieldsetsAdminModel, CustomAdminWithGetFieldsets)
+admin.site.register(StringAdminModel, CustomAdminWithStringPK)
 
 
 class BaseAdmin(admin.ModelAdmin):
-    readonly_fields = ("name", )
+    readonly_fields = ("name",)
 
 
 class InheritedAdmin(BaseAdmin):
     model = InheritedAdminModel
-    raw_id_fields = ("fk_field", )
+    raw_id_fields = ("fk_field",)
 
 
 admin.site.register(InheritedAdminModel, InheritedAdmin)
 
-custom_admin_site = admin.AdminSite(name='myadmin')
+custom_admin_site = admin.AdminSite(name="myadmin")
 custom_admin_site.register(CustomAdminModel, CustomAdmin)

tests/migrations/0003_fieldsetsadminmodel_alter_customadminmodel_id_and_more.py

@@ -0,0 +1,50 @@
+# Generated by Django 4.2.5 on 2023-09-19 09:17
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("tests", "0002_auto_20211217_0041"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="FieldsetsAdminModel",
+            fields=[
+                (
+                    "id",
+                    models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name="ID",
+                    ),
+                ),
+                ("first_name", models.CharField(max_length=32)),
+                ("middle_name", models.CharField(max_length=32)),
+                ("last_name", models.CharField(max_length=32)),
+            ],
+        ),
+        migrations.AlterField(
+            model_name="customadminmodel",
+            name="id",
+            field=models.AutoField(
+                auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+            ),
+        ),
+        migrations.AlterField(
+            model_name="customadminmodel2",
+            name="id",
+            field=models.AutoField(
+                auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+            ),
+        ),
+        migrations.AlterField(
+            model_name="inheritedadminmodel",
+            name="id",
+            field=models.AutoField(
+                auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
+            ),
+        ),
+    ]

tests/migrations/0004_add_model_with_string_primary_key.py

@@ -0,0 +1,22 @@
+# Generated by Django 4.2.5 on 2023-09-19 10:36
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("tests", "0003_fieldsetsadminmodel_alter_customadminmodel_id_and_more"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="StringAdminModel",
+            fields=[
+                (
+                    "primary",
+                    models.CharField(max_length=32, primary_key=True, serialize=False),
+                ),
+                ("name", models.CharField(max_length=32)),
+            ],
+        ),
+    ]

tests/models.py

@@ -9,6 +9,16 @@ class Meta:
         app_label = "tests"
 
 
+class StringAdminModel(models.Model):
+    """Special model for testing string primary keys."""
+
+    primary = models.CharField(max_length=32, primary_key=True)
+    name = models.CharField(max_length=32)
+
+    class Meta:
+        app_label = "tests"
+
+
 class CustomAdminModel2(models.Model):
     name = models.CharField(max_length=32)
 

tests/tests.py

@@ -2,6 +2,7 @@
 from django.contrib.auth.models import User
 from django.contrib import admin
 from django.test import TestCase, override_settings, RequestFactory
+
 try:
     from django.urls import reverse
 except ImportError:  # Django<2.0
@@ -14,6 +15,7 @@
     CustomAdminModel2,
     InheritedAdminModel,
     FieldsetsAdminModel,
+    StringAdminModel,
 )
 from .site import CustomAdminSite
 from .mocks import MockRenderMassAdmin
@@ -76,6 +78,20 @@ def test_update(self, admin_name='admin'):
         # all models have changed
         self.assertEqual(list(new_names), ["new name"] * 3)
 
+    def test_update_with_string_primary_key_and_special_chars(self, admin_name='admin'):
+        models = [StringAdminModel.objects.create(
+            primary="{}/3".format(1 + i),
+            name="model {}".format(i),
+        ) for i in range(0, 3)]
+
+        response = self.client.post(get_massadmin_url(models, self.client.session),
+                                    {"_mass_change": "name",
+                                     "name": "new name"})
+        self.assertRedirects(response, get_changelist_url(StringAdminModel, admin_name))
+        new_names = StringAdminModel.objects.order_by("pk").values_list("name", flat=True)
+        # all models have changed
+        self.assertEqual(list(new_names), ["new name"] * 3)
+
     @override_settings(ROOT_URLCONF='tests.urls_custom_admin')
     def test_custom_admin_site_update(self):
         """ We test_update for a custom admin on top of a regular as well
@@ -164,6 +180,7 @@ def test_excluded_queryset(self):
 class CustomizationTestCase(TestCase):
     """ MassAdmin has all customized options from related ModelAdmin
     """
+
     def setUp(self):
         self.user = User.objects.create_superuser(
             'temporary', '[email protected]', 'temporary')