Opened 17 years ago
Closed 8 years ago
#7028 closed New feature (wontfix)
Better admin raw_id_fields feedback
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | raw-id-fields nfa-someday design_ux raw_id_fields |
Cc: | hv@…, davenaff@…, carlos.palol@…, stan@…, Stephane "Twidi" Angel, depaolim@…, cmawebsite@…, hugo@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | yes |
Description
When you select an item for a raw-id-fields, the description of the id showes up only after saving current record. This is a major drawback for the user when id isn't a slug or a "self speaking" id. This little patch solve the problem showing the description immediately after user chooses the item in the raw id popup window.
Attachments (28)
Change History (112)
by , 17 years ago
Attachment: | improved-raw-id-admin-feedback.diff added |
---|
comment:2 by , 17 years ago
Fixed two minor problem with unicode representation and create id description placeholder also during insertion of a new record
by , 17 years ago
Attachment: | improved-raw-id-admin-feedback-fixed-insertion.diff added |
---|
Fixed two minor problem with unicode representation and create id description placeholder also during insertion of a new record
comment:3 by , 17 years ago
Running tests there are two failure.
The first one is obvious (we added an id to description label):
Expected: <input type="text" name="test" value="1" class="vForeignKeyRawIdAdminField" /><a href="../../../admin_widgets/band/" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="/media/img/admin/selector-search.gif" width="16" height="16" alt="Lookup"></a> <strong>Linkin Park</strong> Got: <input type="text" name="test" value="1" class="vForeignKeyRawIdAdminField" /><a href="../../../admin_widgets/band/" class="related-lookup" id="lookup_id_test" onclick="return showRelatedObjectLookupPopup(this);"> <img src="/media/img/admin/selector-search.gif" width="16" height="16" alt="Lookup"></a> <strong id="view_lookup_id_test">Linkin Park</strong>
The second one less obvious when you have two primary keys identifying foreign record:
Failed example: print conditional_escape(w.render('test', [m1.pk, m2.pk], attrs={})) Exception raised: Traceback (most recent call last): File "/home/marcob/test/lib/django/test/_doctest.py", line 1267, in __run compileflags, 1) in test.globs File "<doctest regressiontests.admin_widgets.models.__test__.WIDGETS_TESTS[21]>", line 1, in ? print conditional_escape(w.render('test', [m1.pk, m2.pk], attrs={})) File "/home/marcob/mytech/lib/django/contrib/admin/widgets.py", line 132, in render return super(ManyToManyRawIdWidget, self).render(name, value, attrs) File "/home/marcob/mytech/lib/django/contrib/admin/widgets.py", line 106, in render output.append(self.label_for_value(value, "id_%s" % name)) TypeError: label_for_value() takes exactly 2 arguments (3 given)
We need a better patch for the second problem
by , 17 years ago
Attachment: | improved-raw-id-admin-feedback-fixed-regression-test.diff added |
---|
This patch leaves only one (trivial) failure running django tests
by , 17 years ago
Attachment: | tests-fix-for-improved-raw-id-admin-patch.patch added |
---|
Little fix (one liner): now django tests run fine
comment:4 by , 17 years ago
With these two patches improved-raw-id-admin-feedback-fixed-regression-test.diff and
tests-fix-for-improved-raw-id-admin-patch.patch (1.2 kB) all django tests run fine:
---------------------------------------------------------------------- Ran 252 tests in 87.861s OK
comment:5 by , 16 years ago
Keywords: | nfa-someday added |
---|
comment:6 by , 16 years ago
Triage Stage: | Unreviewed → Ready for checkin |
---|
by , 16 years ago
Attachment: | improved-raw-id-admin-django-1.0.diff added |
---|
by , 16 years ago
Attachment: | improved-raw-id-admin-django-1.0.1.2.diff added |
---|
patch for django 1.1.0 alpha
comment:10 by , 16 years ago
There is an error, this line in admin_list.py:
result_name = repr(escape(force_unicode(result_id).replace("'", '&prime')))[1:]
Should be:
result_name = repr(escape(force_unicode(result).replace("'", '&prime')))[1:]
comment:11 by , 15 years ago
Needs tests: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
The last comment makes this look not quite ready for checkin. Also there are no tests in the latest patch, not even the earlier posted test changes required to fix test breakage, so it isn't clear what exactly is supposed to be applied. This ticket needs a consolidated patch with tests for the new function before it can be called ready for checkin.
comment:12 by , 15 years ago
Line 182 of the patch to django/contrib/admin/widgets.py currently reads:
def label_for_value(self, value, nome=):
but this is probably meant be:
def label_for_value(self, value, name=):
This would not cause any tests to fail since the arg is not (currently) used.
follow-up: 14 comment:13 by , 15 years ago
I just noticed that the empty string quotes after nome= and name= did not get through to the last comment that I posted.
comment:14 by , 15 years ago
Replying to paul.hide@gmail.com:
I just noticed that the empty string quotes after nome= and name= did not get through to the last comment that I posted.
They were interpreted as Wiki formatting characters which turned on, and then off, italics. To avoid this you should mark code blocks as preformatted text: http://code.djangoproject.com/wiki/WikiFormatting#Preformattedtext. Also checking how the comment will look via Preview before hitting Submit is always a good idea.
comment:15 by , 15 years ago
milestone: | → 1.2 |
---|
comment:16 by , 15 years ago
I've implemented this and also integrated #7923 in my GitHub branch: http://github.com/mrts/django/commits/ticket7028
Admin widget tests pass, but there's a regression in admin views tests -- a spurious False
appears into the output, but I haven't yet got time to figure out from where (probably a trivial problem somewhere).
Attaching a patch.
IMHO #7923 can be closed as duplicate.
by , 15 years ago
Attachment: | ticket7028.patch added |
---|
comment:17 by , 15 years ago
Patch needs improvement: | set |
---|
comment:18 by , 15 years ago
Cc: | added |
---|
by , 15 years ago
Attachment: | ticket7028-2.patch added |
---|
Fixed the mistake that caused False to appear in output when is_popup was False.
comment:19 by , 15 years ago
Tests pass, looks more or less OK to me.
One thing that needs to be resolved is whether the object link should open in a pop-up. That would be more consistent, but would make the corresponding code horrible -- imagine embedding yet another showPopup
into the \'
-quoted part:
' onclick="opener.dismissRelatedLookupPopup(window, %s, \'<a href="%s">%s</a>\'); return false;"'
I'll probably implement this nevertheless.
comment:20 by , 15 years ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
Version: | 1.0 → SVN |
Tried my best to create a clean representation of the intent:
yield mark_safe(u'<%s%s><a href="%s"%s>%s</a></%s>' % (table_tag, row_class, url, (cl.is_popup and ' onclick=' '"opener.dismissRelatedLookupPopup(window, %s, ' "'<a href="%s" " 'onclick="return showRelatedObjectPopup(this);"' '>%s</a>\'); return false;"' % (result_id, result_url, result_name) or ''), conditional_escape(result_repr), table_tag))
Commit: http://github.com/mrts/django/commit/e95f27ad4b74eef3a9d4e9553e5cfb58f159c10a
Tests pass, also tested to be working in a real site.
Patch upcoming.
by , 15 years ago
Attachment: | ticket7028-with_popup.patch added |
---|
Related object links open in popups.
comment:22 by , 15 years ago
I'd be much obliged if someone cared to test the ticket7028-with_popup.patch in IE.
comment:23 by , 15 years ago
mrts, I tried it with IE7 but I got dreaded "Error on page". Can't find where is the problem (I tried for at least on hour, sorry :-( IE is a pita to debug). Hope there is someone smarter than me.
comment:24 by , 15 years ago
There are instructions for JavaScript debugging in IE at http://blogs.msdn.com/ie/archive/2004/10/26/247912.aspx .
I'll get to it eventually myself as well, not sure when though.
comment:25 by , 15 years ago
mrts I followed your link and I discovered the "glich". Thank you!
I've data in my database that contain a ' so this line in the popup window link raises a javascript error:
onclick="opener.dismissRelatedLookupPopup(window, 'A014', 'Servizi connessi all'Agricoltura'); return false;"
The problem is that '
With IE it's enough to open the popup window to get the error, with Firefox you need to click that line. For this I never got it with Firefox, at least once IE helped me :-).
follow-up: 27 comment:26 by , 15 years ago
Fixed with .replace("'", "`")
as was originally intended (but incorrectly implemented). I wonder if this is causes problems for Romanic languages that use `
for accents -- Marco, is it OK for Italian? I.e. you would see Servizi connessi all`
Agricoltura instead of Servizi connessi all'Agricoltura in the object link. Also, I wonder if English speakers would be annoyed by seeing Aesop`
s fables instead of Aesop's fables.
Other fixes:
- more than 7 words are now truncated instead of the somewhat arbitrary 14 in the original patch. Even retaining 7 may be a few too many,
- proper XSS protection with
nameElem.innerHTML = ... html_escape(chosenName) ...;
, also get rid of the ugliness mentioned a few comments above - add
?_popup=1
to the related object popup and dismiss on save - consistent label usage.
Will upload the patch tomorrow.
comment:27 by , 15 years ago
Replying to mrts:
Fixed with
.replace("'", "`")
as was originally intended (but incorrectly implemented). I wonder if this is causes problems for Romanic languages that use`
for accents -- Marco, is it OK for Italian? I.e. you would see Servizi connessi all`
Agricoltura instead of Servizi connessi all'Agricoltura in the object link. Also, I wonder if English speakers would be annoyed by seeing Aesop`
s fables instead of Aesop's fables.
Unfortunately this isn't correct in italian. We use ' (apostrophe) between truncated words ("all'agricoltura") and we sometimes use `
(back-prime? back-apex? how is it called?) with accented word ("menu`" instead of "menù").
Would it be possible to fix it with .replace("'", r"\'")
?
I tried this manually and the error vanished:
onclick="opener.dismissRelatedLookupPopup(window, 'A014', 'Servizi connessi all\'Agricoltura'); return false;"
Will upload the patch tomorrow.
I'll surely try it. Thanks.
by , 15 years ago
Attachment: | ticket7028-with_popup_2.patch added |
---|
Fixes and improvements as described in the comment.
comment:28 by , 15 years ago
Marco, please review and test when you have time (checking with that same failing case in IE is much appreciated). Using `
was both a hack and a bad idea, thanks for rectifying this (my only excuse, albeit a feeble one, is that it doesn't matter in my language, `
and '
are treated more or less equivalently).
comment:29 by , 15 years ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
Although the patch is quite usable, it needs
- more view tests, specifically for the template tag and
options.py/ModelAdmin.response_change
when the next item is implemented - when the object's label changes via the popup, it makes sense to update it with JavaScript in the DOM as well (similarly to
dismissAddAnotherPopup
), there are TODOs in the patch for the places that need updating.
comment:30 by , 15 years ago
Tested and it works! Both in FF than IE. Thanks!
I was to write about update the label if changed when I saw you already write it in the "it needs" part of you comment.
Just to know, are you willing to do it or you left it as an exercise for the reader? :)
Another minor glitch is that I have a table with a FK on a table of hierarchical codes that has a "self" FK. When I open the first popup window and I click on the inside label for the "self" FK I don't get another popup but it opens in the same window. I'm not sure if it's better in this way or to open a new window but to obtain this behaviour it's enough to name the window with the id of the object.
Btw this is nitpicking. You did a wonderful job.
comment:31 by , 15 years ago
mtrs, after looking at your patch I remembered this comment of mine: http://code.djangoproject.com/ticket/6903#comment:30
Fanny, isn't it? I would like to say "great minds think alike" but perhaps is more proper "great asses fart alike" :-)
comment:33 by , 15 years ago
:)
Your nitpicking is most welcome, thanks!
I plan continue taking care of the patch and I'll think about the popup cascading case as well as implement the things mentioned in the TODO comments. However, it is unclear when I have time for it. But I'd say the patch is usable as-is for those who need it.
comment:34 by , 15 years ago
Mrts, I think that we need also to update the label during an add-another:
diff -r be3cfbfcb85b lib/django/contrib/admin/media/js/admin/RelatedObjectLookups.js --- a/lib/django/contrib/admin/media/js/admin/RelatedObjectLookups.js Sat Dec 05 03:10:26 2009 +0100 +++ b/lib/django/contrib/admin/media/js/admin/RelatedObjectLookups.js Sun Dec 06 06:00:58 2009 +0100 @@ -100,6 +100,13 @@ SelectBox.add_to_cache(toId, o); SelectBox.redisplay(toId); } + var nameElem = document.getElementById("view_lookup_" + name); + if (nameElem) { + var chosedIdHref = String(win.location.href).split(/\/add\//)[0] + '/' + newId + '/'; + nameElem.innerHTML = '<a href="' + chosedIdHref + '" ' + + 'onclick="return showRelatedObjectPopup(this);">' + + html_escape(newRepr) + '</a>'; + } win.close(); }
Do you agree?
comment:35 by , 15 years ago
Marco, I've been ill and away, sorry for that. Will get back to this soon.
comment:37 by , 15 years ago
Marco, good point, but here come some of my notes:
- the added block should be within the
} else if (elem.nodeName == 'INPUT') {
branch, - as
win.location.href
already is a string, there is no need to cast it, - we should not
split()
as theoretically it's not impossible that the path already contains an add, e.g. a contrived but not illegal '/add/admin/add/add/(admin is mounted under
/add/, there's an app labeled
add), so I propose the following instead:
var chosenIdHref = win.location.href.replace(/\/add\/[\/]*$/, '/' + newId + '/');(that gets rid of the redundant
?_popup=1` as well), - it makes sense to store the escaped copy of newRepr in the beginning and not escape it again.
Thanks! I've updated the patch accordingly, re-merged master and will upload the resulting patch shortly.
by , 15 years ago
Attachment: | ticket7028-3.patch added |
---|
Merge trunk changes, hadle 'Add another'. Needs a bit more work, but is usable as-is.
by , 15 years ago
Attachment: | ticket7028-4.patch added |
---|
Removed a leftover from a merge conflict, added Marco to authors.
by , 15 years ago
Attachment: | ticket7028-5.patch added |
---|
Change field label on page when object is changed via popup. All functionality done.
comment:38 by , 15 years ago
Patch needs improvement: | unset |
---|
The linked label should properly update now both when objects are added and changed via the popup, so all the required functionality should be there.
Marco and others who are interested in this, please test and report any problems you notice (e.g. I haven't tried in IE).
I'll leave it into "more tests needed" state as of now -- the options.py:response_change():elif request.POST.has_key("_popup")
path needs testing as do quirky PKs and other corner cases.
Another minor glitch is that I have a table with a FK on a table of hierarchical codes that has a "self" FK. When I open the first popup window and I click on the inside label for
the "self" FK I don't get another popup but it opens in the same window. I'm not sure if it's better in this way or to open a new window but to obtain this behaviour it's enough
to name the window with the id of the object.
I'd say handling this is not worth the effort. But you are most welcome to tinker with the case.
comment:39 by , 15 years ago
I've added selection clearing via javascript with the red x button. The ID input field (and the confusing numeric ID) can be entirely hidden from end-users with CSS display: none
now with no loss of functionality.
comment:40 by , 15 years ago
Patch needs improvement: | set |
---|
Note that I haven't even thought about ManyToManyFields, but the patch has to handle them as well eventually. Will deal with it later, left a failing test as a remainder.
by , 15 years ago
Attachment: | ticket7028-7.patch added |
---|
The clearing button only appears if an object is present, next to it. Commit: http://github.com/mrts/django/commit/267b1fea2518b2a39c35708b21b39ebeb069b0ee
comment:41 by , 15 years ago
I've been asked for a 1.1 version by email. See http://github.com/mrts/django/tree/ticket7028-1.1.X .
comment:42 by , 15 years ago
comment:44 by , 15 years ago
mrts, I do agree, but who is in charge to close and link those tickets to this one?
comment:45 by , 15 years ago
A core committer should do this I think. I will presently leave the patch as-is unless instructed otherwise.
comment:46 by , 15 years ago
Keywords: | design_ux added |
---|
comment:47 by , 15 years ago
I created a 3-minute screencast of the new functionality, to make high-level reviewing this easier: http://www.vimeo.com/9322083
comment:49 by , 15 years ago
milestone: | 1.2 |
---|
1.2 is feature-frozen, moving this feature request off the milestone.
comment:50 by , 15 years ago
I hate to do this, but the functionality is merely only close to what I need but not quite there yet. I happen to like (or tolerate rather) the dropdown box for the ForeignKeyField (probably because I make extensive use of def unicode to make my listed names more verbose/descriptive)... and have no need for the pop-up functionality.
Consequently, all I need would be just the linking functionality to the selected Foreign Object. I envision something similar to the way in which the ImageField and the FileField link to a URL of the file/image: with the link *just* *above* the "select" HTML form element.
I've reopened this ticket's 'duplicate' here http://code.djangoproject.com/ticket/12085 for the first issue.
Additionally: It seems like there could be a similarly nice way to link to the foreign key's class' change_list too. Perhaps we could just turn the field's label into a simple link to the class' list table? This second idea should probably be opened up as a separate ticket though...
comment:52 by , 15 years ago
What's blocking progress on one or other of these patches? There seems to be a lot of good work here on a worthy improvement to the admin interface.
In absence of accepted patches does it make sense to re-craft these patches as widgets that could be used with ModelAdmin.formfield_overrides?
comment:53 by , 15 years ago
My patch is complete, but doesn't touch ManyToManyFields. I can not drive this further though for various reasons, so someone else should step up and continue the work.
comment:54 by , 15 years ago
comment:55 by , 14 years ago
While you wait for this ticket to be resolved, I've added a snippet that shows the values for both ForeignKeys and ManyToManyFields, and links each value to its change page in the admin. If this is something that can be used in this patch, feed free to pick whatever you want from it: http://djangosnippets.org/snippets/2217/
by , 14 years ago
Attachment: | patch7028-1.2.4.patch added |
---|
Patch against 1.2.4 (no patch for tests, save file admin_list.py with unix fileformat)
comment:56 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:57 by , 13 years ago
Cc: | added |
---|---|
Easy pickings: | unset |
comment:58 by , 13 years ago
Easy pickings: | set |
---|
Hmm, just meant to add me to cc list - didn't mean to change easy pickings. Setting easy pickings.
comment:59 by , 13 years ago
Easy pickings: | unset |
---|
Hmm, that message is confusing lots of people. (it was technically 'null' before, which counts as 'false', which was correct, but then your update forced it to be explicitly 'false' i.e. unset, which was still correct. Until you toggled it :-)
comment:60 by , 13 years ago
UI/UX: | set |
---|
follow-up: 63 comment:62 by , 13 years ago
by , 13 years ago
Attachment: | patch7028-1.4.rev16669.patch added |
---|
Patch against latest trunk. Get rid of useless `get_related_url()' function.
by , 13 years ago
Attachment: | patch7028-1.4.rev16669.2.patch added |
---|
Oups. Uploaded the wrong file (this one replace the previous upload).
by , 13 years ago
Attachment: | patch7028-1.4.rev16669.3.patch added |
---|
Fix the `ValueError' raised before validation when non-integer value is enter.
comment:63 by , 13 years ago
follow-up: 67 comment:64 by , 13 years ago
This looks really good! However, it really needs to support ManyToManyFields
before it can be included in core. A list builder widget, as specified in [1]
, could be a nice solution.
comment:66 by , 13 years ago
Cc: | added |
---|
comment:67 by , 13 years ago
Cc: | added |
---|
Replying to julien:
This looks really good! However, it really needs to support
ManyToManyFields
before it can be included in core. A list builder widget, as specified in[1]
, could be a nice solution.
That's nice indeed but for the sake of consistency the same interface should also be ported to the ForeignKey's raw_id. This mean to completely rethink this patch.
And what about the non-javascript users ?
Merging #10293 looks more reasonable and simpler to me.
But I like the idea of an auto-completion (and so my users) !
by , 13 years ago
Attachment: | patch7028-1.4.rev16961.patch added |
---|
Patch to handle unregistred FK. +Regression tests.
by , 13 years ago
Attachment: | patch7028-1.4.rev16961.2.patch added |
---|
ManyToMany raw_id_field label added.
comment:68 by , 13 years ago
ManyToMany raw_id_field: Maybe we could add a "Clear" button for each labeled object in order to javascriptly remove its Id in the input ?
comment:69 by , 13 years ago
Cc: | added |
---|
comment:70 by , 12 years ago
This patch looks great!
What's the status of this patch? Is any work being done to include it in core?
Thanks a lot!
comment:72 by , 12 years ago
Add patch against 1.4.5.
Fix a bug contrib.admin.ManyToManyRawIdWidget#label_for_value(self, value, name) when value was empty.
comment:73 by , 11 years ago
Fixed `Clear' button malfunction in admin raw_id fields.
Github branch : https://github.com/Starou/django/tree/ticket_7028_1_4
comment:74 by , 11 years ago
I've worked on a reasonably straightforward implementation (1) for this inspired from DrMeers' "cooked_ids" (2). I've also written a number of selenium tests. It has the advantage of preserving raw_id_fields if you still want them (for whatever reason), while allowing you to use the more user-friendly version (here called "token_fields").
The main piece that's still missing from this implementation is the ability to work with non-integer primary keys.
Any feedback?
(1) https://bitbucket.org/drmeers/django-generic/src/bd20113dce83a5ba6db30a96f0493b93e3aeea17/generic/admin/mixins/cooking.py
(2) https://github.com/jphalip/django/compare/django:04489c7dbf8f69de84ca272a0a1710e7b6067e9d...ticket-7028-token-fields
by , 11 years ago
Attachment: | Add_token_field_event___Django_site_admin.png added |
---|
First look for the suggested token_fields implementation
follow-up: 77 comment:75 by , 11 years ago
@Julien : I have backported your patch to 1.5.x and tested it. The concern I have is about the lack of the hyperlink to *admin_change* (but maybe it is safer to not provide it rather provide it badly (i.e. permission check)) and the clean button for the ForeignKey.
Anyway, looks good to me !
comment:76 by , 11 years ago
comment:77 by , 11 years ago
Replying to stanislas.guerra@…:
[...] and the clean button for the ForeignKey.
My mistake, the clean button is there.
Anyway, looks good to me !
comment:78 by , 11 years ago
But this does not works with inlines. Using token_fields
into an InlineModelAdmin
gives 404 in ajax call to render the FK unicode().
For example, I have added in the test the following models:
class TicketDealer(models.Model): name = models.CharField(blank=False, max_length=20) website = models.URLField(blank=True) def __unicode__(self): return self.name class TokenFieldTicketDealerEvent(models.Model): """ A model that has token fields in the admin inline. """ dealer = models.ForeignKey(TicketDealer) event = models.ForeignKey(TokenFieldEvent)
And the admin:
class TokenFieldTicketDealerEventAdminInline(admin.StackedInline): model = models.TokenFieldTicketDealerEvent token_fields = ('dealer',) class TokenFieldEventAdmin(admin.ModelAdmin): token_fields = ['main_band', 'supporting_bands'] inlines = (TokenFieldTicketDealerEventAdminInline,)
With the TestCase:
@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class AdminInlineTokenWidgetSeleniumFirefoxTests(AdminSeleniumWebDriverTestCase): available_apps = ['admin_widgets'] + AdminSeleniumWebDriverTestCase.available_apps fixtures = ['admin-widgets-users.xml'] urls = "admin_widgets.urls" webdriver_class = 'selenium.webdriver.firefox.webdriver.WebDriver' def setUp(self): band = models.Band.objects.create(id=42, name='Bogey Blues') event = models.TokenFieldEvent.objects.create(id=11, main_band=band) dealer1 = models.TicketDealer.objects.create(id=12, name="Cheap Horse", website="www.ch.com") dealer2 = models.TicketDealer.objects.create(id=21, name="Decent Buy", website="www.db.com") models.TokenFieldTicketDealerEvent.objects.create(event=event, dealer=dealer1) super(AdminInlineTokenWidgetSeleniumFirefoxTests, self).setUp() def test_foreignkey(self): self.admin_login(username='super', password='secret', login_url='/') self.selenium.get( '%s%s' % (self.live_server_url, '/admin_widgets/tokenfieldevent/11/')) main_window = self.selenium.current_window_handle
It fires http://localhost:8081/admin_widgets/tokenfieldevent/objects-by-ids/tokenfieldticketdealerevent_set-0-dealer/12/ -> 404
I though it was just a bug in retrieving the field name into RelatedObjectLookups.js#updateTokenField() and had it patched like that:
diff --git a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js index 5d8a19c..b333cfd 100644 --- a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js +++ b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js @@ -108,7 +108,7 @@ var updateTokenField = function(field) { var $field = $(field); $field.hide(); var container = $field.closest('div'); - var field_name = $field.attr('name'); + var field_name = getTokenFieldName(field); var pks = encodeURI($field.val()); if (pks) { var objects_url = '../objects-by-ids/' + field_name + '/' + pks + '/'; // FIXME: hard-coded url @@ -128,6 +128,18 @@ var updateTokenField = function(field) { } }; +var getTokenFieldName = function(field) { + // inline fields are prefixed. + var $ = django.jQuery; + var $field = $(field); + var rx_prefix = new RegExp("\\\w+-\\\d{1,2}-\(\\\w+\)"); + var field_name = $field.attr('name'); + if (field_name.match(rx_prefix)) { + field_name = field_name.match(rx_prefix)[1]; + } + return field_name; +} + var removeToken = function(removeLink, field) { var $ = django.jQuery; var li = $(removeLink).parent();
Now It fires an Ajax call to http://localhost:8081/admin_widgets/tokenfieldevent/objects-by-ids/dealer/12/ -> 404, dealer not in TokenFieldEventAdmin.token_fields.
This is because in RelatedOjectLookup.js (l.158) the ajax call is on ../objects-by-ids/et/caetera/ which is in the "path" of the parent ModelAdmin.
In fact, there is not any url to call at this point because InlineModelAdmin
can't register any url.
Any idea on the direction to take ? allow InlineModelAdmin to register url ? Where (under its ModelAdmin or not) ?
Thanks.
comment:79 by , 11 years ago
Cc: | added |
---|
comment:80 by , 10 years ago
Keywords: | raw_id_fields added |
---|
comment:81 by , 10 years ago
Summary: | Better raw_id_fields feedback in newform-admins branch → Better admin raw_id_fields feedback |
---|
comment:82 by , 10 years ago
Cc: | added |
---|
comment:83 by , 9 years ago
Cc: | added |
---|
comment:84 by , 8 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
I think it doesn't make sense to add this enhancement while also adding a new autocomplete widget (#14370). After the autocomplete widget is added, we might deprecate raw_id_fields
in some later release.
immediate raw-id-fields selection feedback