diff --git a/.gitignore b/.gitignore index cbb74dd..94908f7 100755 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ django_mass_edit.egg-info .idea .coverage .tox +db.sqlite3 diff --git a/CHANGELOG.md b/CHANGELOG.md index 670a1b4..7398e35 100644 --- a/CHANGELOG.md +++ b/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) ------------------ diff --git a/massadmin/massadmin.py b/massadmin/massadmin.py index 4eb51db..e20a936 100644 --- a/massadmin/massadmin.py +++ b/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 diff --git a/tests/admin.py b/tests/admin.py index 79f58bb..2d71f67 100644 --- a/tests/admin.py +++ b/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) diff --git a/tests/migrations/0003_fieldsetsadminmodel_alter_customadminmodel_id_and_more.py b/tests/migrations/0003_fieldsetsadminmodel_alter_customadminmodel_id_and_more.py new file mode 100644 index 0000000..e5f5d32 --- /dev/null +++ b/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" + ), + ), + ] diff --git a/tests/migrations/0004_add_model_with_string_primary_key.py b/tests/migrations/0004_add_model_with_string_primary_key.py new file mode 100644 index 0000000..f5297f0 --- /dev/null +++ b/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)), + ], + ), + ] diff --git a/tests/models.py b/tests/models.py index eb8a154..373b803 100644 --- a/tests/models.py +++ b/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) diff --git a/tests/tests.py b/tests/tests.py index 88e0978..d615ace 100644 --- a/tests/tests.py +++ b/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', 'temporary@gmail.com', 'temporary')