Opened 6 years ago

Closed 2 years ago

Last modified 2 years ago

#11160 closed Bug (fixed)

Formset non_form_errors returns ErrorList() if is_valid is not called

Reported by: vbmendes Owned by: senko
Component: Forms Version: master
Severity: Normal Keywords: formset non_form_errors validation
Cc: vbmendes@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


If Formset.non_form_errors() is called before Formset.is_valid(), it will return an empty ErrorList.
Let's pretend we have two formsets in one view and we make this test: if formset1.is_valid() and formset2.is_valid(). If formset1 is not valid, formset2's is_valid() will not be called. It means that the user will have to fix all the formset1's validation erros before knowing that formset2 also has errors. It's annoying for the user.

I wrote a little patch to fix this issue. Just changing a little the Formset.non_form_errors() function.

Attachments (1)

formset_non_form_errors.diff (521 bytes) - added by vbmendes 6 years ago.
First patch

Download all attachments as: .zip

Change History (12)

Changed 6 years ago by vbmendes

First patch

comment:1 Changed 6 years ago by anonymous

  • Cc vbmendes@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 4 years ago by julien

  • Needs tests set
  • Severity set to Normal
  • Type set to Bug

comment:4 Changed 4 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:5 Changed 4 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

comment:6 Changed 2 years ago by senko

  • Owner changed from nobody to senko
  • Status changed from new to assigned

comment:7 Changed 2 years ago by senko

  • Needs tests unset

The patch "formset_non_form_errors.diff" doesn't do enough, because if non_form_errors() is called multiple times for any reasons, and there weren't any non-form errors, it will attempt the validation multiple times.

The fix I got in a pull request at is slightly different: full_clean() always sets the _non_form_errors (either to an empty error list, or to the actual errors if there are any), and non_field_errors() just check whether it needs to run full_clean() beforehand.

comment:8 Changed 2 years ago by tome

  • Triage Stage changed from Accepted to Ready for checkin

comment:9 Changed 2 years ago by tome

I have reviewed the solution, looks good as far as I can see.

comment:10 Changed 2 years ago by Senko Rasic <senko.rasic@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 493aca453aa94f160764d9a89c8043f7c9a67a78:

Fixed #11160 - Ensure full_clean is called from non_form_errors

Updated FormSet.non_form_errors() to ensure full_clean() has
been called before returning the errors.

comment:11 Changed 2 years ago by Andrew Godwin <andrew@…>

In 04e0fc029f6868221dbba850e5a8830c8d11c42c:

Merge pull request #1094 from senko/ticket_11160

Fixed #11160: Formset non_form_errors returns ErrorList() if is_valid is not called

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