Opened 5 years ago

Last modified 4 years ago

#13060 new Cleanup/optimization

ManagementForm exception in case of bad prefix should be easier to understand

Reported by: Karel Owned by: nobody
Component: Forms Version:
Severity: Normal Keywords: formset, ValidationError, ManagementForm
Cc: shawntherrien@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

If user adds multiple formsets with prefixes, and specifies the prefix incorrectly when binding the form and validating:

some_formset = SomeFormSet(request.POST, 'articles')

instead of:

some_formset = SomeFormSet(request.POST, prefix='articles') 

Django "suppresses" the original exception and raises only relatively unhelpful "ManagementForm data is missing or has been tampered with".

In file django/forms/formsets.py, line 57:

  54. if self.data or self.files:
  55.     form = ManagementForm(self.data, auto_id=self.auto_id, prefix=self.prefix)
  56.     if not form.is_valid():
  57.         raise ValidationError('ManagementForm data is missing or has been tampered with') 

Suggestion: include form._errors in output, because for such a small bug in code, it can take a really long time find it.

{'INITIAL_FORMS': [u'This field is required.'],
 'MAX_NUM_FORMS': [u'This field is required.'],
 'TOTAL_FORMS': [u'This field is required.']}

Attachments (1)

formsets.diff (710 bytes) - added by stherrien 5 years ago.
validation error with _errors included

Download all attachments as: .zip

Change History (8)

Changed 5 years ago by stherrien

validation error with _errors included

comment:1 Changed 5 years ago by anonymous

  • Cc shawntherrien@… added
  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Not sure I completely agree with the suggested fix, but yes -- the error on failure of the management form could be a little more helpful, at the very least naming the field that is missing or invalid.

comment:3 Changed 4 years ago by lukeplant

  • Type set to New feature

comment:4 Changed 4 years ago by lukeplant

  • Severity set to Normal

comment:5 Changed 4 years ago by julien

  • Needs tests set
  • Patch needs improvement set
  • Type changed from New feature to Cleanup/optimization

Patch would need improvement as per Russell's comment. Also needs tests.

comment:6 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:7 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

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