Ticket #9284: 9284.diff

django/forms/formsets.py

@@ -40 +40 @@
         self.error_class = error_class
         self._errors = None
         self._non_form_errors = None
-        # initialization is different depending on whether we recieved data, initial, or nothing
-        if data or files:
-            self.management_form = ManagementForm(data, auto_id=self.auto_id, prefix=self.prefix)
-            if self.management_form.is_valid():
-                self._total_form_count = self.management_form.cleaned_data[TOTAL_FORM_COUNT]
-                self._initial_form_count = self.management_form.cleaned_data[INITIAL_FORM_COUNT]
-            else:
-                raise ValidationError('ManagementForm data is missing or has been tampered with')
-        else:
-            if initial:
-                self._initial_form_count = len(initial)
-                if self._initial_form_count > self.max_num and self.max_num > 0:
-                    self._initial_form_count = self.max_num
-                self._total_form_count = self._initial_form_count + self.extra
-            else:
-                self._initial_form_count = 0
-                self._total_form_count = self.extra
-            if self._total_form_count > self.max_num and self.max_num > 0:
-                self._total_form_count = self.max_num
-            initial = {TOTAL_FORM_COUNT: self._total_form_count,
-                       INITIAL_FORM_COUNT: self._initial_form_count}
-            self.management_form = ManagementForm(initial=initial, auto_id=self.auto_id, prefix=self.prefix)
-
         # construct the forms in the formset
         self._construct_forms()
 
     def __unicode__(self):
         return self.as_table()
 
+    def _management_form(self):
+        """Returns the ManagementForm instance for this FormSet."""
+        if self.data or self.files:
+            form = ManagementForm(self.data, auto_id=self.auto_id, prefix=self.prefix)
+            if not form.is_valid():
+                raise ValidationError('ManagementForm data is missing or has been tampered with')
+        else:
+            form = ManagementForm(auto_id=self.auto_id, prefix=self.prefix, initial={
+                TOTAL_FORM_COUNT: self._total_forms(),
+                INITIAL_FORM_COUNT: self._initial_forms()
+            })
+        return form
+    management_form = property(_management_form)
+
+    def _total_forms(self):
+        """Returns the total number of forms in this FormSet."""
+        if self.data or self.files:
+            return self.management_form.cleaned_data[TOTAL_FORM_COUNT]
+        else:
+            total_forms = self._initial_forms() + self.extra
+            if total_forms > self.max_num > 0:
+                total_forms = self.max_num
+        return total_forms
+    _total_form_count = property(_total_forms)
+
+    def _initial_forms(self):
+        """Returns the number of forms that are required in this FormSet."""
+        if self.data or self.files:
+            return self.management_form.cleaned_data[INITIAL_FORM_COUNT]
+        else:
+            # Use the length of the inital data if it's there, 0 otherwise.
+            initial_forms = self.initial and len(self.initial) or 0
+            if initial_forms > self.max_num > 0:
+                initial_forms = self.max_num
+        return initial_forms
+    _initial_form_count = property(_initial_forms)
+
     def _construct_forms(self):
         # instantiate all the forms and put them in self.forms
         self.forms = []
@@ -97 +111 @@
         return form
 
     def _get_initial_forms(self):
-        """Return a list of all the intial forms in this formset."""
+        """Return a list of all the initial forms in this formset."""
         return self.forms[:self._initial_form_count]
     initial_forms = property(_get_initial_forms)
 

django/forms/models.py

@@ -54 +54 @@
         # callable upload_to can use the values from other fields.
         if isinstance(f, models.FileField):
             file_field_list.append(f)
+        # OneToOneField doesn't allow assignment of None. Guard against that
+        # instead of allowing it and throwing an error.
+        if isinstance(f, models.OneToOneField) and cleaned_data[f.name] is None:
+            pass
         else:
             f.save_form_data(instance, cleaned_data[f.name])
 
@@ -266 +270 @@
 
             lookup_kwargs = {}
             for field_name in unique_check:
-                lookup_kwargs[field_name] = self.cleaned_data[field_name]
+                lookup_value = self.cleaned_data[field_name]
+                # ModelChoiceField will return an object instance rather than
+                # a raw primary key value, so convert it to a pk value before
+                # using it in a lookup.
+                if isinstance(self.fields[field_name], ModelChoiceField):
+                    lookup_value =  lookup_value.pk
+                lookup_kwargs[field_name] = lookup_value
 
             qs = self.instance.__class__._default_manager.filter(**lookup_kwargs)
 
@@ -357 +367 @@
                  queryset=None, **kwargs):
         self.queryset = queryset
         defaults = {'data': data, 'files': files, 'auto_id': auto_id, 'prefix': prefix}
-        defaults['initial'] = [model_to_dict(obj) for obj in self.get_queryset()]
         defaults.update(kwargs)
         super(BaseModelFormSet, self).__init__(**defaults)
 
+    def _initial_forms(self):
+        """Returns the number of forms that are required in this FormSet."""
+        if not (self.data or self.files):
+            return len(self.get_queryset())
+        return super(BaseModelFormSet, self)._initial_forms()
+    _initial_form_count = property(_initial_forms)
+
     def _construct_form(self, i, **kwargs):
         if i < self._initial_form_count:
             kwargs['instance'] = self.get_queryset()[i]
@@ -380 +396 @@
 
     def save_new(self, form, commit=True):
         """Saves and returns a new model instance for the given form."""
-        return save_instance(form, self.model(), exclude=[self._pk_field.name], commit=commit)
+        return form.save(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, exclude=[self._pk_field.name], commit=commit)
+        return form.save(commit=commit)
 
     def save(self, commit=True):
         """Saves model instances for every form, adding and changing instances
@@ -410 +426 @@
             existing_objects[obj.pk] = obj
         saved_instances = []
         for form in self.initial_forms:
-            obj = existing_objects[form.cleaned_data[self._pk_field.name]]
+            obj = existing_objects[form.cleaned_data[self._pk_field.name].pk]
             if self.can_delete and form.cleaned_data[DELETION_FIELD_NAME]:
                 self.deleted_objects.append(obj)
                 obj.delete()
@@ -438 +454 @@
 
     def add_fields(self, form, index):
         """Add a hidden field for the object's primary key."""
-        from django.db.models import AutoField
+        from django.db.models import AutoField, OneToOneField, ForeignKey
         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)
+            # The pk field we're adding to the form won't be able to get its
+            # value from the ModelForm's instance argument, so provide an
+            # initial value here.
+            try:
+                pk_value = self.get_queryset()[index].pk
+            except IndexError:
+                pk_value = None
+            if isinstance(pk, OneToOneField) or isinstance(pk, ForeignKey):
+                qs = pk.rel.to._default_manager.get_query_set()
+            else:
+                qs = self.model._default_manager.get_query_set()
+            form.fields[self._pk_field.name] = ModelChoiceField(qs, initial=pk_value, required=False, widget=HiddenInput)
         super(BaseModelFormSet, self).add_fields(form, index)
 
 def modelformset_factory(model, form=ModelForm, formfield_callback=lambda f: f.formfield(),
@@ -477 +504 @@
         super(BaseInlineFormSet, self).__init__(data, files, prefix=prefix,
                                                 queryset=qs)
 
-    def _construct_forms(self):
+    def _initial_forms(self):
         if self.save_as_new:
-            self._total_form_count = self._initial_form_count
-            self._initial_form_count = 0
-        super(BaseInlineFormSet, self)._construct_forms()
+            return 0
+        return super(BaseInlineFormSet, self)._initial_forms()
+    _initial_form_count = property(_initial_forms)
+
+    def _total_forms(self):
+        if self.save_as_new:
+            return super(BaseInlineFormSet, self)._initial_forms()
+        return super(BaseInlineFormSet, self)._total_forms()
+    _total_form_count = property(_total_forms)
 
     def _construct_form(self, i, **kwargs):
         form = super(BaseInlineFormSet, self)._construct_form(i, **kwargs)
@@ -498 +531 @@
     get_default_prefix = classmethod(get_default_prefix)
 
     def save_new(self, form, commit=True):
-        fk_attname = self.fk.get_attname()
-        kwargs = {fk_attname: self.instance.pk}
-        new_obj = self.model(**kwargs)
-        if fk_attname == self._pk_field.attname or self._pk_field.auto_created:
-            exclude =  [self._pk_field.name]
-        else:
-            exclude = []
-        return save_instance(form, new_obj, exclude=exclude, commit=commit)
+        # Use commit=False so we can assign the parent key afterwards, then
+        # save the object.
+        obj = form.save(commit=False)
+        setattr(obj, self.fk.get_attname(), self.instance.pk)
+        obj.save()
+        # form.save_m2m() can be called via the formset later on if commit=False
+        if commit and hasattr(form, 'save_m2m'):
+            form.save_m2m()
+        return obj
 
     def add_fields(self, form, index):
         super(BaseInlineFormSet, self).add_fields(form, index)
@@ -620 +654 @@
         # ensure the we compare the values as equal types.
         if force_unicode(value) != force_unicode(self.parent_instance.pk):
             raise ValidationError(self.error_messages['invalid_choice'])
-        if self.pk_field:
-            return self.parent_instance.pk
         return self.parent_instance
 
 class ModelChoiceIterator(object):

tests/modeltests/model_formsets/models.py

@@ -1 @@
-
 import datetime
-
 from django import forms
 from django.db import models
 
@@ -27 +25 @@
 
     def __unicode__(self):
         return self.title
-    
+
 class BookWithCustomPK(models.Model):
     my_pk = models.DecimalField(max_digits=5, decimal_places=0, primary_key=True)
     author = models.ForeignKey(Author)
@@ -35 +33 @@
 
     def __unicode__(self):
         return u'%s: %s' % (self.my_pk, self.title)
-    
+
 class AlternateBook(Book):
     notes = models.CharField(max_length=100)
-    
+
     def __unicode__(self):
         return u'%s - %s' % (self.title, self.notes)
-    
+
 class AuthorMeeting(models.Model):
     name = models.CharField(max_length=100)
     authors = models.ManyToManyField(Author)
@@ -68 +66 @@
     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)
 
@@ -81 +79 @@
 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)
 
@@ -114 +112 @@
 # using inlineformset_factory.
 class Repository(models.Model):
     name = models.CharField(max_length=25)
-    
+
     def __unicode__(self):
         return self.name
 
 class Revision(models.Model):
     repository = models.ForeignKey(Repository)
     revision = models.CharField(max_length=40)
-    
+
     class Meta:
         unique_together = (("repository", "revision"),)
-    
+
     def __unicode__(self):
         return u"%s (%s)" % (self.revision, unicode(self.repository))
 
@@ -146 +144 @@
 class Player(models.Model):
     team = models.ForeignKey(Team, null=True)
     name = models.CharField(max_length=100)
-    
+
+    def __unicode__(self):
+        return self.name
+
+# Models for testing custom ModelForm save methods in formsets and inline formsets
+class Poet(models.Model):
+    name = models.CharField(max_length=100)
+
+    def __unicode__(self):
+        return self.name
+
+class Poem(models.Model):
+    poet = models.ForeignKey(Poet)
+    name = models.CharField(max_length=100)
+
     def __unicode__(self):
         return self.name
 
@@ -337 +349 @@
 
 >>> AuthorFormSet = modelformset_factory(Author, max_num=2)
 >>> formset = AuthorFormSet(queryset=qs)
->>> [sorted(x.items()) for x in formset.initial]
-[[('id', 1), ('name', u'Charles Baudelaire')], [('id', 3), ('name', u'Paul Verlaine')]]
+>>> [x.name for x in formset.get_queryset()]
+[u'Charles Baudelaire', u'Paul Verlaine']
 
 >>> AuthorFormSet = modelformset_factory(Author, max_num=3)
 >>> formset = AuthorFormSet(queryset=qs)
->>> [sorted(x.items()) for x in formset.initial]
-[[('id', 1), ('name', u'Charles Baudelaire')], [('id', 3), ('name', u'Paul Verlaine')], [('id', 2), ('name', u'Walt Whitman')]]
+>>> [x.name for x in formset.get_queryset()]
+[u'Charles Baudelaire', u'Paul Verlaine', u'Walt Whitman']
+
+
+# ModelForm with a custom save method in a formset ###########################
+
+>>> class PoetForm(forms.ModelForm):
+...     def save(self, commit=True):
+...         # change the name to "Vladimir Mayakovsky" just to be a jerk.
+...         author = super(PoetForm, self).save(commit=False)
+...         author.name = u"Vladimir Mayakovsky"
+...         if commit:
+...             author.save()
+...         return author
+
+>>> PoetFormSet = modelformset_factory(Poet, form=PoetForm)
+
+>>> data = {
+...     'form-TOTAL_FORMS': '3', # the number of forms rendered
+...     'form-INITIAL_FORMS': '0', # the number of forms with initial data
+...     'form-0-name': 'Walt Whitman',
+...     'form-1-name': 'Charles Baudelaire',
+...     'form-2-name': '',
+... }
+
+>>> qs = Poet.objects.all()
+>>> formset = PoetFormSet(data=data, queryset=qs)
+>>> formset.is_valid()
+True
+
+>>> formset.save()
+[<Poet: Vladimir Mayakovsky>, <Poet: Vladimir Mayakovsky>]
+
 
 # Model inheritance in model formsets ########################################
 
@@ -553 +596 @@
 [<AlternateBook: Flowers of Evil - English translation of Les Fleurs du Mal>]
 
 
+# ModelForm with a custom save method in an inline formset ###################
+
+>>> class PoemForm(forms.ModelForm):
+...     def save(self, commit=True):
+...         # change the name to "Brooklyn Bridge" just to be a jerk.
+...         poem = super(PoemForm, self).save(commit=False)
+...         poem.name = u"Brooklyn Bridge"
+...         if commit:
+...             poem.save()
+...         return poem
+
+>>> PoemFormSet = inlineformset_factory(Poet, Poem, form=PoemForm)
+
+>>> data = {
+...     'poem_set-TOTAL_FORMS': '3', # the number of forms rendered
+...     'poem_set-INITIAL_FORMS': '0', # the number of forms with initial data
+...     'poem_set-0-name': 'The Cloud in Trousers',
+...     'poem_set-1-name': 'I',
+...     'poem_set-2-name': '',
+... }
+
+>>> poet = Poet.objects.create(name='Vladimir Mayakovsky')
+>>> formset = PoemFormSet(data=data, instance=poet)
+>>> formset.is_valid()
+True
+
+>>> formset.save()
+[<Poem: Brooklyn Bridge>, <Poem: Brooklyn Bridge>]
+
+
 # Test a custom primary key ###################################################
 
 We need to ensure that it is displayed