Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Fixed #14234 -- Re-validating a model instance added via ModelForm 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.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@14612 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit 38ba3775ec86718fabdf0ba1d5baf04a0f2164a7 1 parent 9cebb52
Carl Meyer authored November 18, 2010
13  django/db/models/base.py
@@ -262,6 +262,10 @@ class ModelState(object):
262 262
     """
263 263
     def __init__(self, db=None):
264 264
         self.db = db
  265
+        # If true, uniqueness validation checks will consider this a new, as-yet-unsaved object.
  266
+        # Necessary for correct validation of new instances of objects with explicit (non-auto) PKs.
  267
+        # This impacts validation only; it has no effect on the actual save.
  268
+        self.adding = False
265 269
 
266 270
 class Model(object):
267 271
     __metaclass__ = ModelBase
@@ -555,12 +559,15 @@ def save_base(self, raw=False, cls=None, origin=None, force_insert=False,
555 559
 
556 560
         # Store the database on which the object was saved
557 561
         self._state.db = using
  562
+        # Once saved, this is no longer a to-be-added instance.
  563
+        self._state.adding = False
558 564
 
559 565
         # Signal that the save is complete
560 566
         if origin and not meta.auto_created:
561 567
             signals.post_save.send(sender=origin, instance=self,
562 568
                 created=(not record_exists), raw=raw, using=using)
563 569
 
  570
+
564 571
     save_base.alters_data = True
565 572
 
566 573
     def delete(self, using=None):
@@ -701,7 +708,7 @@ def _perform_unique_checks(self, unique_checks):
701 708
                 if lookup_value is None:
702 709
                     # no value, skip the lookup
703 710
                     continue
704  
-                if f.primary_key and not getattr(self, '_adding', False):
  711
+                if f.primary_key and not self._state.adding:
705 712
                     # no need to check for unique primary key when editing
706 713
                     continue
707 714
                 lookup_kwargs[str(field_name)] = lookup_value
@@ -714,7 +721,7 @@ def _perform_unique_checks(self, unique_checks):
714 721
 
715 722
             # Exclude the current object from the query if we are editing an
716 723
             # instance (as opposed to creating a new one)
717  
-            if not getattr(self, '_adding', False) and self.pk is not None:
  724
+            if not self._state.adding and self.pk is not None:
718 725
                 qs = qs.exclude(pk=self.pk)
719 726
 
720 727
             if qs.exists():
@@ -744,7 +751,7 @@ def _perform_date_checks(self, date_checks):
744 751
             qs = model_class._default_manager.filter(**lookup_kwargs)
745 752
             # Exclude the current object from the query if we are editing an
746 753
             # instance (as opposed to creating a new one)
747  
-            if not getattr(self, '_adding', False) and self.pk is not None:
  754
+            if not self._state.adding and self.pk is not None:
748 755
                 qs = qs.exclude(pk=self.pk)
749 756
 
750 757
             if qs.exists():
4  django/forms/models.py
@@ -254,10 +254,10 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
254 254
             # if we didn't get an instance, instantiate a new one
255 255
             self.instance = opts.model()
256 256
             object_data = {}
257  
-            self.instance._adding = True
  257
+            self.instance._state.adding = True
258 258
         else:
259 259
             self.instance = instance
260  
-            self.instance._adding = False
  260
+            self.instance._state.adding = False
261 261
             object_data = model_to_dict(instance, opts.fields, opts.exclude)
262 262
         # if initial was provided, it should override the values from instance
263 263
         if initial is not None:
13  tests/regressiontests/forms/models.py
@@ -5,28 +5,34 @@
5 5
 from django.db import models
6 6
 from django.core.files.storage import FileSystemStorage
7 7
 
  8
+
8 9
 temp_storage_location = tempfile.mkdtemp()
9 10
 temp_storage = FileSystemStorage(location=temp_storage_location)
10 11
 
  12
+
11 13
 class BoundaryModel(models.Model):
12 14
     positive_integer = models.PositiveIntegerField(null=True, blank=True)
13 15
 
  16
+
14 17
 callable_default_value = 0
15 18
 def callable_default():
16 19
     global callable_default_value
17 20
     callable_default_value = callable_default_value + 1
18 21
     return callable_default_value
19 22
 
  23
+
20 24
 class Defaults(models.Model):
21 25
     name = models.CharField(max_length=255, default='class default value')
22 26
     def_date = models.DateField(default = datetime.date(1980, 1, 1))
23 27
     value = models.IntegerField(default=42)
24 28
     callable_default = models.IntegerField(default=callable_default)
25 29
 
  30
+
26 31
 class ChoiceModel(models.Model):
27 32
     """For ModelChoiceField and ModelMultipleChoiceField tests."""
28 33
     name = models.CharField(max_length=10)
29 34
 
  35
+
30 36
 class ChoiceOptionModel(models.Model):
31 37
     """Destination for ChoiceFieldModel's ForeignKey.
32 38
     Can't reuse ChoiceModel because error_message tests require that it have no instances."""
@@ -38,6 +44,7 @@ class Meta:
38 44
     def __unicode__(self):
39 45
         return u'ChoiceOption %d' % self.pk
40 46
 
  47
+
41 48
 class ChoiceFieldModel(models.Model):
42 49
     """Model with ForeignKey to another model, for testing ModelForm
43 50
     generation with ModelChoiceField."""
@@ -51,11 +58,17 @@ class ChoiceFieldModel(models.Model):
51 58
     multi_choice_int = models.ManyToManyField(ChoiceOptionModel, blank=False, related_name='multi_choice_int',
52 59
                                               default=lambda: [1])
53 60
 
  61
+
54 62
 class FileModel(models.Model):
55 63
     file = models.FileField(storage=temp_storage, upload_to='tests')
56 64
 
  65
+
57 66
 class Group(models.Model):
58 67
     name = models.CharField(max_length=10)
59 68
 
60 69
     def __unicode__(self):
61 70
         return u'%s' % self.name
  71
+
  72
+
  73
+class Cheese(models.Model):
  74
+   name = models.CharField(max_length=100)
21  tests/regressiontests/forms/tests/regressions.py
@@ -3,6 +3,9 @@
3 3
 from django.utils.unittest import TestCase
4 4
 from django.utils.translation import ugettext_lazy, activate, deactivate
5 5
 
  6
+from regressiontests.forms.models import Cheese
  7
+
  8
+
6 9
 class FormsRegressionsTestCase(TestCase):
7 10
     def test_class(self):
8 11
         # Tests to prevent against recurrences of earlier bugs.
@@ -120,3 +123,21 @@ class SomeForm(Form):
120 123
 
121 124
         f = SomeForm({'field': ['<script>']})
122 125
         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>')
  126
+
  127
+    def test_regression_14234(self):
  128
+        """
  129
+        Re-cleaning an instance that was added via a ModelForm should not raise
  130
+        a pk uniqueness error.
  131
+
  132
+        """
  133
+        class CheeseForm(ModelForm):
  134
+            class Meta:
  135
+                model = Cheese
  136
+
  137
+        form = CheeseForm({
  138
+            'name': 'Brie',
  139
+        })
  140
+        if form.is_valid():
  141
+            obj = form.save()
  142
+            obj.name = 'Camembert'
  143
+            obj.full_clean()

0 notes on commit 38ba377

Please sign in to comment.
Something went wrong with that request. Please try again.