Fixed #18906 -- Ignored to-be-deleted forms in formset validate_unique (f44922c7) · Commits · Dom Sekotill / django

django/forms/formsets.py

+2 −3

Original line number	Diff line number	Diff line
		@@ -179,11 +179,10 @@ class BaseFormSet(object):
		@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'):

django/forms/models.py

+6 −13

Original line number	Diff line number	Diff line
		@@ -520,9 +520,9 @@ class BaseModelFormSet(BaseFormSet):
		# 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 @@ class BaseModelFormSet(BaseFormSet):
		# 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 @@ class BaseModelFormSet(BaseFormSet):
		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 @@ class BaseModelFormSet(BaseFormSet):
		return []

		saved_instances = []
		try:
		forms_to_delete = self.deleted_forms
		except AttributeError:
		forms_to_delete = []
		for form in self.initial_forms:
		pk_name = self._pk_field.name
		raw_pk_value = form._raw_value(pk_name)

tests/modeltests/model_formsets/tests.py

+15 −7

Original line number	Diff line number	Diff line
		@@ -42,21 +42,29 @@ class DeletionTests(TestCase):
		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 @@ class DeletionTests(TestCase):

		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)