Opened 3 years ago

Closed 3 years ago

Last modified 3 years 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 by Carlton Gibson, 3 years ago

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 by Ken Whitesell, 3 years ago

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 by Carlton Gibson, 3 years ago

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 by Abhijith Ganesh, 3 years ago

Owner: changed from Ken Whitesell to Abhijith Ganesh

comment:5 by Abhijith Ganesh, 3 years ago

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

comment:6 by Abhijith Ganesh, 3 years ago

Owner: changed from Abhijith Ganesh to Ken Whitesell

Changes ownership again.

comment:7 by Abhijith Ganesh, 3 years ago

I am trying to assign this to myself. So forgive me if I commit a message.

Version 0, edited 3 years ago by Abhijith Ganesh (next)

comment:8 by Carlton Gibson, 3 years ago

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 by Mariusz Felisiak, 3 years ago

Has patch: set
Patch needs improvement: set

comment:10 by Ken Whitesell, 3 years ago

Owner: Ken Whitesell removed
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 by Carlton Gibson, 3 years ago

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 by Mariusz Felisiak, 3 years ago

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

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 0a9aa02e:

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

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

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