From 13f9fd38dc299a682c61e175c83070a7c5a84103 Mon Sep 17 00:00:00 2001 From: Luke Plant Date: Mon, 21 Feb 2011 14:23:02 +0000 Subject: [PATCH] Fixed #14099 - BaseModelFormSet should use _should_delete_form Thanks to kenth for the report and patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@15612 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/forms/models.py | 20 +-- .../model_formsets_regress/tests.py | 124 +++++++++++++++++- 2 files changed, 130 insertions(+), 14 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 912bdf36a8..1356c9f517 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -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,13 +588,10 @@ 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: - 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)) @@ -609,11 +606,8 @@ 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: - 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) diff --git a/tests/regressiontests/model_formsets_regress/tests.py b/tests/regressiontests/model_formsets_regress/tests.py index 3354fc5b7f..eb78a03ac3 100644 --- a/tests/regressiontests/model_formsets_regress/tests.py +++ b/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 @@ -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)