Document raising a dictionary of ValidationErrors

[Michael Barr]

Assuming the following model:

class DateRange(models.Model):
    start = models.DateField()
    end = models.DateField()

    def clean(self):
        super(DateRange, self).clean()

        if self.start and self.end:
            if self.start >= self.end:
                raise ValidationError(
                    message={
                        'start': _(
                            '{start} must come before {end}.'.format(
                                start=self._meta.get_field(
                                    'start'
                                ).verbose_name,
                                end=self._meta.get_field(
                                    'end'
                                ).verbose_name,
                            )
                        ),
                        'end': _(
                            '{end} must come after {start}.'.format(
                                end=self._meta.get_field(
                                    'end'
                                ).verbose_name,
                                start=self._meta.get_field(
                                    'start'
                                ).verbose_name,
                            )
                        ),
                    },
                    code='invalid',
                )

After then validating the model with a ModelForm, the code fails to be set to 'invalid' but is instead an empty string. I discovered this when I was programming tests:

    def test_start_before_end_validation(self):
        """Start date must come before end date."""
        TestForm = modelform_factory(
            model=DateRange,
            fields=('start', 'end')
        )

        form = TestForm(
            data=dict(
                start=date(2015, 1, 1),
                end=date(1900, 12, 31)     # WHOOPS!
            )
        )

        self.assertEqual(first=form.is_valid(), second=False)

        # The below fails because the code is '' - we have to leave off the code
        self.assertTrue(form.has_error(field='start'), code='invalid')
        self.assertTrue(form.has_error(field='end'), code='invalid')

Upon inspection of the the ValidationError code, I discovered that that any ValidationError that is raised with a type of dict or a list as the message will never set the code:

        if isinstance(message, dict):
            self.error_dict = {}
            for field, messages in message.items():
                if not isinstance(messages, ValidationError):
                    messages = ValidationError(messages)
                self.error_dict[field] = messages.error_list

        elif isinstance(message, list):
            self.error_list = []
            for message in message:
                # Normalize plain strings to instances of ValidationError.
                if not isinstance(message, ValidationError):
                    message = ValidationError(message)
                if hasattr(message, 'error_dict'):
                    self.error_list.extend(sum(message.error_dict.values(), []))
                else:
                    self.error_list.extend(message.error_list)

According to the ​documentation on raising ValidationError, it is suggested to "Provide a descriptive error code to the constructor." Is this a bug?

I believe the "fix" would be as simple as to pass code=code following the message/messages, but I may be wrong. Thoughts?

[Tim Graham]

I think it's preferred to use ValidationErrors as the values of the messages dict.

'start': ValidationError_(
    '{start} must come before {end}.'
    params={...},
    code='invalid',
),
'end': ...

The change you suggested makes sense to me, but I'm not sure if we should do it. If not, might be helpful to issue a warning or exception rather than silently discarding the value from the code parameter.

[Michael Barr]

I can confirm that using ValidationError as the value does work as expected. Perhaps if we don't change how it works internally, we can update the documentation. This particular use case is not covered in the ​Raising ValidationError documentation, but can be found as an example in the ​Model.clean() documentation.

[Loic Bistuer]

code and params are really intended for single message ValidationError, if we did pass code=code in the constructor we'd be effectively creating an implicit shortcut, I don't think it's a good idea (explicit better than implicit, TIMTOWTDI, etc.). Also it's ambiguous if you pass a dict that mixes strings (which will be turned into ValidationError) and ValidationError instances.

Raising ValidationError with error_dict is mostly useful in Model.clean() because forms have better APIs (i.e. Form.add_error()), but I guess we can still document it in ​Raising ValidationError if only for the sake of exhaustivity.

[Tim Graham]

I'll attach the diff of my investigation, but it looks like this should be a documentation ticket then.

[Adam Brenecki]

Would something like this do the job?
​https://github.com/django/django/pull/5088

[Adam Brenecki]

I've removed the 'for instance' bit, is that all that needs fixing?

[Tim Graham]

Loic's comment on the PR makes me think it should be moved to the Model.clean() docs?

[Tim Graham <timograham@…>]

In 52a190b:

Fixed #24988 -- Documented passing a dictionary of ValidationErrors to ValidationError

[Tim Graham <timograham@…>]

In 99b5649a:

[1.8.x] Fixed #24988 -- Documented passing a dictionary of ValidationErrors to ValidationError

Backport of 52a190b65781f8dc07abd230aaf9043fbdbf4fba from master