Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#27434 closed Cleanup/optimization (fixed)

Document caveats of raising a ValidationError in Model.clean() for a field not in a model form

Reported by: Matthias Kestenholz Owned by: Matthias Kestenholz
Component: Documentation Version: dev
Severity: Normal Keywords:
Cc: Loic Bistuer Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Example:

class DictionaryValidationErrorModel(models.Model):
    field1 = models.CharField(max_length=100)
    field2 = models.CharField(max_length=100)
    field3 = models.CharField(max_length=100)

    def clean(self):
        super(DictionaryValidationErrorModel, self).clean()
        raise ValidationError({
            'field1': 'field1 error',
            'field2': 'field2 error',
        })
class DictionaryValidationErrorTests(ValidationTestCase):
    def test_default_form(self):
        class ModelForm(forms.ModelForm):
            class Meta:
                model = DictionaryValidationErrorModel
                fields = ('field1', 'field2', 'field3')

        form = ModelForm({
            'field1': '1',
            'field2': '2',
            'field3': '3',
        })
        self.assertFalse(form.is_valid())
        self.assertEqual(set(form.errors.keys()), {'field1', 'field2'})

    def test_crash(self):
        class ModelForm(forms.ModelForm):
            class Meta:
                model = DictionaryValidationErrorModel
                fields = ('field1', 'field3')

        form = ModelForm({
            'field1': '1',
            'field2': '2',
            'field3': '3',
        })
        self.assertFalse(form.is_valid())
        self.assertEqual(set(form.errors.keys()), {'field1', 'field2'})

The problem I'm seeing is

======================================================================
ERROR: test_crash (validation.test_validators.DictionaryValidationErrorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.5/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/usr/lib/python3.5/unittest/case.py", line 601, in run
    testMethod()
  File "/home/matthias/Projects/django/tests/validation/test_validators.py", line 62, in test_crash
    self.assertFalse(form.is_valid())
  File "/home/matthias/Projects/django/django/forms/forms.py", line 169, in is_valid
    return self.is_bound and not self.errors
  File "/home/matthias/Projects/django/django/forms/forms.py", line 161, in errors
    self.full_clean()
  File "/home/matthias/Projects/django/django/forms/forms.py", line 372, in full_clean
    self._post_clean()
  File "/home/matthias/Projects/django/django/forms/models.py", line 400, in _post_clean
    self._update_errors(e)
  File "/home/matthias/Projects/django/django/forms/models.py", line 374, in _update_errors
    self.add_error(None, errors)
  File "/home/matthias/Projects/django/django/forms/forms.py", line 338, in add_error
    "'%s' has no field named '%s'." % (self.__class__.__name__, field))
ValueError: 'ModelForm' has no field named 'field2'.

This might be a thing for me to work on during the sprints. Also, https://github.com/matthiask/django/commit/2d03c38f888262e63c179b57c0c6bd09312f6f99

(Cc:ing Loïc because we discussed this at DUTH on wednesday evening.)

Change History (9)

comment:1 by Matthias Kestenholz, 8 years ago

Has patch: set
Owner: changed from nobody to Matthias Kestenholz
Status: newassigned

comment:2 by Simon Charette, 8 years ago

Triage Stage: UnreviewedAccepted
Version: 1.10master

I'm still not sure if we should discard the error or move it to NON_FIELD_ERRORS as suggested here but it shouldn't be crashing.

PR

comment:3 by Tim Graham, 8 years ago

Patch needs improvement: set

comment:4 by Matthias Kestenholz, 8 years ago

Patch needs improvement: unset

I hope the patch won't need further improvement, removing the flag for now. Thanks.

comment:5 by Matthias Kestenholz, 8 years ago

New pull request containing only a documentation patch:

PR

comment:6 by Matthias Kestenholz, 8 years ago

Triage Stage: AcceptedReady for checkin

IMO this is ready.

comment:7 by Tim Graham, 8 years ago

Component: FormsDocumentation
Patch needs improvement: set
Summary: Crashes during form validation when Model validation raises ValidationErrors for fields not in the current formDocument caveats of raising a ValidationError in Model.clean() for a field not in a model form
Triage Stage: Ready for checkinAccepted
Type: BugCleanup/optimization

I left a few ideas for improvement on the PR.

comment:8 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In e8c056c:

Fixed #27434 -- Doc'd how to raise a model validation error for a field not in a model form.

comment:9 by Tim Graham <timograham@…>, 8 years ago

In a1c6c220:

[1.11.x] Fixed #27434 -- Doc'd how to raise a model validation error for a field not in a model form.

Backport of e8c056c31a5b353e7b50a405c00db12c28f4a756 from master

Note: See TracTickets for help on using tickets.
Back to Top