#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 , 3 years ago
| Severity: | Normal → Release blocker |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 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 , 3 years ago
| Owner: | changed from to |
|---|---|
| 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 by , 3 years ago
| Owner: | changed from to |
|---|
comment:5 by , 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:7 by , 3 years ago
I found out that changes got accepted, sorry for the inconvenience caused.
comment:8 by , 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:10 by , 3 years ago
| Owner: | removed |
|---|---|
| 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 by , 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 , 3 years ago
| Owner: | set to |
|---|---|
| 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!