Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[1.2.X] Fixed #14099 - BaseModelFormSet should use _should_delete_form

Thanks to kenth for the report and patch.

Backport of [15612] from trunk.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.2.X@15613 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit c326ec48d834eb8128ac20a37cb647fdfa769ae6 1 parent 4df0e5f
@spookylukey spookylukey authored
View
20 django/forms/models.py
@@ -16,7 +16,7 @@
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',
@@ -599,13 +599,10 @@ def save_existing_objects(self, commit=True):
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:
- self.deleted_objects.append(obj)
- obj.delete()
- continue
+ if self.can_delete and self._should_delete_form(form):
+ self.deleted_objects.append(obj)
+ obj.delete()
+ continue
if form.has_changed():
self.changed_objects.append((obj, form.changed_data))
saved_instances.append(self.save_existing(form, obj, commit=commit))
@@ -620,11 +617,8 @@ def save_new_objects(self, commit=True):
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:
- continue
+ if self.can_delete and self._should_delete_form(form):
+ continue
self.new_objects.append(self.save_new(form, commit=commit))
if not commit:
self.saved_forms.append(form)
View
124 tests/regressiontests/model_formsets_regress/tests.py
@@ -1,6 +1,7 @@
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
@@ -275,3 +276,124 @@ def test_modelformset_custom_callback(self):
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)
Please sign in to comment.
Something went wrong with that request. Please try again.