Opened 15 years ago
Closed 10 years ago
#13165 closed New feature (fixed)
Display edit link beside add button for ForeignKey fields in admin
Reported by: | Simon Meers | Owned by: | Simon Charette |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | admin foreign key edit link 1.8-alpha |
Cc: | Stefan Wehrmeyer, dev@…, hv@…, ua_django_bugzilla@…, Daniel Samuels, cmawebsite@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | yes |
Description
The reciprocal of #13163 -- when a ForeignKey field is rendered in admin as a select widget with an add (green +) button next to it, also provide an 'edit' button that takes you to the change form for the selected object.
This will require some JavaScript for extracting the selected object.
I'll get onto the code for this shortly; poke me if it doesn't appear soon.
Attachments (7)
Change History (60)
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 3 comment:2 by , 15 years ago
comment:3 by , 15 years ago
Replying to agabel:
Is there any update to this ticket? If not, I'd like to take a stab at it if no one objects.
The code is almost ready, it just hasn't been high on my priority list since I know it won't be committed until after 1.2 lands.
Now that others are expressing interest in the feature I'll try to knock it off ASAP.
comment:4 by , 15 years ago
Has patch: | set |
---|---|
Needs tests: | set |
I have a patch which is working nicely. Confirmation of a few design decisions is required:
- Does the link popup a new window? [I think not, users can control-click for this behaviour]
- Does the link work for the selected object, or just the previously saved one? [I think the latter; otherwise clicking on it will cause changes in the form to be lost. It just needs to be clear that the link is to the saved selection (I've included the name of the object in the link text beside an edit icon).]
- Do we provide edit links for multiple-select widgets? [I've left these blank for now; if you've got 30 objects selected, links will be a bit unwieldy]
- Should it work for
readonly_fields
? [I think it should, so have included this functionality]
Based on my decision for point 2 above, the current patch requires no JavaScript. This also saves us having to do anything ugly with fudging reverse url resolution in JavaScript.
I'll get onto the tests shortly.
comment:5 by , 15 years ago
A further design decision: do we need to check whether the user has permission to edit the related model in question? Currently users will simply see a 'permission denied' message if they click the link and don't have permission. Which is probably OK. Hiding the link for users without permissions requires significant restructuring with the use of custom template tags or similar.
by , 15 years ago
Attachment: | combined_13163_13165.diff added |
---|
comment:6 by , 15 years ago
Needs tests: | unset |
---|
comment:7 by , 15 years ago
Version: | 1.1 → SVN |
---|
follow-up: 9 comment:8 by , 14 years ago
It has some issues with the hungarian accented strings: If there is accent in the object's name, it does not render either the select widget, nor the add/edit buttons.
follow-up: 10 comment:9 by , 14 years ago
Replying to anonymous:
It has some issues with the hungarian accented strings: If there is accent in the object's name, it does not render either the select widget, nor the add/edit buttons.
I cannot reproduce this problem; please provide an example problematic accented string.
follow-up: 11 comment:10 by , 14 years ago
Replying to DrMeers:
Replying to anonymous:
It has some issues with the hungarian accented strings: If there is accent in the object's name, it does not render either the select widget, nor the add/edit buttons.
I cannot reproduce this problem; please provide an example problematic accented string.
Let's say that the model, which is referred through foreign key, has this method:
def unicode(self):
return u"árvíztűrő tükörfúrógép"
If you edit some object of the model, which has the FK field on the admin interface, the source code will not include the select widget. When you press Save, you get an error message that the value is missing (if required). The add page seems like working, because there the field has no value, so the edit button (with the result of unicode() of the linked object) is not present.
follow-ups: 12 13 comment:11 by , 14 years ago
Replying to gk@lka.hu:
Let's say that the model, which is referred through foreign key, has this method:
def unicode(self):
return u"árvíztűrő tükörfúrógép"
If you edit some object of the model, which has the FK field on the admin interface, the source code will not include the select widget. When you press Save, you get an error message that the value is missing (if required). The add page seems like working, because there the field has no value, so the edit button (with the result of unicode() of the linked object) is not present.
Still can't reproduce; see herehttp://share.simonmeers.com/accented_string_foreign_key.png -- are you using trunk/1.2 with this patch applied?
comment:12 by , 14 years ago
Replying to DrMeers:
Still can't reproduce; see herehttp://share.simonmeers.com/accented_string_foreign_key.png -- are you using trunk/1.2 with this patch applied?
I have Django 1.2.1 (stable) with Python 2.6 on Mac OS X.
follow-up: 14 comment:13 by , 14 years ago
Please see this video: http://www.youtube.com/watch?v=BIMRoBbH8ws
follow-up: 15 comment:14 by , 14 years ago
Replying to gklka:
Please see this video: http://www.youtube.com/watch?v=BIMRoBbH8ws
Thank you for taking the time to make this.
Could you please try changing line 374 of django/contrib/admin/util.py
from
'%(text)s</a>' % {'url': url, 'text': text})
to
'%(text)s</a>' % {'url': url, 'text': force_unicode(text)})
and let me know if this solves the problem?
follow-up: 16 comment:15 by , 14 years ago
It didn't help. I have tried to replace text with some constant string like 'text', but it does not work that way either. I think the bug is elsewhere.
follow-up: 17 comment:16 by , 14 years ago
Replying to gklka:
It didn't help. I have tried to replace text with some constant string like 'text', but it does not work that way either. I think the bug is elsewhere.
Very strange; that seems to be the only thing that should be affected by the unicode representation of the object. Can you try something like this in a shell?
>>> from AppName import admin # ensure models get registered >>> from AppName.models import * # get models >>> from django.contrib.admin import site # get admin site >>> from django.contrib.admin.util import * # get get_changelink_ methods >>> obj = ModelName.objects.get(pk=ChooseAnID) >>> get_changelink_url(obj, site) '/admin/AppName/ModelName/ChooseAnID/' >>> get_changelink_html(obj, site) u' <a href="/admin/AppName/ModelName/ChooseAnID/" class="related_field_changelink changelink">edit</a>' >>> get_changelink_html(obj, site, show_name=True) u' <a href="/admin/AppName/ModelName/ChooseAnID/" class="related_field_changelink changelink">edit \xe1rv\xedzt\u0171r\u0151 T\xdcK\xd6RF\xdaR\xd3G\xc9P</a>'
Hopefully something will blow up noisily so we can find where the problem is...
follow-up: 18 comment:17 by , 14 years ago
That's it:
>>> get_changelink_html(obj, site, show_name = True) Traceback (most recent call last): File "<console>", line 1, in <module> File "/Library/Python/2.6/site-packages/django/contrib/admin/util.py", line 370, in get_changelink_html text = _('edit %s' % obj) File "/Library/Python/2.6/site-packages/django/utils/translation/__init__.py", line 55, in ugettext return real_ugettext(message) File "/Library/Python/2.6/site-packages/django/utils/functional.py", line 55, in _curried return _curried_func(*(args+moreargs), **dict(kwargs, **morekwargs)) File "/Library/Python/2.6/site-packages/django/utils/translation/__init__.py", line 36, in delayed_loader return getattr(trans, real_name)(*args, **kwargs) File "/Library/Python/2.6/site-packages/django/utils/translation/trans_real.py", line 276, in ugettext return do_translate(message, 'ugettext') File "/Library/Python/2.6/site-packages/django/utils/translation/trans_real.py", line 262, in do_translate result = getattr(t, translation_function)(eol_message) File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/gettext.py", line 404, in ugettext return unicode(message) UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 26: ordinal not in range(128)
follow-up: 19 comment:18 by , 14 years ago
Replying to gklka:
That's it:
Try replacing
text = _('edit %s' % obj)
with
text = _(u'edit %s' % force_unicode(obj))
and let me know how if this solves it; I'll then update the patch.
follow-up: 21 comment:20 by , 14 years ago
I think the line 370 should have some more fix:
Instead of this:
text = _('edit %s' % force_unicode(obj))
This:
text = _('edit %s') % force_unicode(obj)
So this way it could be translated correctly.
comment:21 by , 14 years ago
Status: | new → assigned |
---|
Replying to gk@lka.hu:
So this way it could be translated correctly.
I believe you are correct, thank you. Patch updated.
follow-up: 23 comment:22 by , 14 years ago
Applying patch does not show any changes at all... what may be the case?
comment:23 by , 14 years ago
Replying to JackLeo:
Applying patch does not show any changes at all... what may be the case?
My bad, patcher couldn't properly read .diff file.
comment:25 by , 14 years ago
Resolution: | → duplicate |
---|---|
Severity: | → Normal |
Status: | assigned → closed |
Type: | → Uncategorized |
Just for the reference, #6723 was closed as dupe.
comment:26 by , 14 years ago
Resolution: | duplicate |
---|---|
Status: | closed → reopened |
Umm; if #6723 was closed as a duplicate of this, why was this one closed also?
comment:27 by , 14 years ago
Type: | Uncategorized → New feature |
---|
comment:30 by , 13 years ago
UI/UX: | set |
---|
comment:31 by , 13 years ago
Component: | User Experience → contrib.admin |
---|
follow-up: 33 comment:32 by , 13 years ago
Easy pickings: | unset |
---|
Hi - I'm just about to start hacking something like this together... any idea when it might make it into trunk? If there is anything useful I can do...
comment:33 by , 13 years ago
Replying to anonymous:
Hi - I'm just about to start hacking something like this together... any idea when it might make it into trunk? If there is anything useful I can do...
We're approaching the alpha/beta release stages so the earlier one starts to work on this and the closer one gets to a fully functional and fully tested code, the higher the chance it has to be included in core. Any contribution is appreciated ;-)
comment:34 by , 13 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | unset |
Status: | reopened → new |
I ported the latest patch to trunk including the following additional changes:
- CSS names now contain - (dashes) instead of _ (underscores)
- link generation in admin.util.get_changelink_html includes escaping
The related-field-changelink CSS class needs some styling (like vertical-align: middle or something).
comment:35 by , 13 years ago
Cc: | added |
---|---|
Status: | new → assigned |
by , 13 years ago
Attachment: | ticket13165_20111004.diff added |
---|
Rewrote patch, removed cruft, added js
comment:37 by , 13 years ago
Patch needs improvement: | set |
---|
jezdez commented in irc and disliked the OPs approach of retrieving the object and wanted the change link to change when a different fk object is selected.
There is a pull request with some open questions: https://github.com/django/django/pull/57
by , 13 years ago
Attachment: | ticket13165_20111005.diff added |
---|
Added links to raw_id and radio_field widgets
comment:38 by , 13 years ago
I posted to the django-ux list about this ticket: https://groups.google.com/forum/#!topic/django-ux/dzkQogwfx4I
comment:39 by , 13 years ago
Here's a little something I implemented for a project. It adds the edit / delete link and enable / disable them when nothing is selected. Plus saving an object after deleting (via popup) updates the repr of the object in the select box.
- http://dl.dropbox.com/u/2759157/empty.png
- http://dl.dropbox.com/u/2759157/selected.png
- http://www.youtube.com/watch?v=H4xqku-BPBU A short demo
The code can be found on django snippet at http://djangosnippets.org/snippets/2562/. I think we should consider adding the delete link also.
comment:40 by , 13 years ago
Cc: | added |
---|
comment:41 by , 13 years ago
Comment comment:39 was me. I packaged a simple app providing admin mixins to add a the edition and deletion links described above. You can find the project on github and on pypi.
comment:42 by , 12 years ago
Cc: | added |
---|
comment:43 by , 12 years ago
Cc: | added |
---|
comment:44 by , 11 years ago
Owner: | changed from | to
---|
@stefanw hope you don't mind I assign this ticket to myself but I'm working on adapting the django-admin-enhancer code to merge it into Django. You can follow the WIP here.
comment:45 by , 11 years ago
Cc: | added |
---|
comment:46 by , 11 years ago
I've been working on this lately and almost everything is working correctly.
The last missing part concerns foreign keys with to_field
since there's no way to retrieve the related object based on the selected to_field
value.
We've got two options here:
- Provides views redirecting to the related object change and delete views based on the provided
to_field
value; - Attach a
data-pk
attribute on all options (in the case ofSelect
widget) or to the raw id input to retrieve.
The second option doesn't seems viable since it's requires a lot of special casing when dealing with different types of widgets.
The first one should be doable by allowing ModelAdmin.get_object
to receive a third parameter specifying which field to retrieve it's model from.
comment:47 by , 11 years ago
I opened a Pull Request with a working implementation of 1.
I'd appreciated feedback on the possible from_field
attack vector those changes introduce. One solution would be to restrict allowed from_field
values based on current admin site registered models pointing to the actual model being looked up.
comment:49 by , 10 years ago
Cc: | added |
---|
comment:50 by , 10 years ago
Created a PR with the changes. It's only missing documentation and a way to deal with the two broken public APIs.
comment:51 by , 10 years ago
Patch needs improvement: | unset |
---|
Added what was missing to ship with 1.8. Review would be greatly appreciated.
comment:52 by , 10 years ago
Keywords: | 1.8-alpha added |
---|
comment:53 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Is there any update to this ticket? If not, I'd like to take a stab at it if no one objects.