Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#8058 closed (fixed)

FilteredSelectMultiple widget should include required Javascript files

Reported by: Erwin Owned by: Brian Rosner
Component: contrib.admin Version: master
Severity: Keywords: aug22sprint
Cc: Brian Rosner Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

When you override the formfield_from_dbfield method from ModelAdmin in an inlined model, to display a ManyToMany field as a FilteredSelectMultiple widget, the necessary javascripts are not loaded, because the widget itself doesn't take care of that. The included patch fixes this only, not the redundancy (simply removing the inclusion of the javascript in options.py will not work)

Attachments (2)

filtered-select-multiple-widget (734 bytes) - added by Erwin 8 years ago.
8058.patch (1.9 KB) - added by Collin Grady 8 years ago.

Download all attachments as: .zip

Change History (11)

Changed 8 years ago by Erwin

comment:1 Changed 8 years ago by Eric Holscher

Has patch: set
milestone: 1.0
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 8 years ago by Jacob

Triage Stage: Design decision neededAccepted

comment:3 Changed 8 years ago by Collin Grady

Keywords: aug22sprint added

at the sprint today we were unable to replicate the issue here exactly (though we were quite close), but I believe the fix doesn't require duplicating it exactly

however, this turned out to be slightly more involved than the original patch - the js for the widget was being added in ModelAdmin for some reason instead of from the FilteredSelectMultiple widget - in addition, the RelatedFieldWidgetWrapper that admin wraps around the FilteredSelectMultiple widget wasn't passing along the media from the inner widget, so even with the js media settings, it wasn't being passed on to the template :)

attaching a patch which I believe will fix the issue as well as making the widget work correctly in non-admin pages

Changed 8 years ago by Collin Grady

Attachment: 8058.patch added

comment:4 Changed 8 years ago by Malcolm Tredinnick

Cc: Brian Rosner added

@brosner: can you sanity check this? The approach looks like the right direction to me (and I discussed this with cgrady and co at the Portland sprint yesterday), but you're much more familiar with some of the reasons in that code than me. Maybe there is no reason for the current version beyond "it works", in which case this is a good patch. But I'll leave that in your capable hands.

comment:5 Changed 8 years ago by Rami Kassab

Needs documentation: set
Patch needs improvement: set

Cgrady and I worked on this together at the code sprint and put together what we though was an example of the issue. We put some models together and tried to do what was explained in this ticket. Here are the models:

from django.db import models
from django.contrib import admin

class Wheel(models.Model):
    name = models.CharField(max_length=255)

    def __unicode__(self):
        return u'%s' % self.name

class Person(models.Model):
    first_name = models.CharField(max_length=255)
    last_name = models.CharField(max_length=255)

class Car(models.Model):
    name = models.CharField(max_length=255)
    wheels = models.ManyToManyField(Wheel)
    person = models.ForeignKey(Person)

class CarInline(admin.TabularInline):
    model = Car

    def formfield_for_dbfield(self, db_field, **kwargs):
        f = super(CarInline, self).formfield_for_dbfield(db_field, **kwargs)
        if isinstance(db_field, models.ManyToManyField):
            f.widget = admin.widgets.FilteredSelectMultiple(db_field.verbose_name, False)
        return f

class PersonAdmin(admin.ModelAdmin):
    inlines = [
        CarInline,
    ]

admin.site.register(Person, PersonAdmin)
admin.site.register(Wheel)

With this code and without the patch, the ManyToMany fields showed up as plain list boxes, the patch introduced the right JS files to the template and styled the boxes properly; however, out data wasn't being displayed. Wheels was empty, even though we had data in the db. We ran out of time at the sprint before we could debug this and get it figured out. Just attaching our material for reference.

Erwin, it would help if you could post a sample project or code to pinpoint the exact issue here. Thanks!

comment:6 Changed 8 years ago by Matt McClanahan

This ticket's patch also addresses #6012.

comment:7 Changed 8 years ago by Brian Rosner

Owner: changed from nobody to Brian Rosner
Status: newassigned

comment:8 Changed 8 years ago by Brian Rosner

Resolution: fixed
Status: assignedclosed

(In [8764]) Fixed #8058 -- Moved media for filter_vertical/filter_horizontal widget to the widget for use outside the admin. This also corrects RelatedFieldWidgetWrapper? to expose the media of the wrapped widget. Thanks Erwin for the report and cgrady and ramikassab for the complete patch.

comment:9 Changed 5 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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