ModelMultipleChoiceField clean() method calls prepare_value instead of to_python

[Adam McKay]

The ​Form and field validation documentation says:

The clean() method on a Field subclass is responsible for running to_python(), validate(), and run_validators() in the correct order and propagating their errors.

However the clean() method of ModelMultipleChoiceField calls value = self.prepare_value(value) which is causing issues for my use case of hiding primary keys on forms using a ​hashids module as I am subclassing the to_python and prepare_values methods to ensure the pk exposed to users are encoded/decoded as appropriate, however the to_python method is not called as an error “encodedValue” is not a valid value. is returned to the user because the value is not correctly decoded when it is checked as a valid choice.

I have written a test case for tests/model_forms/test_modelchoicefield.py in ModelChoiceFieldTests which for this test merely adds 42 to the pk before exposing it to the user to demonstrate the behaviour:

    def test_clean_serializes_input(self):
        class EncryptedModelMultipleChoiceField(forms.ModelMultipleChoiceField):
            """Hide pk by modifying by 42"""
            def to_python(self, value):
                if not value:
                    return []
                if hasattr(value, '__iter__'):
                    return [int(getattr(v, 'pk', v)) - 42 for v in value]
                return int(getattr(value, 'pk', value)) - 42

            def prepare_value(self, value):
                if not value:
                    return []
                if hasattr(value, '__iter__'):
                    return [int(getattr(v, 'pk', v)) + 42 for v in value]
                return int(getattr(value, 'pk', value)) + 42

        f = EncryptedModelMultipleChoiceField(Category.objects.all())
        print(f.widget.render('name', []),)
        self.assertHTMLEqual(
            f.widget.render('name', []),
            """<select name="name" multiple>
                <option value="%s">Entertainment</option>
                <option value="%s">A test</option>
                <option value="%s">Third</option>
            </select>""" % (self.c1.pk + 42, self.c2.pk + 42, self.c3.pk + 42),
        )
        with self.assertRaises(ValidationError):
            f.clean('')
        with self.assertRaises(ValidationError):
            f.clean(None)
        with self.assertRaises(ValidationError):
            f.clean(0)

        self.assertEqual(['Entertainment'], [c.name for c in f.clean([self.c1.pk + 42])])
        self.assertEqual(['Entertainment', 'Third'], [c.name for c in f.clean([self.c1.pk + 42, self.c3.pk + 42])])

[Carlton Gibson]

Hi Adam — thanks for the report. Interesting.

This kind of mapping has never really been supported, so I'd say this is a new feature rather than a bug. That's probably a small distinction but it does point to a way forward, which is that it would be good to see the required implementation out of Django before deciding whether to make a change here. (It's quite a niche use-case.)

However the clean() method of ModelMultipleChoiceField calls value = self.prepare_value(value) which is causing issues for my use case…

Your test doesn't seem right. If I comment out the offending line, the test still fails (43 is not one of the available choices. rather than 85 is not one of the available choices. — so the 42 is not being applied the extra time, but neither is it being removed in to_python) but now we get the additional failure from #26970 (4e861682904744b0ea3ead8552513c6f1a826c5a).

I use hashids myself at times. I'm inclined to think addressing the mapping at the form Field level is something of a no-man's land. I'd either push it towards the model field, which is what ​django-hashid-field does — the choice then just working — or (which is where I'd look since I don't tend to use the custom model field) push the mapping to a custom ​ModelChoiceIterator/ModelChoiceIteratorValue pair for generating the HTML and a custom widget to map the value back in value_from_datadict(), thus keeping the mapping at the periphery. Otherwise a bit more work on ModelMultipleChoiceField subclass looks needed. (Likely Nathan on django-hashid-field would be able to guide you more.)