Opened 3 years ago

Closed 3 years ago

Last modified 12 months 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: master
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 (7)

comment:1 Changed 3 years ago by Héctor Urbina

Description: modified (diff)

comment:2 Changed 3 years ago by Tim Graham

Has patch: unset

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

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

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 Changed 3 years ago by Héctor Urbina

Description: modified (diff)

comment:5 Changed 3 years ago by Tim Graham

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 Changed 3 years ago by Tim Graham

Resolution: wontfix
Status: newclosed

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

comment:7 Changed 12 months ago by 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.

Last edited 12 months ago by Simon Charette (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top