From 15becf23a9e4c9b230745738d2d42f6ab8f0f031 Mon Sep 17 00:00:00 2001 From: Joseph Kocherhans Date: Tue, 31 Mar 2009 19:55:20 +0000 Subject: [PATCH] Forms in model formsets and inline formsets can now be deleted even if they don't validate. Related to #9587. git-svn-id: http://code.djangoproject.com/svn/django/trunk@10283 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/forms/forms.py | 9 +++ django/forms/formsets.py | 5 +- django/forms/models.py | 33 +++++--- tests/modeltests/model_formsets/tests.py | 70 +++++++++++++++++ .../regressiontests/inline_formsets/models.py | 13 ++++ .../regressiontests/inline_formsets/tests.py | 76 +++++++++++++++++++ 6 files changed, 191 insertions(+), 15 deletions(-) create mode 100644 tests/modeltests/model_formsets/tests.py create mode 100644 tests/regressiontests/inline_formsets/tests.py diff --git a/django/forms/forms.py b/django/forms/forms.py index f5bea10978..ef9d657657 100644 --- a/django/forms/forms.py +++ b/django/forms/forms.py @@ -205,6 +205,15 @@ class BaseForm(StrAndUnicode): """ return self.errors.get(NON_FIELD_ERRORS, self.error_class()) + def _raw_value(self, fieldname): + """ + Returns the raw_value for a particular field name. This is just a + convenient wrapper around widget.value_from_datadict. + """ + field = self.fields[fieldname] + prefix = self.add_prefix(fieldname) + return field.widget.value_from_datadict(self.data, self.files, prefix) + def full_clean(self): """ Cleans all of self.data and populates self._errors and diff --git a/django/forms/formsets.py b/django/forms/formsets.py index 3421b56e8c..66ccd6d08d 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -228,9 +228,8 @@ class BaseFormSet(StrAndUnicode): # more code than we'd like, but the form's cleaned_data will # not exist if the form is invalid. field = form.fields[DELETION_FIELD_NAME] - prefix = form.add_prefix(DELETION_FIELD_NAME) - value = field.widget.value_from_datadict(self.data, self.files, prefix) - should_delete = field.clean(value) + raw_value = form._raw_value(DELETION_FIELD_NAME) + should_delete = field.clean(raw_value) if should_delete: # This form is going to be deleted so any of its errors # should not cause the entire formset to be invalid. diff --git a/django/forms/models.py b/django/forms/models.py index 70f0631e2c..d50a4705e8 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -425,16 +425,22 @@ class BaseModelFormSet(BaseFormSet): existing_objects[obj.pk] = obj saved_instances = [] for form in self.initial_forms: - obj = existing_objects[form.cleaned_data[self._pk_field.name].pk] - if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]: - self.deleted_objects.append(obj) - obj.delete() - else: - if form.changed_data: - self.changed_objects.append((obj, form.changed_data)) - saved_instances.append(self.save_existing(form, obj, commit=commit)) - if not commit: - self.saved_forms.append(form) + pk_name = self._pk_field.name + raw_pk_value = form._raw_value(pk_name) + pk_value = form.fields[pk_name].clean(raw_pk_value).pk + obj = existing_objects[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 form.changed_data: + self.changed_objects.append((obj, form.changed_data)) + saved_instances.append(self.save_existing(form, obj, commit=commit)) + if not commit: + self.saved_forms.append(form) return saved_instances def save_new_objects(self, commit=True): @@ -444,8 +450,11 @@ class BaseModelFormSet(BaseFormSet): continue # If someone has marked an add form for deletion, don't save the # object. - if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]: - continue + 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 self.new_objects.append(self.save_new(form, commit=commit)) if not commit: self.saved_forms.append(form) diff --git a/tests/modeltests/model_formsets/tests.py b/tests/modeltests/model_formsets/tests.py new file mode 100644 index 0000000000..23e8a0c01e --- /dev/null +++ b/tests/modeltests/model_formsets/tests.py @@ -0,0 +1,70 @@ +from django.test import TestCase +from django.forms.models import modelformset_factory +from modeltests.model_formsets.models import Poet, Poem + +class DeletionTests(TestCase): + def test_deletion(self): + PoetFormSet = modelformset_factory(Poet, can_delete=True) + poet = Poet.objects.create(name='test') + data = { + 'form-TOTAL_FORMS': u'1', + 'form-INITIAL_FORMS': u'1', + 'form-0-id': u'1', + 'form-0-name': u'test', + 'form-0-DELETE': u'on', + } + formset = PoetFormSet(data, queryset=Poet.objects.all()) + formset.save() + self.assertTrue(formset.is_valid()) + self.assertEqual(Poet.objects.count(), 0) + + def test_add_form_deletion_when_invalid(self): + """ + Make sure that an add form that is filled out, but marked for deletion + doesn't cause validation errors. + """ + PoetFormSet = modelformset_factory(Poet, can_delete=True) + data = { + 'form-TOTAL_FORMS': u'1', + 'form-INITIAL_FORMS': u'0', + 'form-0-id': u'', + 'form-0-name': u'x' * 1000, + } + 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) + + # Then make sure that it *does* pass validation and delete the object, + # even though the data isn't actually valid. + data['form-0-DELETE'] = 'on' + formset = PoetFormSet(data, queryset=Poet.objects.all()) + self.assertEqual(formset.is_valid(), True) + formset.save() + self.assertEqual(Poet.objects.count(), 0) + + def test_change_form_deletion_when_invalid(self): + """ + Make sure that an add form that is filled out, but marked for deletion + doesn't cause validation errors. + """ + PoetFormSet = modelformset_factory(Poet, can_delete=True) + poet = Poet.objects.create(name='test') + data = { + 'form-TOTAL_FORMS': u'1', + 'form-INITIAL_FORMS': u'1', + 'form-0-id': u'1', + 'form-0-name': u'x' * 1000, + } + 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(), 1) + + # Then make sure that it *does* pass validation and delete the object, + # even though the data isn't actually valid. + data['form-0-DELETE'] = 'on' + formset = PoetFormSet(data, queryset=Poet.objects.all()) + self.assertEqual(formset.is_valid(), True) + formset.save() + self.assertEqual(Poet.objects.count(), 0) diff --git a/tests/regressiontests/inline_formsets/models.py b/tests/regressiontests/inline_formsets/models.py index 8b6f5dd342..9b1f8b4932 100644 --- a/tests/regressiontests/inline_formsets/models.py +++ b/tests/regressiontests/inline_formsets/models.py @@ -13,6 +13,19 @@ class Child(models.Model): school = models.ForeignKey(School) name = models.CharField(max_length=100) +class Poet(models.Model): + name = models.CharField(max_length=100) + + def __unicode__(self): + return self.name + +class Poem(models.Model): + poet = models.ForeignKey(Poet) + name = models.CharField(max_length=100) + + def __unicode__(self): + return self.name + __test__ = {'API_TESTS': """ >>> from django.forms.models import inlineformset_factory diff --git a/tests/regressiontests/inline_formsets/tests.py b/tests/regressiontests/inline_formsets/tests.py new file mode 100644 index 0000000000..5cd5209b25 --- /dev/null +++ b/tests/regressiontests/inline_formsets/tests.py @@ -0,0 +1,76 @@ +from django.test import TestCase +from django.forms.models import inlineformset_factory +from regressiontests.inline_formsets.models import Poet, Poem + +class DeletionTests(TestCase): + def test_deletion(self): + PoemFormSet = inlineformset_factory(Poet, Poem, can_delete=True) + poet = Poet.objects.create(name='test') + poet.poem_set.create(name='test poem') + data = { + 'poem_set-TOTAL_FORMS': u'1', + 'poem_set-INITIAL_FORMS': u'1', + 'poem_set-0-id': u'1', + 'poem_set-0-poem': u'1', + 'poem_set-0-name': u'test', + 'poem_set-0-DELETE': u'on', + } + formset = PoemFormSet(data, instance=poet) + formset.save() + self.assertTrue(formset.is_valid()) + self.assertEqual(Poem.objects.count(), 0) + + def test_add_form_deletion_when_invalid(self): + """ + Make sure that an add form that is filled out, but marked for deletion + doesn't cause validation errors. + """ + PoemFormSet = inlineformset_factory(Poet, Poem, can_delete=True) + poet = Poet.objects.create(name='test') + data = { + 'poem_set-TOTAL_FORMS': u'1', + 'poem_set-INITIAL_FORMS': u'0', + 'poem_set-0-id': u'', + 'poem_set-0-poem': u'1', + 'poem_set-0-name': u'x' * 1000, + } + formset = PoemFormSet(data, instance=poet) + # Make sure this form doesn't pass validation. + self.assertEqual(formset.is_valid(), False) + self.assertEqual(Poem.objects.count(), 0) + + # Then make sure that it *does* pass validation and delete the object, + # even though the data isn't actually valid. + data['poem_set-0-DELETE'] = 'on' + formset = PoemFormSet(data, instance=poet) + self.assertEqual(formset.is_valid(), True) + formset.save() + self.assertEqual(Poem.objects.count(), 0) + + def test_change_form_deletion_when_invalid(self): + """ + Make sure that a change form that is filled out, but marked for deletion + doesn't cause validation errors. + """ + PoemFormSet = inlineformset_factory(Poet, Poem, can_delete=True) + poet = Poet.objects.create(name='test') + poet.poem_set.create(name='test poem') + data = { + 'poem_set-TOTAL_FORMS': u'1', + 'poem_set-INITIAL_FORMS': u'1', + 'poem_set-0-id': u'1', + 'poem_set-0-poem': u'1', + 'poem_set-0-name': u'x' * 1000, + } + formset = PoemFormSet(data, instance=poet) + # Make sure this form doesn't pass validation. + self.assertEqual(formset.is_valid(), False) + self.assertEqual(Poem.objects.count(), 1) + + # Then make sure that it *does* pass validation and delete the object, + # even though the data isn't actually valid. + data['poem_set-0-DELETE'] = 'on' + formset = PoemFormSet(data, instance=poet) + self.assertEqual(formset.is_valid(), True) + formset.save() + self.assertEqual(Poem.objects.count(), 0)