Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#27331 closed New feature (wontfix)

Proposed opt_group argument for ModelChoiceField and ModelMultipleChoiceField

Reported by: Héctor Urbina Owned by: nobody
Component: Forms Version: dev
Severity: Normal Keywords: ModelChoiceField optgroup
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Héctor Urbina)

Hello,

I've just implemented this and I thought It could well be incorporated into Django itself; I guess it's a fairly common feature that one may need on any project.

What I propose is to add an optional opt_group argument to ModelChoiceField and ModelMultipleChoiceField; which indicates the item's field whose value is used to group the choices. It should be used in conjunction with a queryset which is (primarily) sorted by the same field.

Let me show with an example:

class Category(models.Model):
    name = models.CharField(max_length=20)

class Item(models.Model):
    name = models.CharField(max_length=20)
    category = models.ForeignKey(Category)

And in some form's initialization process

field = ModelChoiceField(queryset=Item.objects.order_by('category__name', 'name'), opt_group='category')

field.choices will dynamically collect choices into named groups as a 2-tuple, which the underlying widget should present using optgroup HTML elements.

Change History (8)

comment:1 by Héctor Urbina, 7 years ago

Description: modified (diff)

comment:2 by Tim Graham, 7 years ago

Has patch: unset

At first glance, that looks more appropriate as a widget option since ModelChoiceField isn't required to use a Select widget.

in reply to:  2 comment:3 by Héctor Urbina, 7 years ago

Replying to Tim Graham:

At first glance, that looks more appropriate as a widget option since ModelChoiceField isn't required to use a Select widget.

The default ModelChoiceField's widget (Select) is prepared to understand 2-tuples coming on the field's choices attribute and translates them to optgroups.
ModelChoiceField uses ModelChoiceIterator to generate the choices; that is the actual class that is doing the job on my current implementation. My idea is to include support for optgroup, not require it; I'm updating my proposal to clarify that.

comment:4 by Héctor Urbina, 7 years ago

Description: modified (diff)

comment:5 by Tim Graham, 7 years ago

I'm still unconvinced that adding an argument with an HTML looking name to a model field that only affects selected widgets is a good design. I see that it's convenient compared to the alternative of providing a custom ModelChoiceIterator but I don't know that the coupling is a good design.

comment:6 by Tim Graham, 7 years ago

Resolution: wontfix
Status: newclosed

As the discussion has stalled here, you can write to the DevelopersMailingList for other opinions. Thanks.

comment:7 by Simon Charette, 5 years ago

By the way, this is currently possible with the following code

from functools import partial
from itertools import groupby
from operator import attrgetter

class GroupedModelChoiceIterator(ModelChoiceIterator):
    def __init__(self, field, groupby):
        self.groupby = groupby
        super().__init__(field)

    def __iter__(self):
        if self.field.empty_label is not None:
            yield ("", self.field.empty_label)
        queryset = self.queryset
        # Can't use iterator() when queryset uses prefetch_related()
        if not queryset._prefetch_related_lookups:
            queryset = queryset.iterator()
        for group, objs in groupby(queryset, self.groupby):
            yield (group, [self.choice(obj) for obj in objs])

class GroupedModelChoiceField(ModelChoiceField):
    def __init__(self, *args, choices_groupby, **kwargs):
        if isinstance(choices_groupby, str):
            choices_groupby = attrgetter(choices_groupby)
        elif not callable(choices_groupby):
            raise TypeError('choices_groupby must either be a str or a callable accepting a single argument')
        self.iterator = partial(GroupedModelChoiceIterator, groupby=choices_groupby)
        super().__init__(*args, **kwargs)

While I won't push to get this feature included I think that a request for feedback on django-developers has a good chance of being accepted. It looks like the main argument against closing this ticket as _wontfix_ was the naming of the ModelChoice(opt_group) argument which was effectively leaking HTML implementation details at the form layer.

Last edited 5 years ago by Simon Charette (previous) (diff)

in reply to:  7 comment:8 by angelafoli, 4 years ago

Replying to Simon Charette:

By the way, this is currently possible with the following code

from functools import partial
from itertools import groupby
from operator import attrgetter

class GroupedModelChoiceIterator(ModelChoiceIterator):
    def __init__(self, field, groupby):
        self.groupby = groupby
        super().__init__(field)

    def __iter__(self):
        if self.field.empty_label is not None:
            yield ("", self.field.empty_label)
        queryset = self.queryset
        # Can't use iterator() when queryset uses prefetch_related()
        if not queryset._prefetch_related_lookups:
            queryset = queryset.iterator()
        for group, objs in groupby(queryset, self.groupby):
            yield (group, [self.choice(obj) for obj in objs])

class GroupedModelChoiceField(ModelChoiceField):
    def __init__(self, *args, choices_groupby, **kwargs):
        if isinstance(choices_groupby, str):
            choices_groupby = attrgetter(choices_groupby)
        elif not callable(choices_groupby):
            raise TypeError('choices_groupby must either be a str or a callable accepting a single argument')
        self.iterator = partial(GroupedModelChoiceIterator, groupby=choices_groupby)
        super().__init__(*args, **kwargs)

While I won't push to get this feature included I think that a request for feedback on django-developers has a good chance of being accepted. It looks like the main argument against closing this ticket as _wontfix_ was the naming of the ModelChoice(opt_group) argument which was effectively leaking HTML implementation details at the form layer.

This solution no longer works as of this change: https://github.com/django/django/commit/5ec64f96b2d83ec3c0ef574f52e4767a440017b8

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