[Django] #33106: ModelMultipleChoiceField clean() method calls prepare_value instead of to_python
Django
to_python
--------------------------------------+------------------------
Reporter: Adam McKay | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+------------------------
The [https://docs.djangoproject.com/en/3.2/ref/forms/validation/#form-and-
field-validation 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 [https://hashids.org/ 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])])
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33106>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
to_python
Reporter: Adam McKay | Owner: nobody
Component: Forms | Version: 3.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* type: Bug => New feature
* resolution: => wontfix
Comment:
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.
> 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
[https://github.com/nshafer/django-hashid-field 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
[https://docs.djangoproject.com/en/3.2/ref/forms/fields/#modelchoiceiterator
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.)
--
Ticket URL: <https://code.djangoproject.com/ticket/33106#comment:1>