Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

[1.2.X] Fixed #14234 -- Re-validating a model instance added via Mode…

…lForm no longer throws spurious PK uniqueness errors. Thanks to David Reynolds and Jeremy Dunck.

Also moved Model._adding to Model._state.adding to reduce instance namespace footprint.

Backport of r14612.

git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.2.X@14615 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit d88cabd3da4bc82151ffaecc376c59478e191b20 1 parent 395af9d
@carljm carljm authored
View
13 django/db/models/base.py
@@ -260,6 +260,10 @@ class ModelState(object):
"""
def __init__(self, db=None):
self.db = db
+ # If true, uniqueness validation checks will consider this a new, as-yet-unsaved object.
+ # Necessary for correct validation of new instances of objects with explicit (non-auto) PKs.
+ # This impacts validation only; it has no effect on the actual save.
+ self.adding = False
class Model(object):
__metaclass__ = ModelBase
@@ -553,12 +557,15 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
# Store the database on which the object was saved
self._state.db = using
+ # Once saved, this is no longer a to-be-added instance.
+ self._state.adding = False
# Signal that the save is complete
if origin and not meta.auto_created:
signals.post_save.send(sender=origin, instance=self,
created=(not record_exists), raw=raw)
+
save_base.alters_data = True
def _collect_sub_objects(self, seen_objs, parent=None, nullable=False):
@@ -783,7 +790,7 @@ def _perform_unique_checks(self, unique_checks):
if lookup_value is None:
# no value, skip the lookup
continue
- if f.primary_key and not getattr(self, '_adding', False):
+ if f.primary_key and not self._state.adding:
# no need to check for unique primary key when editing
continue
lookup_kwargs[str(field_name)] = lookup_value
@@ -796,7 +803,7 @@ def _perform_unique_checks(self, unique_checks):
# Exclude the current object from the query if we are editing an
# instance (as opposed to creating a new one)
- if not getattr(self, '_adding', False) and self.pk is not None:
+ if not self._state.adding and self.pk is not None:
qs = qs.exclude(pk=self.pk)
if qs.exists():
@@ -826,7 +833,7 @@ def _perform_date_checks(self, date_checks):
qs = model_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 not getattr(self, '_adding', False) and self.pk is not None:
+ if not self._state.adding and self.pk is not None:
qs = qs.exclude(pk=self.pk)
if qs.exists():
View
4 django/forms/models.py
@@ -244,10 +244,10 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
# if we didn't get an instance, instantiate a new one
self.instance = opts.model()
object_data = {}
- self.instance._adding = True
+ self.instance._state.adding = True
else:
self.instance = instance
- self.instance._adding = False
+ self.instance._state.adding = False
object_data = model_to_dict(instance, opts.fields, opts.exclude)
# if initial was provided, it should override the values from instance
if initial is not None:
View
13 tests/regressiontests/forms/models.py
@@ -5,28 +5,34 @@
from django.db import models
from django.core.files.storage import FileSystemStorage
+
temp_storage_location = tempfile.mkdtemp()
temp_storage = FileSystemStorage(location=temp_storage_location)
+
class BoundaryModel(models.Model):
positive_integer = models.PositiveIntegerField(null=True, blank=True)
+
callable_default_value = 0
def callable_default():
global callable_default_value
callable_default_value = callable_default_value + 1
return callable_default_value
+
class Defaults(models.Model):
name = models.CharField(max_length=255, default='class default value')
def_date = models.DateField(default = datetime.date(1980, 1, 1))
value = models.IntegerField(default=42)
callable_default = models.IntegerField(default=callable_default)
+
class ChoiceModel(models.Model):
"""For ModelChoiceField and ModelMultipleChoiceField tests."""
name = models.CharField(max_length=10)
+
class ChoiceOptionModel(models.Model):
"""Destination for ChoiceFieldModel's ForeignKey.
Can't reuse ChoiceModel because error_message tests require that it have no instances."""
@@ -38,6 +44,7 @@ class Meta:
def __unicode__(self):
return u'ChoiceOption %d' % self.pk
+
class ChoiceFieldModel(models.Model):
"""Model with ForeignKey to another model, for testing ModelForm
generation with ModelChoiceField."""
@@ -51,11 +58,17 @@ class ChoiceFieldModel(models.Model):
multi_choice_int = models.ManyToManyField(ChoiceOptionModel, blank=False, related_name='multi_choice_int',
default=lambda: [1])
+
class FileModel(models.Model):
file = models.FileField(storage=temp_storage, upload_to='tests')
+
class Group(models.Model):
name = models.CharField(max_length=10)
def __unicode__(self):
return u'%s' % self.name
+
+
+class Cheese(models.Model):
+ name = models.CharField(max_length=100)
View
21 tests/regressiontests/forms/tests/regressions.py
@@ -3,6 +3,9 @@
from django.forms import *
from django.utils.translation import ugettext_lazy, activate, deactivate
+from regressiontests.forms.models import Cheese
+
+
class FormsRegressionsTestCase(TestCase):
def test_class(self):
# Tests to prevent against recurrences of earlier bugs.
@@ -120,3 +123,21 @@ class SomeForm(Form):
f = SomeForm({'field': ['<script>']})
self.assertEqual(t.render(Context({'form': f})), u'<ul class="errorlist"><li>field<ul class="errorlist"><li>&quot;&lt;script&gt;&quot; is not a valid value for a primary key.</li></ul></li></ul>')
+
+ def test_regression_14234(self):
+ """
+ Re-cleaning an instance that was added via a ModelForm should not raise
+ a pk uniqueness error.
+
+ """
+ class CheeseForm(ModelForm):
+ class Meta:
+ model = Cheese
+
+ form = CheeseForm({
+ 'name': 'Brie',
+ })
+ if form.is_valid():
+ obj = form.save()
+ obj.name = 'Camembert'
+ obj.full_clean()
Please sign in to comment.
Something went wrong with that request. Please try again.