Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #18906 -- Ignored to-be-deleted forms in formset validate_unique

Thanks c.pollock at bangor.ac.uk for the report.
  • Loading branch information...
commit f44922c79516b9caf0e09fb060f20c896668b90f 1 parent db09a2d
@claudep claudep authored
View
5 django/forms/formsets.py
@@ -179,11 +179,10 @@ def cleaned_data(self):
@property
def deleted_forms(self):
"""
- Returns a list of forms that have been marked for deletion. Raises an
- AttributeError if deletion is not allowed.
+ Returns a list of forms that have been marked for deletion.
"""
if not self.is_valid() or not self.can_delete:
- raise AttributeError("'%s' object has no attribute 'deleted_forms'" % self.__class__.__name__)
+ return []
# construct _deleted_form_indexes which is just a list of form indexes
# that have had their deletion widget set to True
if not hasattr(self, '_deleted_form_indexes'):
View
19 django/forms/models.py
@@ -520,9 +520,9 @@ def validate_unique(self):
# Collect unique_checks and date_checks to run from all the forms.
all_unique_checks = set()
all_date_checks = set()
- for form in self.forms:
- if not form.is_valid():
- continue
+ forms_to_delete = self.deleted_forms
+ valid_forms = [form for form in self.forms if form.is_valid() and form not in forms_to_delete]
+ for form in valid_forms:
exclude = form._get_validation_exclusions()
unique_checks, date_checks = form.instance._get_unique_checks(exclude=exclude)
all_unique_checks = all_unique_checks.union(set(unique_checks))
@@ -532,9 +532,7 @@ def validate_unique(self):
# Do each of the unique checks (unique and unique_together)
for uclass, unique_check in all_unique_checks:
seen_data = set()
- for form in self.forms:
- if not form.is_valid():
- continue
+ for form in valid_forms:
# get data for each field of each of unique_check
row_data = tuple([form.cleaned_data[field] for field in unique_check if field in form.cleaned_data])
if row_data and not None in row_data:
@@ -554,9 +552,7 @@ def validate_unique(self):
for date_check in all_date_checks:
seen_data = set()
uclass, lookup, field, unique_for = date_check
- for form in self.forms:
- if not form.is_valid():
- continue
+ for form in valid_forms:
# see if we have data for both fields
if (form.cleaned_data and form.cleaned_data[field] is not None
and form.cleaned_data[unique_for] is not None):
@@ -611,10 +607,7 @@ def save_existing_objects(self, commit=True):
return []
saved_instances = []
- try:
- forms_to_delete = self.deleted_forms
- except AttributeError:
- forms_to_delete = []
+ forms_to_delete = self.deleted_forms
for form in self.initial_forms:
pk_name = self._pk_field.name
raw_pk_value = form._raw_value(pk_name)
View
22 tests/modeltests/model_formsets/tests.py
@@ -42,21 +42,29 @@ def test_add_form_deletion_when_invalid(self):
doesn't cause validation errors.
"""
PoetFormSet = modelformset_factory(Poet, can_delete=True)
+ poet = Poet.objects.create(name='test')
+ # One existing untouched and two new unvalid forms
data = {
- 'form-TOTAL_FORMS': '1',
- 'form-INITIAL_FORMS': '0',
+ 'form-TOTAL_FORMS': '3',
+ 'form-INITIAL_FORMS': '1',
'form-MAX_NUM_FORMS': '0',
- 'form-0-id': '',
- 'form-0-name': 'x' * 1000,
+ 'form-0-id': six.text_type(poet.id),
+ 'form-0-name': 'test',
+ 'form-1-id': '',
+ 'form-1-name': 'x' * 1000, # Too long
+ 'form-1-id': six.text_type(poet.id), # Violate unique constraint
+ 'form-1-name': 'test2',
}
formset = PoetFormSet(data, queryset=Poet.objects.all())
# Make sure this form doesn't pass validation.
self.assertEqual(formset.is_valid(), False)
- self.assertEqual(Poet.objects.count(), 0)
+ self.assertEqual(Poet.objects.count(), 1)
# Then make sure that it *does* pass validation and delete the object,
- # even though the data isn't actually valid.
+ # even though the data in new forms aren't actually valid.
data['form-0-DELETE'] = 'on'
+ data['form-1-DELETE'] = 'on'
+ data['form-2-DELETE'] = 'on'
formset = PoetFormSet(data, queryset=Poet.objects.all())
self.assertEqual(formset.is_valid(), True)
formset.save()
@@ -64,7 +72,7 @@ def test_add_form_deletion_when_invalid(self):
def test_change_form_deletion_when_invalid(self):
"""
- Make sure that an add form that is filled out, but marked for deletion
+ Make sure that a change form that is filled out, but marked for deletion
doesn't cause validation errors.
"""
PoetFormSet = modelformset_factory(Poet, can_delete=True)
Please sign in to comment.
Something went wrong with that request. Please try again.