ModelForm fails to generate proper ModelMultipleChoiceField when using iterable model

[eric.olstad@…]

When using default admin forms for a model with a ManyToManyField to an iterable model (defines __iter__), the select control generated will have options that have list values instead of the primary key on the model.

Example:

class MyIterableModel(models.Model):
   def __iter__(self):
      for item in self.items:
         yield item

class MyItem(models.Model):
   owner = models.ForeignKey(MyIterableModel, related_name=items)

class MyOtherModel(models.Model):
   myiterables = model.ManyToManyField(MyIterableModel)

In the admin:

admin.site.register(models.MyOtherModel)

When viewing the MyOtherModel page in the admin, the control for the ManyToManyField will have lists of primary keys as each option value.

<select multiple="multiple" name="myiterablemodels" id="id_myiterablemodels">
   <option value="[35636, 35383]">Yada Yada</option>
</select>

This of course generates "not a valid value for primary key" when submitting.

This is because django/forms/models.py prepare_value returns a list.

1035	    def prepare_value(self, value):
1036	        if hasattr(value, '__iter__'):
1037	            return [super(ModelMultipleChoiceField, self).prepare_value(v) for v in value]
1038	        return super(ModelMultipleChoiceField, self).prepare_value(value)

I'm not sure why prepare_value returns a list in that case, so I don't have suggestions for a fix. The workaround is to not use iterable models.

[Junyang Xin]

Hi there,

I am afraid I've tried but failed in reproducing this bug. I created files containing almost identical code snippets you provided in this ticket (with some typos fixed) and tried it on Django 1.4, but when I clicked 'add' to add an object for table MyOtherModel on the admin interface, I immediately got

TypeError at /admin/bugfix/myothermodel/add/

'RelatedManager' object is not iterable

Not to mention getting into the change list page of a certain object to see the multiple-select box. Am I misunderstanding or missing anything?

Best regards

[anonymous]

Sorry about all the typos. The 'RelatedManager object not iterable' is yet another typo. The loop in MyIterableObject's __iter__ method should have a .all().

class MyIterableModel(models.Model):
   def __iter__(self):
      for item in self.items.all():
         yield item

That should do it. Now when trying to add a new MyOtherModel, you should see that the values for each option in the select control is a list. Trying to save should show the primary key error.

[anonymous]

Here is a working typo-free version of the models.py:

from django.db import models


class MyIterableModel(models.Model):
    def __iter__(self):
        for item in self.items.all():
            yield item
    name = models.CharField(max_length=1024)


class MyItem(models.Model):
    owner = models.ForeignKey(MyIterableModel, related_name='items')


class MyOtherModel(models.Model):
    myiterables = models.ManyToManyField(MyIterableModel)

[Melvyn Sopacua]

Testcase

[Melvyn Sopacua]

Added test case that confirms the problem.

[Anssi Kääriäinen]

I think this could be fixed by adding a check for models in the code:

1035	    def prepare_value(self, value):
1036	        if hasattr(value, '__iter__'):
1037	            return [super(ModelMultipleChoiceField, self).prepare_value(v) for v in value]
1038	        return super(ModelMultipleChoiceField, self).prepare_value(value)

the line 1036 should read:

1036	        if hasattr(value, '__iter__') and not isinstance(value, Model):

Anybody willing to write a full patch for this?

[Patryk Zawadzki]

Submitted a pull request: ​https://github.com/django/django/pull/601

[Anssi Kääriäinen <akaariai@…>]

In 3989ce52ef78840eefe01541628daa220191c0ad:

Fixed #18172 -- Made models with iter usable in ModelMultipleChoiceField

Thanks to Patryk Zawadzki for the patch.

[Anssi Kääriäinen <akaariai@…>]

In 9892919b0df0e7369ab793119342182cf91d549d:

[1.5.x] Fixed #18172 -- Made models with iter usable in ModelMultipleChoiceField

Thanks to Patryk Zawadzki for the patch.

Backpatch of 3989ce52ef78840eefe01541628daa220191c0ad