#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 21 months ago by
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 Changed 21 months ago by
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 21 months ago by
Owner: | changed from nobody to Ken Whitesell |
---|---|
Status: | new → assigned |
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 21 months ago by
Owner: | changed from Ken Whitesell to Abhijith Ganesh |
---|
comment:5 Changed 21 months ago by
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 21 months ago by
Owner: | changed from Abhijith Ganesh to Ken Whitesell |
---|
Changes ownership again.
comment:7 Changed 21 months ago by
I found out that changes got accepted, sorry for the inconvenience caused.
comment:8 Changed 21 months ago by
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:10 Changed 21 months ago by
Owner: | Ken Whitesell deleted |
---|---|
Status: | assigned → new |
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 21 months ago by
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 21 months ago by
Owner: | set to Ken Whitesell |
---|---|
Patch needs improvement: | unset |
Status: | new → assigned |
Triage Stage: | Accepted → Ready for checkin |
Hey Ken, yes seems right. Good spot.
Looks like this should have been part of b79088306513d5ed76d31ac40ab3c15f858946ea for #31181 (which was Django 3.2) here.
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!