Opened 14 years ago

Closed 7 years ago

Last modified 7 years ago

#14370 closed New feature (fixed)

Adding support for Autocomplete in contrib.admin

Reported by: Germano Gabbianelli Owned by: Johannes Maron
Component: contrib.admin Version: dev
Severity: Normal Keywords: autocomplete, jquery, javascript, widgets, admin
Cc: Yeago, Germano Gabbianelli, sehmaschine@…, istruble, thomas.schwaerzl@…, thijs.triemstra@…, hv@…, 4glitch@…, cmawebsite@…, ramezashraf@…, info@…, James Pic, zachborboa@…, tymoteusz.jankowski@…, tymoteusz.jankowski@…, Kevin Brown Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

I've tried to implement Autocomplete for contrib.admin using jQuery UI Autocomplete.
Here's the code: http://bitbucket.org/tyrion/django
Here's a live demo: http://djangoac.tyrion.mx/admin/ (login with test/test).
(It supports both ForeignKey and ManyToMany).

I don't know if I should attach the complete patch here. It's kinda big (I included jquery-ui), however you can see it online at http://bitbucket.org/tyrion/django/changeset/04488ec05e92 ).

Now it's implemented using an "autocomplete_fields" attribute which is a dict (field:related_fields):

class MyModelAdmin(admin.ModelAdmin):
    autocomplete_fields = {'user': ('username', 'email')}


But because I've been asked:

<jezdez> tyrion-mx: what about making the autocomplete_fields a dict of dicts, to be able to specify additional options for the autocompletion, e.g. number of search results or a custom label function?

I've written this patch (for my code) http://dpaste.com/hold/251220/
that let's you control the "value" (what is displayed in the input), "label" (what is displayed in the dropdown) and "limit" (max number of results) properties.

With it a modeladmin could look something like this:

class DummyAdmin(admin.ModelAdmin):
    autocomplete_fields = dict(
        user1 = dict(
            fields = ('username', 'email'),
            value = 'username',
            limit = 10,
        ),
        friends = dict(
            fields = ('username', 'email'),
            label = u'%(first_name)s %(last_name)s',
            value = 'email',
        ),
    )

In that case "value" and "label" can be either a function, a fieldname or a string like u'%(username)s "%(email)s"'.

Attachments (1)

ticket-14730.diff (92.3 KB ) - added by istruble 13 years ago.

Download all attachments as: .zip

Change History (67)

comment:1 by Germano Gabbianelli, 14 years ago

Has patch: set

comment:2 by Yeago, 14 years ago

Cc: Yeago added

comment:3 by Yeago, 14 years ago

Would it be reasonable to deprecate raw_id_fields in favor of this?

comment:4 by Germano Gabbianelli, 14 years ago

Now the autocomplete feature can be used with any type of field.
i.e. (email is an EmailField)

class MyModelAdmin(admin.ModelAdmin):
    autocomplete_fields = dict(
        email = dict(
            queryset = User.objects.all(),
            fields = ('^email', '^username'),
            id = 'email',
            value = 'email',
        ),
    )

comment:5 by anonymous, 13 years ago

Cc: Germano Gabbianelli added

comment:6 by Jannis Leidel, 13 years ago

Triage Stage: UnreviewedAccepted

Marking this accepted after the discussions on IRC and the mailing list.

comment:7 by Rob Hudson, 13 years ago

A couple notes after trying to get this to work on an existing project...

  1. This definitely needs documentation on how to enable these widgets. I took what seemed like the obvious approach and tried adding the MultipleAutocompleteWidget to an existing m2m field and got an error that the __init__ takes 2 arguments and only 1 was given. After perusing the source I see it takes a settings dict so I passed in an empty one that seemed to make it happy. Personally I think an empty dict should be a default along with default settings so the user only has to override via:
    formfield_overrides = {
        models.ManyToManyField: {'widget': MultipleAutocompleteWidget}
    }
  1. It looks like the jquery-ui.min.js isn't loaded anywhere. I get an error in the console that $.widget is not a function. If I add jquery-ui to the list of inner class Media it moves me past this error to another one...
  1. (void 0) is undefined on line 15 of jquery-ui.js, which is the $.ui = $.ui || {}; line. This is about where I stopped. I tried changing jquery-ui to load django.jQuery rather than jQuery as a test and still got the error. Not sure about this one.

Hope that feedback helps some. I'm happy to continue trying and poking at this.

comment:8 by Germano Gabbianelli, 13 years ago

  1. The documentation is at the top of the page. You can't use formfield_overrides with autocomplete_fields.
  2. jquery-ui.min.js is loaded in ModelAdmin._media.
  3. You shouldn't get any js error if you use it as documented.

Let me know if you manage to make it work :)

comment:9 by Rob Hudson, 13 years ago

  1. I should have caught the above code sample when trying to hook this up, but this will need docs in the patch as part of the Django docs before this is committed. That's what I was looking for and referring to.

2 & 3. Ignore. Apologies.

OK, so now that I have it working a few comments...

  1. Looks like it could use a little CSS touch-up work. I didn't see any CSS changes or additions on the bitbucket changes.
  1. I think some reasonable defaults should be considered so not all keys of the dict need to be specified. e.g. if queryset isn't specified assume an .all() query?
  1. Django docs.
  1. Have you considered how we might test this? It would probably be pretty easy to test that the admin hooked up the view that the autocomplete calls. Testing the JS itself would prove a bit tougher but jezdez and I have mentioned that we probably need to figure this out eventually (QUnit?).

comment:10 by anonymous, 13 years ago

Sure, some documentation should be added.

  1. I am not good with CSS, In my local copy I included the jquery lightness theme, with some improvements for contrib.admin...
  1. There are already some "reasonable" defaults, the queryset is required only for non fk/m2m fields.

For fk/m2m fields the queryset if not specified is taken from the field (with respect to limit_choices_to).
The only required key should be "fields" (which maybe should be renamed to "search_fields").

  1. I don't know how the javascript could be tested. Do other Admin widgets have javascript tests?

comment:11 by Jannis Leidel, 13 years ago

Needs documentation: set

It would be great if you could attach a diff of the current state to this ticket.

comment:12 by anonymous, 13 years ago

Cc: sehmaschine@… added

comment:13 by sehmaschine@…, 13 years ago

I´ve just tested the patch and I think it´s great.

there´s just one minor issue: I´d still show the lookup-icon, because in some (rare) cases you won´t find an object without.

comment:14 by Jannis Leidel, 13 years ago

Would making that an option per autocomplete field option make sense?

comment:15 by sehmaschine@…, 13 years ago

I personally wouldn´t make that an option because of coherence (the raw-id-field also shows the lookup-icon).
however, if the icon is there by default and I´m having the option to disable it ... why not. the main point is that the lookup-icon is needed.

just to clarify: with showing the lookup-icon, I don´t mean to show the input-field for the object-id as well.

comment:16 by Jannis Leidel, 13 years ago

Ah, ok, that'd make sense indeed, agreed.

comment:17 by istruble, 13 years ago

Cc: istruble added

I have added the lookup/search icon, a splash of CSS, a pinch of documentation and a sprinkling of testing.

No automated javascript testing at this point. Based on manual testing, I do know that IE6 and IE7 do not work with the current code. FF, Safari and Chrome seem like they are good to go. It would be great to hear from someone with IE8 or greater what the experience is like.

Patch file attached to the ticket based on django-trunk within the last few days.

by istruble, 13 years ago

Attachment: ticket-14730.diff added

comment:18 by istruble, 13 years ago

Performed a jslint cleanup of the autocomplete.js and found some trailing commas that were breaking things for IE. IE6 and IE7 now work. IE8 and up should also work.

comment:19 by Eduardo Cereto, 13 years ago

Cc: Eduardo Cereto added

is jqueryUI really needed? If that dependency is removed I think this is more likely to be accepted. Since jqueryUI is quite large and there's no reason to use it at all.

comment:20 by Thomas Schwärzl, 13 years ago

Cc: thomas.schwaerzl@… added
Severity: Normal
Type: Uncategorized

comment:21 by Andy Terra, 13 years ago

Cc: andreterra@… added
Keywords: jquery javascript widgets admin added
Patch needs improvement: set
Type: UncategorizedNew feature
Version: 1.2SVN

could we possibly get a "real" patch for this? 1.3 has been released and the bitbucket repo now differs greatly from trunk, plus some of code was on dpaste (?) and is now gone, so it's extra hard to dive into the issue at the moment.

i'd like to help with this and make it "official" as soon as possible. in a worst case biggest noob scenario i'll test and write docs!

in reply to:  21 comment:22 by Germano Gabbianelli, 13 years ago

Replying to airstrike:

could we possibly get a "real" patch for this? 1.3 has been released and the bitbucket repo now differs greatly from trunk, plus some of code was on dpaste (?) and is now gone, so it's extra hard to dive into the issue at the moment.

If you look at contrib.admin in my django bb repo, you can get an idea of how it was implemented.
In the mean time I used that code to rewrite django-autocomplete. Now it's far more customizable than the initial patch in this ticket (but it is implemented as a generic app, not as a patch to contrib.admin, since I thought that other apps could benefit of the code too).

  • Is tying a generic AutocompleteWidget that could be used by other apps to contrib.admin a good thing to do?
  • Should we also add the "add another" link?

If we're satisfied with the current implementation, updating it to the trunk shouldn't be too difficult.

comment:23 by Matt Harasymczuk, 13 years ago

Cc: djangoproject.com@… added
Easy pickings: unset

comment:24 by Julien Phalip, 13 years ago

UI/UX: set

comment:25 by Thijs Triemstra <thijs.triemstra@…>, 12 years ago

Cc: thijs.triemstra@… added

comment:26 by Etienne Desautels, 12 years ago

I just want to let you know that I did a fork of tyrion django-autocomplete that add a little bit more integration with the Django Admin and other things:

  • Highlight (put in bold the search term in the results list)
  • Auto Focus (automatically select the first result of the list) need jQuery UI >= 1.8.11
  • Zebra (colorize list rows in alternance)
  • Simple Cache (cache each different query)
  • Add button (the green plus to add a new item for a Relation Field)
  • Lookup button (the loupe like in a RawIdField to select from the complete list)
  • Multiterms (possibility to have completion on many terms in one field separated by a delimiter, for example: a tag field)
  • Distinct result (get only distinct results for all fields excepted Relation fields)

Maybe something there could help doing the integration to the Django Admin. Here's a screenshot of it in action.

Version 0, edited 12 years ago by Etienne Desautels (next)

comment:27 by Thomas Güttler, 12 years ago

Cc: hv@… added

comment:28 by 4glitch@…, 11 years ago

Cc: 4glitch@… added

comment:29 by Collin Anderson, 8 years ago

Cc: cmawebsite@… added

comment:30 by Collin Anderson, 8 years ago

comment:31 by Ramez Issac, 8 years ago

May i ask why select2 ?!

Can this patch be done in a way that ease 3rd party integration , (or make it optional to use select2) rather then changing the default ?!

Thanks.

comment:32 by Johannes Maron, 8 years ago

I don't know, this is django.contrib.admin not core were're talking about. I don't see why we should improve it using more javascript libraries.
I don't like the fact tho, that it is being distributed within our repo. I would go for a CDN or some build method.

comment:33 by Aymeric Augustin, 8 years ago

Not using a CDN is a conscious choice.

There are at least two reasons for this:

  • privacy (avoid leaking info through the Referer header)
  • offline use (download the tar.gz and the docs once and use them for local development: not everyone on Earth has convenient access to a high-speed, permanent Internet connection, in doubt we'd rather favor the use case of the least privileged people)

I'm aware that these are social arguments, not technical ones, and I stand by them.

comment:34 by Carl Meyer, 8 years ago

And a third argument for not using a CDN -- we'd be quietly introducing another point of failure into people's projects.

That said, the "or some build method" idea is probably a better one. It might be nice if we used bower or something similar to automate pulling in the outside JS dependencies, instead of manually vendoring them. But it's also not really a big deal. Vendored dependencies are more easily auditable, and everyone can figure out how to upgrade them when necessary.

comment:35 by Ramez Issac, 8 years ago

Cc: ramezashraf@… added

comment:36 by Collin Anderson, 8 years ago

why select2 ? It seems like a popular and mature library. I have to admit I've never used an autocomplete library before so I'm a bit of an imposter here. :) I'm a little scared that it just got major breaking changes. What would you use? jQuery UI? Typeahead?

changing the default It's opt-in. You have to specify ajax_autocomplete = ['user'] or maybe you'll have to use formfield_overrides. 3rd-parties could re-use the backend view.

vendoring / distributing with django I think it's a necessary evil. I'm a little scared that we went from vendoring just jQuery to including xregexp and Roboto in just 6 months. The native html5 datalist doesn't seem good enough for the job, and I tried hand-writing autocomplete javascript in django and quickly realized a dependency is a must. Currently vendoring is the only option for django, and it's a reasonable size (slightly smaller than jQuery, 80k vs 84k). I see this as the main hurdle to getting this merged. We would need to run it by the DevelopersMailingList for sure.

reusing django-select2 I think the widget code in django-select2 is much better than what I have, and of course django-select2 actually has tests. I'd be interested in re-using those. I'm a bit worried about the backend code. Currently I'm trying to implement this as similar to raw_id_fields as possible. If we reuse the admin code we get permissions, to_field, limit_choices_to, search, get_queryset, urls, etc for free. What would it take to add the missing features to django-select2?

next steps I mostly made this pull request as a discussion starter and to show what I think is a good way forward. I don't actually have the energy right now to push this to completion, but I'd help review pull requests.

comment:37 by Johannes Maron, 8 years ago

Cc: info@… added

I'm maintaining django-select2 and wrote the current version. Let me share my thoughts on integrating something similar in django.conrtib.admin.

django-select2 pickels the query and stores it in django.core.cache, to work as a drop in widget. I think this would be a bit over the top. I would suggest using app_name, model_name and field_name to uniquely identify the relation.
From that point it should be possible to determine the queryset including limit_choices_to constraints.

I think that select2 is a great choice, I worked with other libraries, but since select2 version 4, I'd recommend it over other solutions.

If you want I could give it a shot. In any case feel free to use code from django-select2.

comment:38 by Simon Charette, 8 years ago

I also think Select2 is one if not the best JavaScript autocomplete library around at the moment.

I've tried alternative such as Chosen and multiple jQuery/Bootstrap alternatives in the past and but just like @codingjoe said since version 4 Select2 is quite solid. It's a great shim that binds correctly to the DOM state of the <select> it's attached to and doesn't require any extra manipulation to be marked as disabled, focused, etc

comment:39 by James Pic, 8 years ago

Since codingjoe told me a couple of days ago that he didn't know about django-autocomplete-light (which i maintain) before he took over on django-select2, it should be mentioned that django-autocomplete-light also has support for generic foreign key which we might want in django admin as well (and it supports django-generic-m2m too), supports add-another (even outside the admin) and so on. While django-select2 might be lucky enough that it gets its features inside contrib.admin, I'm affraid that DAL will still require maintainance even in future versions of django. Anyway, autocompletion is a really rich subject and I recommend to write the documentation before the implementation itself, which use cases do you want to support and so on.

comment:40 by James Pic, 8 years ago

Cc: James Pic added

comment:41 by Zach Borboa, 8 years ago

Cc: zachborboa@… added

comment:42 by xliiv, 8 years ago

Cc: tymoteusz.jankowski@… tymoteusz.jankowski@… added

comment:43 by Thomas Güttler, 8 years ago

Just for the records. This comparison grid can help to find a matching library: https://www.djangopackages.com/grids/g/auto-complete/

comment:44 by Johannes Maron, 8 years ago

Owner: changed from nobody to Johannes Maron
Status: newassigned

James and me just discussed selectize.js vs select2.
We agreed that selectize is promising but doesn't have all the features yet, to be implemented in django without much JS work.
Select2 seems to be the better choices because it supports html data attributes.

We also discussed how to support limit_choices_to for the AJAX result set. We're probably going to send the field reference to the AJAX endpoint to figure out the QuerysetChoices.

comment:45 by Johannes Maron, 8 years ago

Patch can be found here: https://github.com/django/django/pull/6385
There are still some implementation details to be discussed.

  • Is this going to replace the RawIdWidget?
  • Do we add a new autocomplete_fields attribute?

comment:46 by Eduardo Cereto, 8 years ago

Cc: Eduardo Cereto removed

comment:47 by Andy Terra, 8 years ago

Cc: andreterra@… removed

comment:48 by Kevin Brown, 8 years ago

Cc: Kevin Brown added

comment:49 by Tim Graham, 8 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:50 by steve yeago, 8 years ago

I'd like to raise something else for discussion...

Regarding permission checks, I think matching the way raw_id_fields would just be building in unwanted limitations in many cases. Firstly, the permission says 'change' where no change is occurring. It just seemed like the original implementation was taking a short-cut by using that permission (since it was effectively a pop-out changeview).

In many cases, someone may want to grant access to a form that has an autocomplete list of users but that doesn't mean they want to grant change access to their user list. Moreover, we're already granting a permission when we are allowing access to the admin form the autocomplete would be used on. I'm not sure what case we are covering by granting access to a form but not granting access to the autocomplete we explicitly added.

in reply to:  50 comment:51 by Johannes Maron, 8 years ago

Replying to yeago:

I'd like to raise something else for discussion...

Regarding permission checks, I think matching the way raw_id_fields would just be building in unwanted limitations in many cases. Firstly, the permission says 'change' where no change is occurring. It just seemed like the original implementation was taking a short-cut by using that permission (since it was effectively a pop-out changeview).

In many cases, someone may want to grant access to a form that has an autocomplete list of users but that doesn't mean they want to grant change access to their user list. Moreover, we're already granting a permission when we are allowing access to the admin form the autocomplete would be used on. I'm not sure what case we are covering by granting access to a form but not granting access to the autocomplete we explicitly added.

It's not implemented that way. In fact you need change permission on the model the you are currently editing not the related object. e.g. If you have access to groups but not to users, the groups m2m autocomplete would still work.
Does that solve the issue for you?

comment:52 by Johannes Maron, 8 years ago

Since there are so many people following this ticket. Please review. From where I'm standing this ticket is only shy of a review to be merged into 1.10. So please, please, please :)

comment:53 by steve yeago, 8 years ago

@codingjoe I'm not sure that you understood properly what I am saying, because I'm not wrong. Go look at a raw_id_fields to a user object then, as a user without the user.change permission, try to edit that field. You will get a Forbidden error. You need the user.change permission in order to pull up the eyeglass required to populate a raw_id_field. The only argument is whether this is a feature and not a bug.

comment:54 by Collin Anderson, 8 years ago

yeago: I think codingjoe is saying that his autocomplete is implemented differently from raw_id_fields. In his implementation, you need change permission on the model the you are currently editing not the related object. (Try it out and see if you agree.)

comment:55 by Matt Harasymczuk, 8 years ago

Cc: djangoproject.com@… removed

comment:56 by Tim Graham, 8 years ago

Patch needs improvement: set

Not an extensive review yet, but I left some comments for improvement on the PR.

comment:57 by Johannes Maron, 8 years ago

Patch needs improvement: unset

comment:58 by Tim Graham, 7 years ago

Patch needs improvement: set

As Florian noted on the PR, "Using the "edit"-icon next to a FK does not update the autocomplete widget."

comment:60 by Johannes Maron, 7 years ago

@James

yes, but that only works for actual option tags doesn't it? In this case the we are only using a JSON API and the dynamic options.
I haven't looked into that yet. You are a better JS coder than me. I would greatly appreciate a patch or suggestion form your side.

comment:61 by James Pic, 7 years ago

This is the code that does the trick for me: https://github.com/yourlabs/django-autocomplete-light/blob/master/src/dal_select2/static/autocomplete_light/select2.js#L91-L97

    // Remove this block when this is merged upstream:
    // https://github.com/select2/select2/pull/4249
    $(document).on('DOMSubtreeModified', '[data-autocomplete-light-function=select2] option', function() {
        $(this).parents('select').next().find(
            '.select2-selection--single .select2-selection__rendered'
        ).text($(this).text());
    });

Just replace [data-autocomplete-light-function=select2] by your own select tag selector and you should be good to go.

However, it was meant as a temporary fix even for DAL, so I'm not sure if this is of any help to you. I would suggest to add a breakpoint somewhere in the code that Kevin's patch adds or changes and hopefully if it's executed at all then the solution should be easy to find.

comment:62 by James Pic, 7 years ago

Ok so I was wrong, here's the new ticket for this issue: https://github.com/select2/select2/issues/4733

comment:63 by Johannes Maron, 7 years ago

Patch needs improvement: unset

comment:64 by Tim Graham <timograham@…>, 7 years ago

In 01a294f8:

Refs #14370 -- Vendored Select2 4.0.3 for use by the admin.

From https://github.com/select2/select2/releases/tag/4.0.3

comment:65 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 94cd8ef:

Fixed #14370 -- Allowed using a Select2 widget for ForeignKey and ManyToManyField in the admin.

Thanks Florian Apolloner and Tim Graham for review and
contributing to the patch.

comment:66 by Tim Graham <timograham@…>, 7 years ago

In f13bd321:

Refs #14370 -- Fixed typo in ModelAdmin.autocomplete_fields docs.

Note: See TracTickets for help on using tickets.
Back to Top