More flexible choices validation in models.Field

[Jannon Frank]

Disclaimers:
- relatively new to Django (since 1.1)
- couldn't decide whether this was a bug of feature request, so I'll
start here rather than the issue tracker

The Problem:
What I (and at least a couple other people I've seen in django-users)
want to do is have a CommaSeparatedIntegerField(CSIF) with choices in
a model and, let's say, a MultipleChoiceField in a ModelForm
associated with it. So the setup might be like so:

class MyModel(models.Model):
my_field = models.CommaSeparatedIntegerField(max_length = 10,
choices = MY_CHOICES)

class MyForm(forms.ModelForm):
my_field = forms.MultipleChoiceField(choices = MY_CHOICES)

class Meta:
model = MyModel

def clean_my_field(self):
return ','.join(self.cleaned_data["my_field"])

This produces a validation error on MyModel because it is checking the
combined string (e.g. '1,2,5') against the list of choices, when the
intent is to have each of the values in the string checked
individually against the choices. At least, that seems the most
reasonable behavior to me when specifying choices on a CSIF. If folks
don't agree, I guess I'll just go write my own field that validates
properly, but if you do agree...read on.

The Solution?:
Seems to me models.Field could be a little more flexible.
Currently(1.2.1) it just goes about validating choices how it wants
to, without any thought for others:

def validate(self, value, model_instance):
"""
Validates value and throws ValidationError. Subclasses should
override
this to provide validation logic.
"""
if not self.editable:
# Skip validation for non-editable fields.
return
if self._choices and value:
for option_key, option_value in self.choices:
if isinstance(option_value, (list, tuple)):
# This is an optgroup, so look inside the group
for options.
for optgroup_key, optgroup_value in option_value:
if value == optgroup_key:
return
elif value == option_key:
return
raise
exceptions.ValidationError(self.error_messages['invalid_choice'] %
value)

if value is None and not self.null:
raise
exceptions.ValidationError(self.error_messages['null'])

if not self.blank and value in validators.EMPTY_VALUES:
raise
exceptions.ValidationError(self.error_messages['blank'])

but a simple refactoring to pull out the choices validation would
allow subclasses (particularly CSIF, but potentially others) to do
something different:

def validate(self, value, model_instance):
"""
Validates value and throws ValidationError. Subclasses should
override
this to provide validation logic.
"""
if not self.editable:
# Skip validation for non-editable fields.
return

self.validate_choices(value)

if value is None and not self.null:
raise
exceptions.ValidationError(self.error_messages['null'])

if not self.blank and value in validators.EMPTY_VALUES:
raise
exceptions.ValidationError(self.error_messages['blank'])

def validate_choices(self, value):
self._validate_choice(value)

def _validate_choice(self, value):
if self._choices and value:
for option_key, option_value in self.choices:
if isinstance(option_value, (list, tuple)):
# This is an optgroup, so look inside the group
for options.
for optgroup_key, optgroup_value in option_value:
if value == optgroup_key:
return
elif value == option_key:
return
raise
exceptions.ValidationError(self.error_messages['invalid_choice'] %
value)

class CommaSeparatedIntegerField(CharField):
def validate_choices(self, value):
if self._choices and value:
for v in value.split(','):
self._validate_choice(v)

Or something like that.

[Michael Radziej]

On 29.05.2010 19:23, Jannon Frank wrote:
> This produces a validation error on MyModel because it is checking the
> combined string (e.g. '1,2,5') against the list of choices, when the
> intent is to have each of the values in the string checked
> individually against the choices.

Is there any disagreement at all? To me, this looks simply like a bug,
so can you file a ticket for it, please? Tickets don't get lost as
easily as tickets.

Kind regards

Michael