From f259494f827b24228bbbadb1d118a09f8e6329ac Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 7 May 2009 12:17:52 +0000 Subject: [PATCH] Fixed #9493 -- Corrected error handling of formsets that violate unique constraints across the component forms. Thanks to Alex Gaynor for the patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@10682 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/forms/models.py | 136 +++++++++++++++++--- docs/topics/forms/modelforms.txt | 83 ++++++++---- tests/modeltests/model_formsets/models.py | 148 +++++++++++++++++++++- 3 files changed, 316 insertions(+), 51 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 910253c188..96ce06519f 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -6,10 +6,10 @@ and database field objects. from django.utils.encoding import smart_unicode, force_unicode from django.utils.datastructures import SortedDict from django.utils.text import get_text_list, capfirst -from django.utils.translation import ugettext_lazy as _ +from django.utils.translation import ugettext_lazy as _, ugettext from util import ValidationError, ErrorList -from forms import BaseForm, get_declared_fields +from forms import BaseForm, get_declared_fields, NON_FIELD_ERRORS from fields import Field, ChoiceField, IntegerField, EMPTY_VALUES from widgets import Select, SelectMultiple, HiddenInput, MultipleHiddenInput from widgets import media_property @@ -231,6 +231,26 @@ class BaseModelForm(BaseForm): return self.cleaned_data def validate_unique(self): + unique_checks, date_checks = self._get_unique_checks() + form_errors = [] + bad_fields = set() + + field_errors, global_errors = self._perform_unique_checks(unique_checks) + bad_fields.union(field_errors) + form_errors.extend(global_errors) + + field_errors, global_errors = self._perform_date_checks(date_checks) + bad_fields.union(field_errors) + form_errors.extend(global_errors) + + for field_name in bad_fields: + del self.cleaned_data[field_name] + if form_errors: + # Raise the unique together errors since they are considered + # form-wide. + raise ValidationError(form_errors) + + def _get_unique_checks(self): from django.db.models.fields import FieldDoesNotExist, Field as ModelField # Gather a list of checks to perform. We only perform unique checks @@ -271,24 +291,8 @@ class BaseModelForm(BaseForm): date_checks.append(('year', name, f.unique_for_year)) if f.unique_for_month and self.cleaned_data.get(f.unique_for_month) is not None: date_checks.append(('month', name, f.unique_for_month)) + return unique_checks, date_checks - form_errors = [] - bad_fields = set() - - field_errors, global_errors = self._perform_unique_checks(unique_checks) - bad_fields.union(field_errors) - form_errors.extend(global_errors) - - field_errors, global_errors = self._perform_date_checks(date_checks) - bad_fields.union(field_errors) - form_errors.extend(global_errors) - - for field_name in bad_fields: - del self.cleaned_data[field_name] - if form_errors: - # Raise the unique together errors since they are considered - # form-wide. - raise ValidationError(form_errors) def _perform_unique_checks(self, unique_checks): bad_fields = set() @@ -504,6 +508,96 @@ class BaseModelFormSet(BaseFormSet): self.save_m2m = save_m2m return self.save_existing_objects(commit) + self.save_new_objects(commit) + def clean(self): + self.validate_unique() + + def validate_unique(self): + # Iterate over the forms so that we can find one with potentially valid + # data from which to extract the error checks + for form in self.forms: + if hasattr(form, 'cleaned_data'): + break + else: + return + unique_checks, date_checks = form._get_unique_checks() + errors = [] + # Do each of the unique checks (unique and unique_together) + for unique_check in unique_checks: + seen_data = set() + for form in self.forms: + # if the form doesn't have cleaned_data then we ignore it, + # it's already invalid + if not hasattr(form, "cleaned_data"): + continue + # get each of the fields for which we have data on this form + if [f for f in unique_check if f in form.cleaned_data and form.cleaned_data[f] is not None]: + # get the data itself + row_data = tuple([form.cleaned_data[field] for field in unique_check]) + # if we've aready seen it then we have a uniqueness failure + if row_data in seen_data: + # poke error messages into the right places and mark + # the form as invalid + errors.append(self.get_unique_error_message(unique_check)) + form._errors[NON_FIELD_ERRORS] = self.get_form_error() + del form.cleaned_data + break + # mark the data as seen + seen_data.add(row_data) + # iterate over each of the date checks now + for date_check in date_checks: + seen_data = set() + lookup, field, unique_for = date_check + for form in self.forms: + # if the form doesn't have cleaned_data then we ignore it, + # it's already invalid + if not hasattr(self, 'cleaned_data'): + continue + # 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): + # if it's a date lookup we need to get the data for all the fields + if lookup == 'date': + date = form.cleaned_data[unique_for] + date_data = (date.year, date.month, date.day) + # otherwise it's just the attribute on the date/datetime + # object + else: + date_data = (getattr(form.cleaned_data[unique_for], lookup),) + data = (form.cleaned_data[field],) + date_data + # if we've aready seen it then we have a uniqueness failure + if data in seen_data: + # poke error messages into the right places and mark + # the form as invalid + errors.append(self.get_date_error_message(date_check)) + form._errors[NON_FIELD_ERRORS] = self.get_form_error() + del form.cleaned_data + break + seen_data.add(data) + if errors: + raise ValidationError(errors) + + def get_unique_error_message(self, unique_check): + if len(unique_check) == 1: + return ugettext("Please correct the duplicate data for %(field)s.") % { + "field": unique_check[0], + } + else: + return ugettext("Please correct the duplicate data for %(field)s, " + "which must be unique.") % { + "field": get_text_list(unique_check, _("and")), + } + + def get_date_error_message(self, date_check): + return ugettext("Please correct the duplicate data for %(field_name)s " + "which must be unique for the %(lookup)s in %(date_field)s.") % { + 'field_name': date_check[1], + 'date_field': date_check[2], + 'lookup': unicode(date_check[0]), + } + + def get_form_error(self): + return ugettext("Please correct the duplicate values below.") + def save_existing_objects(self, commit=True): self.changed_objects = [] self.deleted_objects = [] @@ -657,6 +751,10 @@ class BaseInlineFormSet(BaseModelFormSet): label=getattr(form.fields.get(self.fk.name), 'label', capfirst(self.fk.verbose_name)) ) + def get_unique_error_message(self, unique_check): + unique_check = [field for field in unique_check if field != self.fk.name] + return super(BaseInlineFormSet, self).get_unique_error_message(unique_check) + def _get_foreign_key(parent_model, model, fk_name=None): """ Finds and returns the ForeignKey from model to parent if there is one. diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt index be67a38b6f..8ccf9b0cfc 100644 --- a/docs/topics/forms/modelforms.txt +++ b/docs/topics/forms/modelforms.txt @@ -45,61 +45,61 @@ the full list of conversions: Model field Form field =============================== ======================================== ``AutoField`` Not represented in the form - + ``BooleanField`` ``BooleanField`` - + ``CharField`` ``CharField`` with ``max_length`` set to the model field's ``max_length`` - + ``CommaSeparatedIntegerField`` ``CharField`` - + ``DateField`` ``DateField`` - + ``DateTimeField`` ``DateTimeField`` - + ``DecimalField`` ``DecimalField`` - + ``EmailField`` ``EmailField`` - + ``FileField`` ``FileField`` - + ``FilePathField`` ``CharField`` - + ``FloatField`` ``FloatField`` - + ``ForeignKey`` ``ModelChoiceField`` (see below) - + ``ImageField`` ``ImageField`` - + ``IntegerField`` ``IntegerField`` - + ``IPAddressField`` ``IPAddressField`` - + ``ManyToManyField`` ``ModelMultipleChoiceField`` (see below) - + ``NullBooleanField`` ``CharField`` - + ``PhoneNumberField`` ``USPhoneNumberField`` (from ``django.contrib.localflavor.us``) - + ``PositiveIntegerField`` ``IntegerField`` - + ``PositiveSmallIntegerField`` ``IntegerField`` - + ``SlugField`` ``SlugField`` - + ``SmallIntegerField`` ``IntegerField`` - - ``TextField`` ``CharField`` with + + ``TextField`` ``CharField`` with ``widget=forms.Textarea`` - + ``TimeField`` ``TimeField`` - + ``URLField`` ``URLField`` with ``verify_exists`` set to the model field's ``verify_exists`` - - ``XMLField`` ``CharField`` with + + ``XMLField`` ``CharField`` with ``widget=forms.Textarea`` =============================== ======================================== @@ -487,7 +487,7 @@ queryset that includes all objects in the model (e.g., Alternatively, you can create a subclass that sets ``self.queryset`` in ``__init__``:: - + from django.forms.models import BaseModelFormSet class BaseAuthorFormSet(BaseModelFormSet): @@ -515,6 +515,22 @@ exclude:: .. _saving-objects-in-the-formset: +Overriding clean() method +------------------------- + +You can override the ``clean()`` method to provide custom validation to +the whole formset at once. By default, the ``clean()`` method will validate +that none of the data in the formsets violate the unique constraints on your +model (both field ``unique`` and model ``unique_together``). To maintain this +default behavior be sure you call the parent's ``clean()`` method:: + + class MyModelFormSet(BaseModelFormSet): + def clean(self): + super(MyModelFormSet, self).clean() + # example custom validation across forms in the formset: + for form in self.forms: + # your custom formset validation + Saving objects in the formset ----------------------------- @@ -599,6 +615,17 @@ than that of a "normal" formset. The only difference is that we call ``formset.save()`` to save the data into the database. (This was described above, in :ref:`saving-objects-in-the-formset`.) + +Overiding ``clean()`` on a ``model_formset`` +-------------------------------------------- + +Just like with ``ModelForms``, by default the ``clean()`` method of a +``model_formset`` will validate that none of the items in the formset validate +the unique constraints on your model(either unique or unique_together). If you +want to overide the ``clean()`` method on a ``model_formset`` and maintain this +validation, you must call the parent classes ``clean`` method. + + Using a custom queryset ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/modeltests/model_formsets/models.py b/tests/modeltests/model_formsets/models.py index f30b2125e2..cda3aa9d8e 100644 --- a/tests/modeltests/model_formsets/models.py +++ b/tests/modeltests/model_formsets/models.py @@ -23,6 +23,12 @@ class Book(models.Model): author = models.ForeignKey(Author) title = models.CharField(max_length=100) + class Meta: + unique_together = ( + ('author', 'title'), + ) + ordering = ['id'] + def __unicode__(self): return self.title @@ -58,7 +64,7 @@ class CustomPrimaryKey(models.Model): class Place(models.Model): name = models.CharField(max_length=50) city = models.CharField(max_length=50) - + def __unicode__(self): return self.name @@ -85,7 +91,7 @@ class OwnerProfile(models.Model): class Restaurant(Place): serves_pizza = models.BooleanField() - + def __unicode__(self): return self.name @@ -166,6 +172,15 @@ class Poem(models.Model): def __unicode__(self): return self.name +class Post(models.Model): + title = models.CharField(max_length=50, unique_for_date='posted', blank=True) + slug = models.CharField(max_length=50, unique_for_year='posted', blank=True) + subtitle = models.CharField(max_length=50, unique_for_month='posted', blank=True) + posted = models.DateField() + + def __unicode__(self): + return self.name + __test__ = {'API_TESTS': """ >>> from datetime import date @@ -573,7 +588,7 @@ True ... print book.title Les Fleurs du Mal -Test inline formsets where the inline-edited object uses multi-table inheritance, thus +Test inline formsets where the inline-edited object uses multi-table inheritance, thus has a non AutoField yet auto-created primary key. >>> AuthorBooksFormSet3 = inlineformset_factory(Author, AlternateBook, can_delete=False, extra=1) @@ -740,7 +755,7 @@ True >>> formset.save() [] -# ForeignKey with unique=True should enforce max_num=1 +# ForeignKey with unique=True should enforce max_num=1 >>> FormSet = inlineformset_factory(Place, Location, can_delete=False) >>> formset = FormSet(instance=place) @@ -943,4 +958,129 @@ True >>> FormSet = modelformset_factory(ClassyMexicanRestaurant, fields=["tacos_are_yummy"]) >>> sorted(FormSet().forms[0].fields.keys()) ['restaurant', 'tacos_are_yummy'] + +# Prevent duplicates from within the same formset +>>> FormSet = modelformset_factory(Product, extra=2) +>>> data = { +... 'form-TOTAL_FORMS': 2, +... 'form-INITIAL_FORMS': 0, +... 'form-0-slug': 'red_car', +... 'form-1-slug': 'red_car', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset._non_form_errors +[u'Please correct the duplicate data for slug.'] + +>>> FormSet = modelformset_factory(Price, extra=2) +>>> data = { +... 'form-TOTAL_FORMS': 2, +... 'form-INITIAL_FORMS': 0, +... 'form-0-price': '25', +... 'form-0-quantity': '7', +... 'form-1-price': '25', +... 'form-1-quantity': '7', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset._non_form_errors +[u'Please correct the duplicate data for price and quantity, which must be unique.'] + +# only the price field is specified, this should skip any unique checks since the unique_together is not fulfilled. +# this will fail with a KeyError if broken. +>>> FormSet = modelformset_factory(Price, fields=("price",), extra=2) +>>> data = { +... 'form-TOTAL_FORMS': '2', +... 'form-INITIAL_FORMS': '0', +... 'form-0-price': '24', +... 'form-1-price': '24', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +True + +>>> FormSet = inlineformset_factory(Author, Book, extra=0) +>>> author = Author.objects.order_by('id')[0] +>>> book_ids = author.book_set.values_list('id', flat=True) +>>> data = { +... 'book_set-TOTAL_FORMS': '2', +... 'book_set-INITIAL_FORMS': '2', +... +... 'book_set-0-title': 'The 2008 Election', +... 'book_set-0-author': str(author.id), +... 'book_set-0-id': str(book_ids[0]), +... +... 'book_set-1-title': 'The 2008 Election', +... 'book_set-1-author': str(author.id), +... 'book_set-1-id': str(book_ids[1]), +... } +>>> formset = FormSet(data=data, instance=author) +>>> formset.is_valid() +False +>>> formset._non_form_errors +[u'Please correct the duplicate data for title.'] +>>> formset.errors +[{}, {'__all__': u'Please correct the duplicate values below.'}] + +>>> FormSet = modelformset_factory(Post, extra=2) +>>> data = { +... 'form-TOTAL_FORMS': '2', +... 'form-INITIAL_FORMS': '0', +... +... 'form-0-title': 'blah', +... 'form-0-slug': 'Morning', +... 'form-0-subtitle': 'foo', +... 'form-0-posted': '2009-01-01', +... 'form-1-title': 'blah', +... 'form-1-slug': 'Morning in Prague', +... 'form-1-subtitle': 'rawr', +... 'form-1-posted': '2009-01-01' +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset._non_form_errors +[u'Please correct the duplicate data for title which must be unique for the date in posted.'] +>>> formset.errors +[{}, {'__all__': u'Please correct the duplicate values below.'}] + +>>> data = { +... 'form-TOTAL_FORMS': '2', +... 'form-INITIAL_FORMS': '0', +... +... 'form-0-title': 'foo', +... 'form-0-slug': 'Morning in Prague', +... 'form-0-subtitle': 'foo', +... 'form-0-posted': '2009-01-01', +... 'form-1-title': 'blah', +... 'form-1-slug': 'Morning in Prague', +... 'form-1-subtitle': 'rawr', +... 'form-1-posted': '2009-08-02' +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset._non_form_errors +[u'Please correct the duplicate data for slug which must be unique for the year in posted.'] + +>>> data = { +... 'form-TOTAL_FORMS': '2', +... 'form-INITIAL_FORMS': '0', +... +... 'form-0-title': 'foo', +... 'form-0-slug': 'Morning in Prague', +... 'form-0-subtitle': 'rawr', +... 'form-0-posted': '2008-08-01', +... 'form-1-title': 'blah', +... 'form-1-slug': 'Prague', +... 'form-1-subtitle': 'rawr', +... 'form-1-posted': '2009-08-02' +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset._non_form_errors +[u'Please correct the duplicate data for subtitle which must be unique for the month in posted.'] """}