From ea05e61b2b8c0c9e5e10faa3f738b09b5f59adf1 Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Mon, 1 Sep 2008 19:08:08 +0000 Subject: [PATCH] Fixed #8209: `ModelForm`s now validate unique constraints. Alex Gaynor did much of this work, and Brian Rosner helped as well. git-svn-id: http://code.djangoproject.com/svn/django/trunk@8805 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/forms/models.py | 106 ++++++++++++++++++++-- django/utils/itercompat.py | 1 + docs/topics/forms/modelforms.txt | 12 ++- tests/modeltests/model_forms/models.py | 53 ++++++++++- tests/modeltests/model_formsets/models.py | 68 ++++++++++++++ 5 files changed, 229 insertions(+), 11 deletions(-) diff --git a/django/forms/models.py b/django/forms/models.py index 4563ace3a2..c153512f75 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -3,9 +3,10 @@ Helper functions for creating Form classes from Django models and database field objects. """ -from django.utils.translation import ugettext_lazy as _ from django.utils.encoding import smart_unicode from django.utils.datastructures import SortedDict +from django.utils.text import get_text_list +from django.utils.translation import ugettext_lazy as _ from util import ValidationError, ErrorList from forms import BaseForm, get_declared_fields @@ -20,6 +21,7 @@ __all__ = ( 'ModelMultipleChoiceField', ) + def save_instance(form, instance, fields=None, fail_message='saved', commit=True, exclude=None): """ @@ -202,6 +204,76 @@ class BaseModelForm(BaseForm): object_data.update(initial) super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data, error_class, label_suffix, empty_permitted) + def clean(self): + self.validate_unique() + return self.cleaned_data + + def validate_unique(self): + from django.db.models.fields import FieldDoesNotExist + unique_checks = list(self.instance._meta.unique_together[:]) + form_errors = [] + + # Make sure the unique checks apply to actual fields on the ModelForm + for name, field in self.fields.items(): + try: + f = self.instance._meta.get_field_by_name(name)[0] + except FieldDoesNotExist: + # This is an extra field that's not on the ModelForm, ignore it + continue + # MySQL can't handle ... WHERE pk IS NULL, so make sure we don't + # don't generate queries of that form. + is_null_pk = f.primary_key and self.cleaned_data[name] is None + if name in self.cleaned_data and f.unique and not is_null_pk: + unique_checks.append((name,)) + + # Don't run unique checks on fields that already have an error. + unique_checks = [check for check in unique_checks if not [x in self._errors for x in check if x in self._errors]] + + for unique_check in unique_checks: + # Try to look up an existing object with the same values as this + # object's values for all the unique field. + + lookup_kwargs = {} + for field_name in unique_check: + lookup_kwargs[field_name] = self.cleaned_data[field_name] + + qs = self.instance.__class__._default_manager.filter(**lookup_kwargs) + + # Exclude the current object from the query if we are editing an + # instance (as opposed to creating a new one) + if self.instance.pk is not None: + qs = qs.exclude(pk=self.instance.pk) + + # This cute trick with extra/values is the most efficiant way to + # tell if a particular query returns any results. + if qs.extra(select={'a': 1}).values('a').order_by(): + model_name = self.instance._meta.verbose_name.title() + + # A unique field + if len(unique_check) == 1: + field_name = unique_check[0] + field_label = self.fields[field_name].label + # Insert the error into the error dict, very sneaky + self._errors[field_name] = ErrorList([ + _("%(model_name)s with this %(field_label)s already exists.") % \ + {'model_name': model_name, 'field_label': field_label} + ]) + # unique_together + else: + field_labels = [self.fields[field_name].label for field_name in unique_check] + field_labels = get_text_list(field_labels, _('and')) + form_errors.append( + _("%(model_name)s with this %(field_label)s already exists.") % \ + {'model_name': model_name, 'field_label': field_labels} + ) + + # Remove the data from the cleaned_data dict since it was invalid + for field_name in unique_check: + 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 save(self, commit=True): """ @@ -246,18 +318,26 @@ class BaseModelFormSet(BaseFormSet): queryset=None, **kwargs): self.queryset = queryset defaults = {'data': data, 'files': files, 'auto_id': auto_id, 'prefix': prefix} - if self.max_num > 0: - qs = self.get_queryset()[:self.max_num] - else: - qs = self.get_queryset() - defaults['initial'] = [model_to_dict(obj) for obj in qs] + defaults['initial'] = [model_to_dict(obj) for obj in self.get_queryset()] defaults.update(kwargs) super(BaseModelFormSet, self).__init__(**defaults) + def _construct_form(self, i, **kwargs): + if i < self._initial_form_count: + kwargs['instance'] = self.get_queryset()[i] + return super(BaseModelFormSet, self)._construct_form(i, **kwargs) + def get_queryset(self): - if self.queryset is not None: - return self.queryset - return self.model._default_manager.get_query_set() + if not hasattr(self, '_queryset'): + if self.queryset is not None: + qs = self.queryset + else: + qs = self.model._default_manager.get_query_set() + if self.max_num > 0: + self._queryset = qs[:self.max_num] + else: + self._queryset = qs + return self._queryset def save_new(self, form, commit=True): """Saves and returns a new model instance for the given form.""" @@ -359,6 +439,14 @@ class BaseInlineFormSet(BaseModelFormSet): self._initial_form_count = 0 super(BaseInlineFormSet, self)._construct_forms() + def _construct_form(self, i, **kwargs): + form = super(BaseInlineFormSet, self)._construct_form(i, **kwargs) + if self.save_as_new: + # Remove the primary key from the form's data, we are only + # creating new instances + form.data[form.add_prefix(self._pk_field.name)] = None + return form + def get_queryset(self): """ Returns this FormSet's queryset, but restricted to children of diff --git a/django/utils/itercompat.py b/django/utils/itercompat.py index c166da35b8..4a28f81d8b 100644 --- a/django/utils/itercompat.py +++ b/django/utils/itercompat.py @@ -72,3 +72,4 @@ def sorted(in_value): out_value = in_value[:] out_value.sort() return out_value + diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt index d161b3f7f5..fe1e053d50 100644 --- a/docs/topics/forms/modelforms.txt +++ b/docs/topics/forms/modelforms.txt @@ -338,6 +338,16 @@ parameter when declaring the form field:: ... class Meta: ... model = Article +Overriding the clean() method +----------------------------- + +You can overide the ``clean()`` method on a model form to provide additional +validation in the same way you can on a normal form. However, by default the +``clean()`` method validates the uniqueness of fields that are marked as unique +on the model, and those marked as unque_together, if you would like to overide +the ``clean()`` method and maintain the default validation you must call the +parent class's ``clean()`` method. + Form inheritance ---------------- @@ -500,4 +510,4 @@ books of a specific author. Here is how you could accomplish this:: >>> from django.forms.models import inlineformset_factory >>> BookFormSet = inlineformset_factory(Author, Book) >>> author = Author.objects.get(name=u'Orson Scott Card') - >>> formset = BookFormSet(instance=author) \ No newline at end of file + >>> formset = BookFormSet(instance=author) diff --git a/tests/modeltests/model_forms/models.py b/tests/modeltests/model_forms/models.py index 5f714fbedb..99680ff34c 100644 --- a/tests/modeltests/model_forms/models.py +++ b/tests/modeltests/model_forms/models.py @@ -117,9 +117,26 @@ class CommaSeparatedInteger(models.Model): def __unicode__(self): return self.field +class Product(models.Model): + slug = models.SlugField(unique=True) + + def __unicode__(self): + return self.slug + +class Price(models.Model): + price = models.DecimalField(max_digits=10, decimal_places=2) + quantity = models.PositiveIntegerField() + + def __unicode__(self): + return u"%s for %s" % (self.quantity, self.price) + + class Meta: + unique_together = (('price', 'quantity'),) + class ArticleStatus(models.Model): status = models.CharField(max_length=2, choices=ARTICLE_STATUS_CHAR, blank=True, null=True) + __test__ = {'API_TESTS': """ >>> from django import forms >>> from django.forms.models import ModelForm, model_to_dict @@ -1132,8 +1149,42 @@ u'1,,2' >>> f.clean('1') u'1' -# Choices on CharField and IntegerField +# unique/unique_together validation +>>> class ProductForm(ModelForm): +... class Meta: +... model = Product +>>> form = ProductForm({'slug': 'teddy-bear-blue'}) +>>> form.is_valid() +True +>>> obj = form.save() +>>> obj + +>>> form = ProductForm({'slug': 'teddy-bear-blue'}) +>>> form.is_valid() +False +>>> form._errors +{'slug': [u'Product with this Slug already exists.']} +>>> form = ProductForm({'slug': 'teddy-bear-blue'}, instance=obj) +>>> form.is_valid() +True + +# ModelForm test of unique_together constraint +>>> class PriceForm(ModelForm): +... class Meta: +... model = Price +>>> form = PriceForm({'price': '6.00', 'quantity': '1'}) +>>> form.is_valid() +True +>>> form.save() + +>>> form = PriceForm({'price': '6.00', 'quantity': '1'}) +>>> form.is_valid() +False +>>> form._errors +{'__all__': [u'Price with this Price and Quantity already exists.']} + +# Choices on CharField and IntegerField >>> class ArticleForm(ModelForm): ... class Meta: ... model = Article diff --git a/tests/modeltests/model_formsets/models.py b/tests/modeltests/model_formsets/models.py index 332c5a7235..1e25baa3a7 100644 --- a/tests/modeltests/model_formsets/models.py +++ b/tests/modeltests/model_formsets/models.py @@ -73,6 +73,22 @@ class Restaurant(Place): def __unicode__(self): return self.name +class Product(models.Model): + slug = models.SlugField(unique=True) + + def __unicode__(self): + return self.slug + +class Price(models.Model): + price = models.DecimalField(max_digits=10, decimal_places=2) + quantity = models.PositiveIntegerField() + + def __unicode__(self): + return u"%s for %s" % (self.quantity, self.price) + + class Meta: + unique_together = (('price', 'quantity'),) + class MexicanRestaurant(Restaurant): serves_tacos = models.BooleanField() @@ -553,4 +569,56 @@ True >>> type(_get_foreign_key(MexicanRestaurant, Owner)) +# unique/unique_together validation ########################################### + +>>> FormSet = modelformset_factory(Product, extra=1) +>>> data = { +... 'form-TOTAL_FORMS': '1', +... 'form-INITIAL_FORMS': '0', +... 'form-0-slug': 'car-red', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +True +>>> formset.save() +[] + +>>> data = { +... 'form-TOTAL_FORMS': '1', +... 'form-INITIAL_FORMS': '0', +... 'form-0-slug': 'car-red', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset.errors +[{'slug': [u'Product with this Slug already exists.']}] + +# unique_together + +>>> FormSet = modelformset_factory(Price, extra=1) +>>> data = { +... 'form-TOTAL_FORMS': '1', +... 'form-INITIAL_FORMS': '0', +... 'form-0-price': u'12.00', +... 'form-0-quantity': '1', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +True +>>> formset.save() +[] + +>>> data = { +... 'form-TOTAL_FORMS': '1', +... 'form-INITIAL_FORMS': '0', +... 'form-0-price': u'12.00', +... 'form-0-quantity': '1', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +False +>>> formset.errors +[{'__all__': [u'Price with this Price and Quantity already exists.']}] + """}