Fixed handling of primary keys in model formsets. Model formsets should now work…

Fixed handling of primary keys in model formsets. Model formsets should now work nicely with custom primary keys that are OneToOneField, ForeignKey and AutoField. Added tests to handle each of them.

Fixes #8241, #8694, #8695 and #8719.

Thanks Karen Tracey, jonloyens, sciyoshi, semenov and magneto for tracking down various parts of this patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@8756 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/admin/helpers.py

@@ -126,7 +126,7 @@ class InlineAdminForm(AdminForm):
         super(InlineAdminForm, self).__init__(form, fieldsets, prepopulated_fields)
 
     def pk_field(self):
-        return AdminField(self.form, self.formset._pk_field_name, False)
+        return AdminField(self.form, self.formset._pk_field.name, False)
 
     def deletion_field(self):
         from django.forms.formsets import DELETION_FIELD_NAME

django/db/models/fields/related.py

@@ -684,7 +684,7 @@ class ForeignKey(RelatedField, Field):
 
     def contribute_to_related_class(self, cls, related):
         setattr(cls, related.get_accessor_name(), ForeignRelatedObjectsDescriptor(related))
-
+        
     def formfield(self, **kwargs):
         defaults = {'form_class': forms.ModelChoiceField, 'queryset': self.rel.to._default_manager.complex_filter(self.rel.limit_choices_to)}
         defaults.update(kwargs)

django/forms/models.py

@@ -21,7 +21,7 @@ __all__ = (
 )
 
 def save_instance(form, instance, fields=None, fail_message='saved',
-                  commit=True):
+                  commit=True, exclude=None):
     """
     Saves bound Form ``form``'s cleaned_data into model instance ``instance``.
 
@@ -40,6 +40,8 @@ def save_instance(form, instance, fields=None, fail_message='saved',
             continue
         if fields and f.name not in fields:
             continue
+        if exclude and f.name in exclude:
+            continue
         f.save_form_data(instance, cleaned_data[f.name])
     # Wrap up the saving of m2m data as a function.
     def save_m2m():
@@ -115,8 +117,6 @@ def model_to_dict(instance, fields=None, exclude=None):
             else:
                 # MultipleChoiceWidget needs a list of pks, not object instances.
                 data[f.name] = [obj.pk for obj in f.value_from_object(instance)]
-        elif isinstance(f, OneToOneField):
-            data[f.attname] = f.value_from_object(instance)
         else:
             data[f.name] = f.value_from_object(instance)
     return data
@@ -261,11 +261,11 @@ class BaseModelFormSet(BaseFormSet):
 
     def save_new(self, form, commit=True):
         """Saves and returns a new model instance for the given form."""
-        return save_instance(form, self.model(), commit=commit)
+        return save_instance(form, self.model(), exclude=[self._pk_field.name], commit=commit)
 
     def save_existing(self, form, instance, commit=True):
         """Saves and returns an existing model instance for the given form."""
-        return save_instance(form, instance, commit=commit)
+        return save_instance(form, instance, exclude=[self._pk_field.name], commit=commit)
 
     def save(self, commit=True):
         """Saves model instances for every form, adding and changing instances
@@ -291,7 +291,7 @@ class BaseModelFormSet(BaseFormSet):
             existing_objects[obj.pk] = obj
         saved_instances = []
         for form in self.initial_forms:
-            obj = existing_objects[form.cleaned_data[self.model._meta.pk.attname]]
+            obj = existing_objects[form.cleaned_data[self._pk_field.name]]
             if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]:
                 self.deleted_objects.append(obj)
                 obj.delete()
@@ -319,9 +319,10 @@ class BaseModelFormSet(BaseFormSet):
 
     def add_fields(self, form, index):
         """Add a hidden field for the object's primary key."""
-        if self.model._meta.pk.auto_created:
-            self._pk_field_name = self.model._meta.pk.attname
-            form.fields[self._pk_field_name] = IntegerField(required=False, widget=HiddenInput)
+        from django.db.models import AutoField
+        self._pk_field = pk = self.model._meta.pk
+        if pk.auto_created or isinstance(pk, AutoField):
+            form.fields[self._pk_field.name] = IntegerField(required=False, widget=HiddenInput)
         super(BaseModelFormSet, self).add_fields(form, index)
 
 def modelformset_factory(model, form=ModelForm, formfield_callback=lambda f: f.formfield(),
@@ -369,7 +370,12 @@ class BaseInlineFormSet(BaseModelFormSet):
     def save_new(self, form, commit=True):
         kwargs = {self.fk.get_attname(): self.instance.pk}
         new_obj = self.model(**kwargs)
-        return save_instance(form, new_obj, commit=commit)
+        return save_instance(form, new_obj, exclude=[self._pk_field.name], commit=commit)
+    
+    def add_fields(self, form, index):
+        super(BaseInlineFormSet, self).add_fields(form, index)
+        if self._pk_field == self.fk:
+            form.fields[self._pk_field.name] = IntegerField(required=False, widget=HiddenInput)
 
 def _get_foreign_key(parent_model, model, fk_name=None):
     """

tests/modeltests/model_forms/models.py

@@ -69,6 +69,13 @@ class ImprovedArticleWithParentLink(models.Model):
 class BetterWriter(Writer):
     pass
 
+class WriterProfile(models.Model):
+    writer = models.OneToOneField(Writer, primary_key=True)
+    age = models.PositiveIntegerField()
+    
+    def __unicode__(self):
+        return "%s is %s" % (self.writer, self.age)
+
 class PhoneNumber(models.Model):
     phone = models.PhoneNumberField()
     description = models.CharField(max_length=20)
@@ -811,7 +818,41 @@ ValidationError: [u'Select a valid choice. 4 is not one of the available choices
 >>> bw = BetterWriter(name=u'Joe Better')
 >>> bw.save()
 >>> sorted(model_to_dict(bw).keys())
-['id', 'name', 'writer_ptr_id']
+['id', 'name', 'writer_ptr']
+
+>>> class WriterProfileForm(ModelForm):
+...     class Meta:
+...         model = WriterProfile
+>>> form = WriterProfileForm()
+>>> print form.as_p()
+<p><label for="id_writer">Writer:</label> <select name="writer" id="id_writer">
+<option value="" selected="selected">---------</option>
+<option value="1">Mike Royko</option>
+<option value="2">Bob Woodward</option>
+<option value="3">Carl Bernstein</option>
+<option value="4">Joe Better</option>
+</select></p>
+<p><label for="id_age">Age:</label> <input type="text" name="age" id="id_age" /></p>
+
+>>> data = {
+...     'writer': u'2',
+...     'age': u'65',
+... }
+>>> form = WriterProfileForm(data)
+>>> instance = form.save()
+>>> instance
+<WriterProfile: Bob Woodward is 65>
+
+>>> form = WriterProfileForm(instance=instance)
+>>> print form.as_p()
+<p><label for="id_writer">Writer:</label> <select name="writer" id="id_writer">
+<option value="">---------</option>
+<option value="1">Mike Royko</option>
+<option value="2" selected="selected">Bob Woodward</option>
+<option value="3">Carl Bernstein</option>
+<option value="4">Joe Better</option>
+</select></p>
+<p><label for="id_age">Age:</label> <input type="text" name="age" value="65" id="id_age" /></p>
 
 # PhoneNumberField ############################################################
 

tests/modeltests/model_formsets/models.py

@@ -47,8 +47,19 @@ class Place(models.Model):
         return self.name
 
 class Owner(models.Model):
+    auto_id = models.AutoField(primary_key=True)
     name = models.CharField(max_length=100)
     place = models.ForeignKey(Place)
+    
+    def __unicode__(self):
+        return "%s at %s" % (self.name, self.place)
+
+class OwnerProfile(models.Model):
+    owner = models.OneToOneField(Owner, primary_key=True)
+    age = models.PositiveIntegerField()
+    
+    def __unicode__(self):
+        return "%s is %d" % (self.owner.name, self.age)
 
 class Restaurant(Place):
     serves_pizza = models.BooleanField()
@@ -262,12 +273,12 @@ used.
 >>> for form in formset.forms:
 ...     print form.as_p()
 <p><label for="id_form-0-name">Name:</label> <input id="id_form-0-name" type="text" name="form-0-name" maxlength="100" /></p>
-<p><label for="id_form-0-write_speed">Write speed:</label> <input type="text" name="form-0-write_speed" id="id_form-0-write_speed" /><input type="hidden" name="form-0-author_ptr_id" id="id_form-0-author_ptr_id" /></p>
+<p><label for="id_form-0-write_speed">Write speed:</label> <input type="text" name="form-0-write_speed" id="id_form-0-write_speed" /><input type="hidden" name="form-0-author_ptr" id="id_form-0-author_ptr" /></p>
 
 >>> data = {
 ...     'form-TOTAL_FORMS': '1', # the number of forms rendered
 ...     'form-INITIAL_FORMS': '0', # the number of forms with initial data
-...     'form-0-author_ptr_id': '',
+...     'form-0-author_ptr': '',
 ...     'form-0-name': 'Ernest Hemingway',
 ...     'form-0-write_speed': '10',
 ... }
@@ -283,17 +294,17 @@ True
 >>> for form in formset.forms:
 ...     print form.as_p()
 <p><label for="id_form-0-name">Name:</label> <input id="id_form-0-name" type="text" name="form-0-name" value="Ernest Hemingway" maxlength="100" /></p>
-<p><label for="id_form-0-write_speed">Write speed:</label> <input type="text" name="form-0-write_speed" value="10" id="id_form-0-write_speed" /><input type="hidden" name="form-0-author_ptr_id" value="..." id="id_form-0-author_ptr_id" /></p>
+<p><label for="id_form-0-write_speed">Write speed:</label> <input type="text" name="form-0-write_speed" value="10" id="id_form-0-write_speed" /><input type="hidden" name="form-0-author_ptr" value="..." id="id_form-0-author_ptr" /></p>
 <p><label for="id_form-1-name">Name:</label> <input id="id_form-1-name" type="text" name="form-1-name" maxlength="100" /></p>
-<p><label for="id_form-1-write_speed">Write speed:</label> <input type="text" name="form-1-write_speed" id="id_form-1-write_speed" /><input type="hidden" name="form-1-author_ptr_id" id="id_form-1-author_ptr_id" /></p>
+<p><label for="id_form-1-write_speed">Write speed:</label> <input type="text" name="form-1-write_speed" id="id_form-1-write_speed" /><input type="hidden" name="form-1-author_ptr" id="id_form-1-author_ptr" /></p>
 
 >>> data = {
 ...     'form-TOTAL_FORMS': '2', # the number of forms rendered
 ...     'form-INITIAL_FORMS': '1', # the number of forms with initial data
-...     'form-0-author_ptr_id': hemingway_id,
+...     'form-0-author_ptr': hemingway_id,
 ...     'form-0-name': 'Ernest Hemingway',
 ...     'form-0-write_speed': '10',
-...     'form-1-author_ptr_id': '',
+...     'form-1-author_ptr': '',
 ...     'form-1-name': '',
 ...     'form-1-write_speed': '',
 ... }
@@ -419,6 +430,105 @@ We need to ensure that it is displayed
 <p><label for="id_form-0-my_pk">My pk:</label> <input id="id_form-0-my_pk" type="text" name="form-0-my_pk" maxlength="10" /></p>
 <p><label for="id_form-0-some_field">Some field:</label> <input id="id_form-0-some_field" type="text" name="form-0-some_field" maxlength="100" /></p>
 
+# Custom primary keys with ForeignKey, OneToOneField and AutoField ############
+
+>>> place = Place(name=u'Giordanos', city=u'Chicago')
+>>> place.save()
+
+>>> FormSet = inlineformset_factory(Place, Owner, extra=2, can_delete=False)
+>>> formset = FormSet(instance=place)
+>>> for form in formset.forms:
+...     print form.as_p()
+<p><label for="id_owner_set-0-name">Name:</label> <input id="id_owner_set-0-name" type="text" name="owner_set-0-name" maxlength="100" /><input type="hidden" name="owner_set-0-auto_id" id="id_owner_set-0-auto_id" /></p>
+<p><label for="id_owner_set-1-name">Name:</label> <input id="id_owner_set-1-name" type="text" name="owner_set-1-name" maxlength="100" /><input type="hidden" name="owner_set-1-auto_id" id="id_owner_set-1-auto_id" /></p>
+
+>>> data = {
+...     'owner_set-TOTAL_FORMS': '2',
+...     'owner_set-INITIAL_FORMS': '0',
+...     'owner_set-0-auto_id': '',
+...     'owner_set-0-name': u'Joe Perry',
+...     'owner_set-1-auto_id': '',
+...     'owner_set-1-name': '',
+... }
+>>> formset = FormSet(data, instance=place)
+>>> formset.is_valid()
+True
+>>> formset.save()
+[<Owner: Joe Perry at Giordanos>]
+
+>>> formset = FormSet(instance=place)
+>>> for form in formset.forms:
+...     print form.as_p()
+<p><label for="id_owner_set-0-name">Name:</label> <input id="id_owner_set-0-name" type="text" name="owner_set-0-name" value="Joe Perry" maxlength="100" /><input type="hidden" name="owner_set-0-auto_id" value="1" id="id_owner_set-0-auto_id" /></p>
+<p><label for="id_owner_set-1-name">Name:</label> <input id="id_owner_set-1-name" type="text" name="owner_set-1-name" maxlength="100" /><input type="hidden" name="owner_set-1-auto_id" id="id_owner_set-1-auto_id" /></p>
+<p><label for="id_owner_set-2-name">Name:</label> <input id="id_owner_set-2-name" type="text" name="owner_set-2-name" maxlength="100" /><input type="hidden" name="owner_set-2-auto_id" id="id_owner_set-2-auto_id" /></p>
+
+>>> data = {
+...     'owner_set-TOTAL_FORMS': '3',
+...     'owner_set-INITIAL_FORMS': '1',
+...     'owner_set-0-auto_id': u'1',
+...     'owner_set-0-name': u'Joe Perry',
+...     'owner_set-1-auto_id': '',
+...     'owner_set-1-name': u'Jack Berry',
+...     'owner_set-2-auto_id': '',
+...     'owner_set-2-name': '',
+... }
+>>> formset = FormSet(data, instance=place)
+>>> formset.is_valid()
+True
+>>> formset.save()
+[<Owner: Jack Berry at Giordanos>]
+
+# Ensure a custom primary key that is a ForeignKey or OneToOneField get rendered for the user to choose.
+
+>>> FormSet = modelformset_factory(OwnerProfile)
+>>> formset = FormSet()
+>>> for form in formset.forms:
+...     print form.as_p()
+<p><label for="id_form-0-owner">Owner:</label> <select name="form-0-owner" id="id_form-0-owner">
+<option value="" selected="selected">---------</option>
+<option value="1">Joe Perry at Giordanos</option>
+<option value="2">Jack Berry at Giordanos</option>
+</select></p>
+<p><label for="id_form-0-age">Age:</label> <input type="text" name="form-0-age" id="id_form-0-age" /></p>
+
+>>> owner = Owner.objects.get(name=u'Joe Perry')
+>>> FormSet = inlineformset_factory(Owner, OwnerProfile, max_num=1, can_delete=False)
+
+>>> formset = FormSet(instance=owner)
+>>> for form in formset.forms:
+...     print form.as_p()
+<p><label for="id_ownerprofile-0-age">Age:</label> <input type="text" name="ownerprofile-0-age" id="id_ownerprofile-0-age" /><input type="hidden" name="ownerprofile-0-owner" id="id_ownerprofile-0-owner" /></p>
+
+>>> data = {
+...     'ownerprofile-TOTAL_FORMS': '1',
+...     'ownerprofile-INITIAL_FORMS': '0',
+...     'ownerprofile-0-owner': '',
+...     'ownerprofile-0-age': u'54',
+... }
+>>> formset = FormSet(data, instance=owner)
+>>> formset.is_valid()
+True
+>>> formset.save()
+[<OwnerProfile: Joe Perry is 54>]
+
+>>> formset = FormSet(instance=owner)
+>>> for form in formset.forms:
+...     print form.as_p()
+<p><label for="id_ownerprofile-0-age">Age:</label> <input type="text" name="ownerprofile-0-age" value="54" id="id_ownerprofile-0-age" /><input type="hidden" name="ownerprofile-0-owner" value="1" id="id_ownerprofile-0-owner" /></p>
+
+>>> data = {
+...     'ownerprofile-TOTAL_FORMS': '1',
+...     'ownerprofile-INITIAL_FORMS': '1',
+...     'ownerprofile-0-owner': u'1',
+...     'ownerprofile-0-age': u'55',
+... }
+>>> formset = FormSet(data, instance=owner)
+>>> formset.is_valid()
+True
+>>> formset.save()
+[<OwnerProfile: Joe Perry is 55>]
+
 # Foreign keys in parents ########################################
 
 >>> from django.forms.models import _get_foreign_key