Opened 7 years ago

Closed 7 years ago

Last modified 3 years ago

#8058 closed (fixed)

FilteredSelectMultiple widget should include required Javascript files

Reported by: Erwin Owned by: brosner
Component: contrib.admin Version: master
Severity: Keywords: aug22sprint
Cc: brosner 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 7 years ago.
8058.patch (1.9 KB) - added by cgrady 7 years ago.

Download all attachments as: .zip

Change History (11)

Changed 7 years ago by Erwin

comment:1 Changed 7 years ago by ericholscher

  • Has patch set
  • milestone set to 1.0
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 7 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:3 Changed 7 years ago by cgrady

  • 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 7 years ago by cgrady

comment:4 Changed 7 years ago by mtredinnick

  • Cc brosner 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 7 years ago by ramikassab

  • 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 7 years ago by mattmcc

This ticket's patch also addresses #6012.

comment:7 Changed 7 years ago by brosner

  • Owner changed from nobody to brosner
  • Status changed from new to assigned

comment:8 Changed 7 years ago by brosner

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

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