[Django] #35953: Add composite PK admin support

18 views
Skip to first unread message

Django

unread,
Nov 29, 2024, 9:07:52 AM11/29/24
to django-...@googlegroups.com
#35953: Add composite PK admin support
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Type: New
| feature
Status: new | Component:
| contrib.admin
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
This is a follow up to #373 (''CompositePrimaryKey'').

My proposal is to separate the primary key fields with a comma e.g.
`/admin/posts/post/foo,bar/change`.

The comma is not escaped when used as a separator, but it is escaped
(`%2C`) when part of the primary key's value.
e.g. `pk = ("The,quick", "brown")` ->
`/admin/posts/post/The%2Cquick,brown/change`

According to [https://datatracker.ietf.org/doc/html/rfc3986 RFC 3986],
unesacped commas are allowed in URI paths (as far as I understand).

What if an existing, non-composite primary key contains a comma?
''While Django URL encodes commas in the admin URLs (see `QUOTE_MAP`), it
must be able to look up objects by not-encoded PKs too.
The solution is simple. We must check if the model the user is trying
access has a composite primary key or not.''

Since admin's `LogEntry` model uses ''"generic foreign keys"'', this
ticket depends on #35941 (''composite GenericForeignKeys'').
--
Ticket URL: <https://code.djangoproject.com/ticket/35953>
Django <https://code.djangoproject.com/>
The web framework for perfectionists with deadlines.

Django

unread,
Nov 29, 2024, 10:27:09 AM11/29/24
to django-...@googlegroups.com
#35953: Add composite PK admin support
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* has_patch: 0 => 1
* needs_better_patch: 0 => 1
* owner: (none) => Csirmaz Bendegúz
* stage: Unreviewed => Accepted
* status: new => assigned

Comment:

PR: https://github.com/django/django/pull/18865

Marking as "Needs improvement" due to the dependency on #35941
--
Ticket URL: <https://code.djangoproject.com/ticket/35953#comment:1>

Django

unread,
Nov 29, 2024, 10:35:57 AM11/29/24
to django-...@googlegroups.com
#35953: Add composite PK admin support
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Old description:

> This is a follow up to #373 (''CompositePrimaryKey'').
>
> My proposal is to separate the primary key fields with a comma e.g.
> `/admin/posts/post/foo,bar/change`.
>
> The comma is not escaped when used as a separator, but it is escaped
> (`%2C`) when part of the primary key's value.
> e.g. `pk = ("The,quick", "brown")` ->
> `/admin/posts/post/The%2Cquick,brown/change`
>
> According to [https://datatracker.ietf.org/doc/html/rfc3986 RFC 3986],
> unesacped commas are allowed in URI paths (as far as I understand).
>
> What if an existing, non-composite primary key contains a comma?
> ''While Django URL encodes commas in the admin URLs (see `QUOTE_MAP`), it
> must be able to look up objects by not-encoded PKs too.
> The solution is simple. We must check if the model the user is trying
> access has a composite primary key or not.''
>
> Since admin's `LogEntry` model uses ''"generic foreign keys"'', this
> ticket depends on #35941 (''composite GenericForeignKeys'').

New description:

This is a follow up to #373 (''CompositePrimaryKey'').

My proposal is to separate the primary key fields with a comma e.g.
`/admin/posts/post/foo,bar/change`.

The comma is not escaped when used as a separator, but it is escaped
(`%2C`) when part of the primary key's value.
e.g. `pk = ("The,quick", "brown")` ->
`/admin/posts/post/The%2Cquick,brown/change`

According to [https://datatracker.ietf.org/doc/html/rfc3986 RFC 3986],
unesacped commas are allowed in URI paths (as far as I understand).

What if an existing, non-composite primary key contains a comma?
''While Django URL encodes commas in the admin URLs (see `QUOTE_MAP`), it
must be able to look up objects by non-encoded PKs too (see #12349,
#18550).
The solution is simple. We must check if the model the user is trying
access has a composite primary key or not.''

Since admin's `LogEntry` model uses ''"generic foreign keys"'', this
ticket depends on #35941 (''composite GenericForeignKeys'').

--
Comment (by Csirmaz Bendegúz):

Yes that's correct, thanks Sarah
--
Ticket URL: <https://code.djangoproject.com/ticket/35953#comment:2>

Django

unread,
Jan 13, 2025, 6:24:40 PMJan 13
to django-...@googlegroups.com
#35953: Add composite PK admin support
-------------------------------------+-------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Csirmaz
| Bendegúz
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Antoliny):

* cc: Antoliny (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/35953#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 16, 2025, 3:51:29 AMMar 16
to django-...@googlegroups.com
#35953: Add composite PK admin support
----------------------------------+------------------------------------
Reporter: Csirmaz Bendegúz | Owner: (none)
Type: New feature | Status: new
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by Csirmaz Bendegúz):

* owner: Csirmaz Bendegúz => (none)
* status: assigned => new

Comment:

I don't have the time to work on this right now
--
Ticket URL: <https://code.djangoproject.com/ticket/35953#comment:4>

Django

unread,
Mar 16, 2025, 3:55:21 AMMar 16
to django-...@googlegroups.com
#35953: Add composite PK admin support
----------------------------------+------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Antoliny
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Changes (by Antoliny):

* owner: (none) => Antoliny
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35953#comment:5>

Django

unread,
Apr 24, 2025, 10:31:36 AMApr 24
to django-...@googlegroups.com
#35953: Add composite PK admin support
----------------------------------+------------------------------------
Reporter: Csirmaz Bendegúz | Owner: Antoliny
Type: New feature | Status: assigned
Component: contrib.admin | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by Natalia Bidart):

#36351 was a dupe.
--
Ticket URL: <https://code.djangoproject.com/ticket/35953#comment:6>
Reply all
Reply to author
Forward
0 new messages