Opened 14 years ago

Closed 6 years ago

Last modified 6 years ago

#14009 closed Bug (fixed)

custom formset validation documentation is incomplete

Reported by: splatEric <mike@…> Owned by: Tobias Kunze
Component: Documentation Version: dev
Severity: Normal Keywords: formset validation
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The example for performing custom formset validation is incomplete. It does not take into account deleted forms, or empty forms for checking the unique titles constraint. It should read something along the lines of (changes highlighted by *>):

>>> class BaseArticleFormSet(BaseFormSet):
...     def clean(self):
...         """Checks that no two articles have the same title."""
...         if any(self.errors):
...             # Don't bother validating the formset unless each form is valid on its own
...             return
...         titles = []
...         for i in range(0, self.total_form_count()):
*>             if self.can_delete and self._should_delete_form(form):
*>                continue
...             form = self.forms[i]
*>             if 'title' in form.cleaned_data:
...                 title = form.cleaned_data['title']
...                 if title in titles:
...                     raise forms.ValidationError, "Articles in a set must have distinct titles."
...                 titles.append(title)

Change History (10)

comment:1 by Gabriel Hurley, 14 years ago

Has patch: unset
Triage Stage: UnreviewedAccepted

Accepted. For future reference, the file in question is docs/topics/forms/formsets/.

comment:2 by leonelfreire, 14 years ago

Severity: Normal
Type: Uncategorized

Hi,

I think there's an "error" with the above suggestion too. self.errors will contain the errors of all forms (even if the form is marked for deletion). Supposing all NOT DELETED forms are valid, if some deleted form is in an invalid state, the code:

if any(self.errors):
	return

will make the "clean" method return too soon, avoiding the execution of the validation of the NOT DELETED forms.

AFAIK, formset.is_valid() can be True, even if formset.errors contains forms errors (from deleted forms).

In my case, I did something like:

(...)
def clean(self)
	if self.is_valid():
		try:
			# Get only the forms not marked for deletion
			cleaned_forms = list(set(self.forms) - set(self.deleted_forms))
		except AttributError: # There're no forms marked for deletion (no self.deleted_forms)
			cleaned_forms = self.forms
	
		# validate logic with cleaned_forms
		(...)

If there's a better way to do this, let me know.

comment:3 by Julien Phalip, 14 years ago

Type: UncategorizedBug

comment:4 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:5 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:6 by Tobias Kunze, 6 years ago

Owner: changed from nobody to Tobias Kunze
Status: newassigned

comment:7 by Tobias Kunze, 6 years ago

Has patch: set

comment:8 by Mariusz Felisiak, 6 years ago

Version: 1.2master

comment:9 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In d610521b:

Fixed #14009 -- Fixed custom formset validation example in docs.

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

In c0dc49a7:

[2.2.x] Fixed #14009 -- Fixed custom formset validation example in docs.

Backport of d610521bffe9d44a070ebe3a719b474aff6d3d1e from master

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