Fixed #14099 - BaseModelFormSet should use _should_delete_form (13f9fd38) · Commits · Dom Sekotill / django

django/forms/models.py

+7 −13

Original line number	Diff line number	Diff line
		@@ -17,7 +17,7 @@ from forms import BaseForm, get_declared_fields
		from fields import Field, ChoiceField
		from widgets import SelectMultiple, HiddenInput, MultipleHiddenInput
		from widgets import media_property
		from formsets import BaseFormSet, formset_factory, DELETION_FIELD_NAME
		from formsets import BaseFormSet, formset_factory

		__all__ = (
		'ModelForm', 'BaseModelForm', 'model_to_dict', 'fields_for_model',
		@@ -588,10 +588,7 @@ class BaseModelFormSet(BaseFormSet):
		pk_value = getattr(pk_value, 'pk', pk_value)

		obj = self._existing_object(pk_value)
		if self.can_delete:
		raw_delete_value = form._raw_value(DELETION_FIELD_NAME)
		should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value)
		if should_delete:
		if self.can_delete and self._should_delete_form(form):
		self.deleted_objects.append(obj)
		obj.delete()
		continue
		@@ -609,10 +606,7 @@ class BaseModelFormSet(BaseFormSet):
		continue
		# If someone has marked an add form for deletion, don't save the
		# object.
		if self.can_delete:
		raw_delete_value = form._raw_value(DELETION_FIELD_NAME)
		should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value)
		if should_delete:
		if self.can_delete and self._should_delete_form(form):
		continue
		self.new_objects.append(self.save_new(form, commit=commit))
		if not commit:

tests/regressiontests/model_formsets_regress/tests.py

+123 −1

Original line number	Diff line number	Diff line
		from django import forms
		from django.forms.formsets import BaseFormSet, DELETION_FIELD_NAME
		from django.forms.util import ErrorDict, ErrorList
		from django.forms.models import modelform_factory, inlineformset_factory, modelformset_factory
		from django.forms.models import modelform_factory, inlineformset_factory, modelformset_factory, BaseModelFormSet
		from django.test import TestCase

		from models import User, UserSite, Restaurant, Manager, Network, Host
		@@ -283,3 +284,124 @@ class FormfieldCallbackTests(TestCase):
		modelformset_factory(UserSite, form=UserSiteForm,
		formfield_callback=callback)
		self.assertCallbackCalled(callback)


		class BaseCustomDeleteFormSet(BaseFormSet):
		"""
		A formset mix-in that lets a form decide if it's to be deleted.
		Works for BaseFormSets. Also works for ModelFormSets with #14099 fixed.

		form.should_delete() is called. The formset delete field is also suppressed.
		"""
		def add_fields(self, form, index):
		super(BaseCustomDeleteFormSet, self).add_fields(form, index)
		self.can_delete = True
		if DELETION_FIELD_NAME in form.fields:
		del form.fields[DELETION_FIELD_NAME]

		def _should_delete_form(self, form):
		return hasattr(form, 'should_delete') and form.should_delete()


		class FormfieldShouldDeleteFormTests(TestCase):
		"""
		Regression for #14099: BaseModelFormSet should use ModelFormSet method _should_delete_form
		"""

		class BaseCustomDeleteModelFormSet(BaseModelFormSet, BaseCustomDeleteFormSet):
		""" Model FormSet with CustomDelete MixIn """

		class CustomDeleteUserForm(forms.ModelForm):
		""" A model form with a 'should_delete' method """
		class Meta:
		model = User

		def should_delete(self):
		""" delete form if odd PK """
		return self.instance.id % 2 != 0

		NormalFormset = modelformset_factory(User, form=CustomDeleteUserForm, can_delete=True)
		DeleteFormset = modelformset_factory(User, form=CustomDeleteUserForm, formset=BaseCustomDeleteModelFormSet)

		data = {
		'form-TOTAL_FORMS': '4',
		'form-INITIAL_FORMS': '0',
		'form-MAX_NUM_FORMS': '4',
		'form-0-username': 'John',
		'form-0-serial': '1',
		'form-1-username': 'Paul',
		'form-1-serial': '2',
		'form-2-username': 'George',
		'form-2-serial': '3',
		'form-3-username': 'Ringo',
		'form-3-serial': '5',
		}

		bound_ids = {
		'form-INITIAL_FORMS': '4',
		'form-0-id': '1',
		'form-1-id': '2',
		'form-2-id': '3',
		'form-3-id': '4',
		}

		delete_all_ids = {
		'form-0-DELETE': '1',
		'form-1-DELETE': '1',
		'form-2-DELETE': '1',
		'form-3-DELETE': '1',
		}

		def test_init_database(self):
		""" Add test data to database via formset """
		formset = self.NormalFormset(self.data)
		self.assertTrue(formset.is_valid())
		self.assertEqual(len(formset.save()), 4)

		def test_no_delete(self):
		""" Verify base formset doesn't modify database """
		# reload database
		self.test_init_database()

		# pass standard data dict & see none updated
		data = dict(self.data)
		data.update(self.bound_ids)
		formset = self.NormalFormset(data, queryset=User.objects.all())
		self.assertTrue(formset.is_valid())
		self.assertEqual(len(formset.save()), 0)
		self.assertEqual(len(User.objects.all()), 4)

		def test_all_delete(self):
		""" Verify base formset honors DELETE field """
		# reload database
		self.test_init_database()

		# create data dict with all fields marked for deletion
		data = dict(self.data)
		data.update(self.bound_ids)
		data.update(self.delete_all_ids)
		formset = self.NormalFormset(data, queryset=User.objects.all())
		self.assertTrue(formset.is_valid())
		self.assertEqual(len(formset.save()), 0)
		self.assertEqual(len(User.objects.all()), 0)

		def test_custom_delete(self):
		""" Verify DeleteFormset ignores DELETE field and uses form method """
		# reload database
		self.test_init_database()

		# Create formset with custom Delete function
		# create data dict with all fields marked for deletion
		data = dict(self.data)
		data.update(self.bound_ids)
		data.update(self.delete_all_ids)
		formset = self.DeleteFormset(data, queryset=User.objects.all())

		# verify two were deleted
		self.assertTrue(formset.is_valid())
		self.assertEqual(len(formset.save()), 0)
		self.assertEqual(len(User.objects.all()), 2)

		# verify no "odd" PKs left
		odd_ids = [user.id for user in User.objects.all() if user.id % 2]
		self.assertEqual(len(odd_ids), 0)