Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#33077 closed Bug (fixed)

Wrong URL generated by get_admin_url for readonly field in custom Admin Site

Reported by: Ken Whitesell Owned by: Ken Whitesell
Component: contrib.admin Version: 3.2
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When a model containing a ForeignKey field is viewed (or edited) in a custom Admin Site, and that ForeignKey field is listed in readonly_fields, the url generated for the link is /admin/... instead of /custom-admin/....

This appears to be caused by the following line in django.contrib.admin.helpers get_admin_url:
url = reverse(url_name, args=[quote(remote_obj.pk)])

Other parts of the admin use the current_app keyword parameter to identify the correct current name of the Admin Site. (See django.contrib.admin.options.ModelAdmin response_add as just one example)

I have been able to correct this specific issue by replacing the above line with:

url = reverse(
    url_name,
    args=[quote(remote_obj.pk)],
    current_app=self.model_admin.admin_site.name
)

However, I don't know if there are any side effects and I have not yet run the full suite of tests on this. Mostly looking for feedback whether I'm on the right track.

Change History (14)

comment:1 Changed 10 months ago by Carlton Gibson

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Hey Ken, yes seems right. Good spot.

Looks like this should have been part of b79088306513d5ed76d31ac40ab3c15f858946ea for #31181 (which was Django 3.2) here.

However, I don't know if there are any side effects and I have not yet run the full suite of tests on this. Mostly looking for feedback whether I'm on the right track.

I ran your suggestion against most of the usual suspects admin_* tests without issue so...

Would you like to prepare a patch? Looks like setting up the test case is the most of it...

Thanks!

comment:2 Changed 10 months ago by Ken Whitesell

I'll be happy to try - but I'm not likely to be able to get to it before the weekend. (I don't know how "urgent" you consider it.) If it can sit that long, I'll see what I can do. (First "real patch" and all that - want to make sure I do it reasonably right.)

comment:3 Changed 10 months ago by Carlton Gibson

Owner: changed from nobody to Ken Whitesell
Status: newassigned

Hey Ken. Super thanks!

Since it's a bug in a new feature it's marked as release blocker and will be backported to Django 3.2.
We'll target 3.2.8, which is slated for the beginning of October.
If it gets close to that and you've not had time we can pick it up.

Reach out on the Forum if you'd like input at all. 🙂

Thanks! (And Welcome Aboard! ⛵️)

comment:4 Changed 10 months ago by Abhijith Ganesh

Owner: changed from Ken Whitesell to Abhijith Ganesh

comment:5 Changed 10 months ago by Abhijith Ganesh

Heyy folks, I wanted to assign the ticket to myself and fix the issue, instead it assigned the ownership to me. Apologies

comment:6 Changed 10 months ago by Abhijith Ganesh

Owner: changed from Abhijith Ganesh to Ken Whitesell

Changes ownership again.

comment:7 Changed 10 months ago by Abhijith Ganesh

I found out that changes got accepted, sorry for the inconvenience caused.

Last edited 10 months ago by Abhijith Ganesh (previous) (diff)

comment:8 Changed 10 months ago by Carlton Gibson

Hi Abhijith — just to confirm, according to the discussion Ken is currently working on this ticket, so let's give him a window to do that before re-assigning it. Thanks! (I think that's the conclusion you came to, but just double-checking so you don't both work on the same ticket at the same time.)

comment:9 Changed 9 months ago by Mariusz Felisiak

Has patch: set
Patch needs improvement: set

comment:10 Changed 9 months ago by Ken Whitesell

Owner: Ken Whitesell deleted
Status: assignednew

Sorry, I just can't seem to get the hang of working with git the way you want things done. I'm removing my self from the ticket so that it's available for anyone else wanting to move forward with it.

comment:11 Changed 9 months ago by Carlton Gibson

Hey Ken. No problem. The PR is essentially there. Mariusz or I can pick it up to make the small edits and rebase etc. Thanks for the report and the fix. Welcome aboard!

comment:12 Changed 9 months ago by Mariusz Felisiak

Owner: set to Ken Whitesell
Patch needs improvement: unset
Status: newassigned
Triage Stage: AcceptedReady for checkin

comment:13 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 0a9aa02e:

Fixed #33077 -- Fixed links to related models for admin's readonly fields in custom admin site.

comment:14 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In d4a587a5:

[3.2.x] Fixed #33077 -- Fixed links to related models for admin's readonly fields in custom admin site.

Backport of 0a9aa02e6f1d1b9ceca155d281a2be624bb1d3a2 from main

Note: See TracTickets for help on using tickets.
Back to Top