Fixed #35953 -- Added composite PK admin support.

django/contrib/admin/options.py

@@ -1828,7 +1828,7 @@ class ModelAdmin(BaseModelAdmin):
         """
         msg = _("%(name)s with ID “%(key)s” doesn’t exist. Perhaps it was deleted?") % {
             "name": opts.verbose_name,
-            "key": unquote(object_id),
+            "key": self.unquote(object_id),
         }
         self.message_user(request, msg, messages.WARNING)
         url = reverse("admin:index", current_app=self.admin_site.name)
@@ -1860,7 +1860,7 @@ class ModelAdmin(BaseModelAdmin):
             obj = None
 
         else:
-            obj = self.get_object(request, unquote(object_id), to_field)
+            obj = self.get_object(request, self.unquote(object_id), to_field)
 
             if request.method == "POST":
                 if not self.has_change_permission(request, obj):
@@ -2216,7 +2216,7 @@ class ModelAdmin(BaseModelAdmin):
                 "The field %s cannot be referenced." % to_field
             )
 
-        obj = self.get_object(request, unquote(object_id), to_field)
+        obj = self.get_object(request, self.unquote(object_id), to_field)
 
         if not self.has_delete_permission(request, obj):
             raise PermissionDenied
@@ -2278,7 +2278,7 @@ class ModelAdmin(BaseModelAdmin):
 
         # First check if the user can see this history.
         model = self.model
-        obj = self.get_object(request, unquote(object_id))
+        obj = self.get_object(request, self.unquote(object_id))
         if obj is None:
             return self._get_obj_does_not_exist_redirect(
                 request, model._meta, object_id
@@ -2291,7 +2291,7 @@ class ModelAdmin(BaseModelAdmin):
         app_label = self.opts.app_label
         action_list = (
             LogEntry.objects.filter(
-                object_id=unquote(object_id),
+                object_id=self.unquote(object_id),
                 content_type=get_content_type_for_model(model),
             )
             .select_related()
@@ -2382,6 +2382,9 @@ class ModelAdmin(BaseModelAdmin):
             inline_instances.append(inline)
         return formsets, inline_instances
 
+    def unquote(self, pk):
+        return unquote(pk, is_composite=self.opts.is_composite_pk)
+
 
 class InlineModelAdmin(BaseModelAdmin):
     """

django/contrib/admin/sites.py

@@ -113,11 +113,6 @@ class AdminSite:
                     "The model %s is abstract, so it cannot be registered with admin."
                     % model.__name__
                 )
-            if model._meta.is_composite_pk:
-                raise ImproperlyConfigured(
-                    "The model %s has a composite primary key, so it cannot be "
-                    "registered with admin." % model.__name__
-                )
 
             if self.is_registered(model):
                 registered_admin = str(self.get_model_admin(model))

django/contrib/admin/utils.py

@@ -23,6 +23,7 @@ from django.utils.translation import override as translation_override
 QUOTE_MAP = {i: "_%02X" % i for i in b'":/_#?;@&=+$,"[]<>%\n\\'}
 UNQUOTE_MAP = {v: chr(k) for k, v in QUOTE_MAP.items()}
 UNQUOTE_RE = _lazy_re_compile("_(?:%s)" % "|".join([x[1:] for x in UNQUOTE_MAP]))
+PK_SEP = ","
 
 
 class FieldIsAForeignKeyColumnName(Exception):
@@ -91,12 +92,20 @@ def quote(s):
     Similar to urllib.parse.quote(), except that the quoting is slightly
     different so that it doesn't get automatically unquoted by the web browser.
     """
-    return s.translate(QUOTE_MAP) if isinstance(s, str) else s
+    if isinstance(s, str):
+        return s.translate(QUOTE_MAP)
+    elif isinstance(s, tuple):
+        return PK_SEP.join(str(quote(f)) for f in s)
+    else:
+        return s
 
 
-def unquote(s):
+def unquote(s, is_composite=False):
     """Undo the effects of quote()."""
-    return UNQUOTE_RE.sub(lambda m: UNQUOTE_MAP[m[0]], s)
+    if is_composite:
+        return tuple(unquote(f) for f in s.split(PK_SEP))
+    else:
+        return UNQUOTE_RE.sub(lambda m: UNQUOTE_MAP[m[0]], s)
 
 
 def flatten(fields):

django/contrib/auth/admin.py

@@ -1,7 +1,6 @@
 from django.conf import settings
 from django.contrib import admin, messages
 from django.contrib.admin.options import IS_POPUP_VAR
-from django.contrib.admin.utils import unquote
 from django.contrib.auth import update_session_auth_hash
 from django.contrib.auth.forms import (
     AdminPasswordChangeForm,
@@ -153,7 +152,7 @@ class UserAdmin(admin.ModelAdmin):
 
     @sensitive_post_parameters_m
     def user_change_password(self, request, id, form_url=""):
-        user = self.get_object(request, unquote(id))
+        user = self.get_object(request, self.unquote(id))
         if not self.has_change_permission(request, user):
             raise PermissionDenied
         if user is None:

tests/admin_registration/tests.py

@@ -93,12 +93,8 @@ class TestRegistration(SimpleTestCase):
             self.site.register(Location)
 
     def test_composite_pk_model(self):
-        msg = (
+        self.site.register(Guest)
-            "The model Guest has a composite primary key, so it cannot be registered "
+        self.assertTrue(self.site.is_registered(Guest))
-            "with admin."
-        )
-        with self.assertRaisesMessage(ImproperlyConfigured, msg):
-            self.site.register(Guest)
 
     def test_is_registered_model(self):
         "Checks for registered models should return true."

tests/admin_utils/tests.py

@@ -16,6 +16,7 @@ from django.contrib.admin.utils import (
     label_for_field,
     lookup_field,
     quote,
+    unquote,
 )
 from django.core.validators import EMPTY_VALUES
 from django.db import DEFAULT_DB_ALIAS, models
@@ -436,7 +437,38 @@ class UtilsTests(SimpleTestCase):
         )
 
     def test_quote(self):
-        self.assertEqual(quote("something\nor\nother"), "something_0Aor_0Aother")
+        test_cases = (
+            ("something\nor\nother", "something_0Aor_0Aother"),
+            ("f,o,o", "f_2Co_2Co"),
+            ("b-a-r", "b-a-r"),
+            ((), ""),
+            ((1, 2), "1,2"),
+            ((3, "f,o,o"), "3,f_2Co_2Co"),
+            ((4, "b-a-r"), "4,b-a-r"),
+        )
+
+        for s, expected in test_cases:
+            with self.subTest(s=s, expected=expected):
+                self.assertEqual(quote(s), expected)
+
+    def test_unquote(self):
+        test_cases = (
+            ("something_0Aor_0Aother", False, "something\nor\nother"),
+            ("f_2Co_2Co", False, "f,o,o"),
+            ("b-a-r", False, "b-a-r"),
+            ("", False, ""),
+            ("", True, ("",)),
+            ("1,2,3", False, "1,2,3"),
+            ("1,2,3", True, ("1", "2", "3")),
+            ("3,f_2Co_2Co", False, "3,f,o,o"),
+            ("3,f_2Co_2Co", True, ("3", "f,o,o")),
+            ("4,b-a-r", False, "4,b-a-r"),
+            ("4,b-a-r", True, ("4", "b-a-r")),
+        )
+
+        for s, is_composite, expected in test_cases:
+            with self.subTest(s=s, is_composite=is_composite, expected=expected):
+                self.assertEqual(unquote(s, is_composite=is_composite), expected)
 
     def test_build_q_object_from_lookup_parameters(self):
         parameters = {

tests/admin_views/admin.py

@@ -46,6 +46,7 @@ from .models import (
     Color,
     Color2,
     ComplexSortedPerson,
+    CompositePKModel,
     Country,
     CoverLetter,
     CustomArticle,
@@ -1200,6 +1201,7 @@ site.register(
     search_fields=["name"],
 )
 site.register(ModelWithStringPrimaryKey)
+site.register(CompositePKModel)
 site.register(Color)
 site.register(Thing, ThingAdmin)
 site.register(Actor)

tests/admin_views/models.py

@@ -145,6 +145,12 @@ class ModelWithStringPrimaryKey(models.Model):
         return "/dummy/%s/" % self.string_pk
 
 
+class CompositePKModel(models.Model):
+    pk = models.CompositePrimaryKey("foo", "bar")
+    foo = models.CharField(max_length=10)
+    bar = models.CharField(max_length=10)
+
+
 class Color(models.Model):
     value = models.CharField(max_length=10)
     warm = models.BooleanField(default=False)

tests/admin_views/tests.py

@@ -72,6 +72,7 @@ from .models import (
     Collector,
     Color,
     ComplexSortedPerson,
+    CompositePKModel,
     CoverLetter,
     CustomArticle,
     CyclicOne,
@@ -4078,6 +4079,67 @@ class AdminViewStringPrimaryKeyTest(TestCase):
         self.assertIn("/123_2Fhistory/", response.headers["location"])  # PK is quoted
 
 
+@override_settings(ROOT_URLCONF="admin_views.urls")
+class AdminViewCompositePKTests(TestCase):
+    MODEL = "compositepkmodel"
+    CHANGE_VIEW = "admin:admin_views_%s_change"
+    HISTORY_VIEW = "admin:admin_views_%s_history"
+    DELETE_VIEW = "admin:admin_views_%s_delete"
+    CHANGELIST_VIEW = "admin:admin_views_%s_changelist"
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.obj = CompositePKModel.objects.create(foo="f,o,o", bar="b-a-r")
+        cls.ct = ContentType.objects.get_for_model(CompositePKModel)
+        cls.superuser = User.objects.create_superuser(
+            username="super", password="secret", email="super@example.com"
+        )
+
+    def setUp(self):
+        self.client.force_login(self.superuser)
+
+    def test_history_view(self):
+        viewname = self.HISTORY_VIEW % (self.MODEL,)
+        url = reverse(viewname, args=(quote(self.obj.pk),))
+        response = self.client.get(url)
+        self.assertContains(response, escape(self.obj.pk))
+
+    def test_history_view_redirects_if_does_not_exist(self):
+        viewname = self.HISTORY_VIEW % (self.MODEL,)
+        url = reverse(viewname, args=("1,2,3",))
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 302)
+
+    def test_change_view(self):
+        viewname = self.CHANGE_VIEW % (self.MODEL,)
+        url = reverse(viewname, args=(quote(self.obj.pk),))
+        response = self.client.get(url)
+        self.assertContains(response, escape(self.obj.pk))
+
+        response = self.client.post(url, {"foo": self.obj.foo, "bar": self.obj.bar})
+        self.assertRedirects(response, reverse(self.CHANGELIST_VIEW % (self.MODEL,)))
+        log_entry = LogEntry.objects.get(content_type=self.ct)
+        self.assertEqual(log_entry.object_id, '["f,o,o", "b,a,r"]')
+
+    def test_change_view_redirects_if_does_not_exist(self):
+        viewname = self.CHANGE_VIEW % (self.MODEL,)
+        url = reverse(viewname, args=("f,o,o",))
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 302)
+
+    def test_delete_view(self):
+        viewname = self.DELETE_VIEW % (self.MODEL,)
+        url = reverse(viewname, args=(quote(self.obj.pk),))
+        response = self.client.get(url)
+        self.assertContains(response, escape(self.obj.pk))
+
+    def test_delete_view_redirects_if_does_not_exist(self):
+        viewname = self.DELETE_VIEW % (self.MODEL,)
+        url = reverse(viewname, args=("1,2",))
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 302)
+
+
 @override_settings(ROOT_URLCONF="admin_views.urls")
 class SecureViewTests(TestCase):
     """