From 27f9b96fa0b57857cb660e56c66e3ede5dee8c18 Mon Sep 17 00:00:00 2001 From: Brian Rosner Date: Sun, 31 Aug 2008 09:49:55 +0000 Subject: [PATCH] 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 | 2 +- django/db/models/fields/related.py | 2 +- django/forms/models.py | 26 +++-- tests/modeltests/model_forms/models.py | 43 +++++++- tests/modeltests/model_formsets/models.py | 122 ++++++++++++++++++++-- 5 files changed, 176 insertions(+), 19 deletions(-) diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 7a60943d46..56b4f56a68 100644 --- a/django/contrib/admin/helpers.py +++ b/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 diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 2764a87ca8..0513496be2 100644 --- a/django/db/models/fields/related.py +++ b/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) diff --git a/django/forms/models.py b/django/forms/models.py index bd9597d67a..6acd32ce4f 100644 --- a/django/forms/models.py +++ b/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): """ diff --git a/tests/modeltests/model_forms/models.py b/tests/modeltests/model_forms/models.py index b8d3f932f8..0ea20751d9 100644 --- a/tests/modeltests/model_forms/models.py +++ b/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() +

+

+ +>>> data = { +... 'writer': u'2', +... 'age': u'65', +... } +>>> form = WriterProfileForm(data) +>>> instance = form.save() +>>> instance + + +>>> form = WriterProfileForm(instance=instance) +>>> print form.as_p() +

+

# PhoneNumberField ############################################################ diff --git a/tests/modeltests/model_formsets/models.py b/tests/modeltests/model_formsets/models.py index 65987c1b43..b0ad420003 100644 --- a/tests/modeltests/model_formsets/models.py +++ b/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()

-

+

>>> 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()

-

+

-

+

>>> 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

+# 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() +

+

+ +>>> 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() +[] + +>>> formset = FormSet(instance=place) +>>> for form in formset.forms: +... print form.as_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() +[] + +# 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() +

+

+ +>>> 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() +

+ +>>> 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() +[] + +>>> formset = FormSet(instance=owner) +>>> for form in formset.forms: +... print form.as_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() +[] + # Foreign keys in parents ######################################## >>> from django.forms.models import _get_foreign_key