ModelMultipleChoiceField doesn't select initial choices

[uhlr@…]

When using a ModelMultipleChoiceField, initial values are not selected. After some investigation, I discovered that this was because SelectMultiple.render() was comparing the primary key of the choices to the unicode string of each value; this obviously only works if one's model returns the primary key, which is near-universally suboptimal.

I have modified SelectMultiple to call a method value_to_str() (perhaps not the best name) which defaults to returning the force_unicode() of the value; I've added a subclass ModelSelectMultiple which overrides this to return SelectMultiple.value_to_str(value._get_pk_val()). Finally, I modified ModelMultipleChoiceField to use a ModelSelectMultiple widget rather than a SelectMultiple.

This does what's expected, and is extensible in the future.

[uhlr@…]

svn diff for patch application

[uhlr@…]

Note also that in my SVN branch smart_unicode() is used; in my installed version force_unicode() is used instead. In my patch I use smart_unicode(); if it has gone away then simply replace it with force_unicode().

[Philippe Raoult]

might be a duplicate of #5481 which also has a patch.

[Robert Coup]

It's not a duplicate, but its semi-relevant. In #5481, whatever ends up as keys in ChoiceField.choices gets faithfully returned as cleaned_data - be it integer, string, object, etc.

Should creating keys be the widgets responsibility or the fields?

[Peter Baumgartner]

patch against #6702 that passes tests

[Luke Plant]

Just by looking at them, you can see the patches can't work. ModelMultipleChoiceField is defined as a function when it should be a class. In the latest patch, it is then imported into another model and never used!

To determine correctness, there should be a test which fails without the patch, and passes with it.

[Peter Baumgartner]

OK, here's a better patch, although I'm not sure it is the right one. It works for me but I'm not sure that testing for _get_pk_val is the best way to determine whether we're working with a model instance or not. Unfortunately, django.db.models can't be imported here to use isinstance

Here is a simple test case for the problem:

models.py:

from django.db import models

class MyModel(models.Model):
    name = models.CharField(max_length=100)

forms.py:

from django import newforms as forms
from myapp.models import MyModel

class MyForm(forms.Form):
    my_field = forms.ModelMultipleChoiceField(queryset=MyModel.objects.all())

from myapp import forms
from myapp.models import MyModel
MyModel.objects.create(name="one")
f = forms.MyForm({'my_field':MyModel.objects.all()})
f['my_field'].data #correct
f['my_field'].as_widget() #incorrect, without patch nothing is selected

[Peter Baumgartner]

better patch

[Brian Rosner]

I really don't think special casing objects is the right thing to do here (Especially a hacky type checking using hasattr). While what is described doesn't work, I feel this is a very uncommon use-case that can be easily fixed by simple using list comprehension:

>>> form = forms.MyForm(initial={"my_field": [o.pk for o in MyModel.objects.all()]})
>>> print form["my_field"]
<select multiple="multiple" name="my_field" id="id_my_field">
<option value="1" selected="selected">Test 1</option>
<option value="2" selected="selected">Test 2</option>
<option value="3" selected="selected">Test 3</option>
<option value="4" selected="selected">Test 4</option>

[Brian Rosner]

Jacob,

I meant to close this ticket as a wontfix, but looks like I forgot to do that, per my reasoning above. Do you see that the current patch something that is acceptable or another way to solve this? I feel a simple list comprehension above would be fine the correct way to solve this.

[Brian Rosner]

Ugh, ok, I thought about this a bit more on my way into work. I am completely missing the point ;) Discard what I said earlier. I am going to look into this a bit more.

[Jacob]

Yeah, I would expect if I've got a ModelMultipleChoiceField I should be able to do initial={'f': MyModel.objects.filter(....)} and have that Just Work™.

[drano]

I have the same problem with initial choice selected. I have applied the patch, and the problem is solved. Then, why this patch is not included in the trunk ?

[draxus]

Please, add this patch to the trunk. I was geting mad.

[Karen Tracey]

Replying to draxus:

Please, add this patch to the trunk. I was geting mad.

Patch is very out of date -- if you've updated it to work with current trunk please attach it, since the latest patch here makes changes to code I can't even find in the current file. Also, a test integrated into the current Django test suite that demonstrates the problem (fails on current trunk code, passes with the patch) would go a long way to helping move this one along. Unfortunately, updating the patch and adapting the test that is mentioned in the comments to fit in the test suite requires more work than I have time for at the moment. If, however, a current patch with integrated test were to appear, it would be much more likely to get review and be checked in.

[Tim Kersten]

I've attached a patch against the current trunk.
I've also attached a patch adding a test, which fails if the fix isn't applied.
Could you please review this and give feedback if it cannot be applied to trunk?

Thanks,
Tim

[Tim Kersten]

Um, actually scratch my patch. I didn't read the original bug report correctly :)

[anonymous]

Having ran into a similar problem I am wondering on the status of this ticket. My situation is using a CheckboxSelectMultiple for a ModelMultipleChoiceField with CheckboxSelectMultiple. Behaviour of this combination seems fine with the notable exception of initial values, much like the problem mentioned in this ticket. My fix is trivial to subclass the checkbox widget thus:

class ModelCheckboxSelectMultiple(CheckboxSelectMultiple):
    def render(self, name, value, attrs=None, choices=()):
        # convert the 'value' into a list of pk values before rendering forces the instances to unicode
        if value is None:
            value = []
        selected_keys = []
        for v in value:
            selected_keys.append(v.pk)
        return super(ModelCheckboxSelectMultiple, self).render(name, selected_keys, attrs, choices)

Looking for direction on this and I would be happy to open a new ticket if that makes sense.

[davidcooper]

Apologies on the anonymous post about CheckboxSelectMultiple, forgot to pay attention to my login state.

[Alex Robbins]

I tested the original problem a couple different ways and couldn't get it to show up.

I tested it with a ModelForm, which correctly highlighted values when given an existing instance. I also tested with a normal Form and passed a queryset as the initial argument. This worked too.

Marking this as worksforme. David, if you have a problem that is similar but different, it'd probably be good to make a new ticket so it doesn't get lost in the 4 year history of this ticket.