ModelFormset ignores form properties

[andrew.mcmurry@…]

The implementation of modelformset_factory and inlineformset_factory ignore 'fields' and 'excludes' attributes of the supplied form (in fact all of Meta), and only use the values passed to the factory function.
Also the ModelFormset and InlineFormset 'save' method ignores the 'save' method of the supplied form.

class MyModelForm (django.forms.models.ModelForm):
    class Meta:
        model = models.MyModel
        fields = ('field1', 'field2')

    def save(self):
        m = super(MyModelForm, self).save()
        do_something_with(m)
        return m

MyFormSet = modelformset_factory(models.MyModel, MyModelForm)
fset = MyFormSet()

The forms that are part of fset have all fields of models.MyModel and not just 'field1' and 'field2' as expected.
Also fset.save() will not call 'do_something_with' on the models saved.

Much of the functionality of the ModelForm is ignored when used as part of a formset. This may be intended behavior, but it is not mentioned in the manual as far as I can see, and is not what one might expect.

[Justin]

I attached a rough initial patch that fixes both issues in this ticket.

modelform_factory() is changed so that the form argument's Meta is used to construct the model form. if 'fields' or 'exclude' are passed in, they override Meta. This fixes the first issue for both modelformset_factory and inlineformset_factory.

To get save() called on the forms, I rewrote BaseModelFormSet.save() to call it's forms.save(). I got rid of save_existing_objects() and save_new_objects(), but I'm not sure if those are considered part of the public API. BaseModelFormSet also now overrides _construct_form() in order to set the instance attribute of the forms. This also care makes the forms in a model formset behave more like normal model forms.

There's one nasty hack where a {{QuerySet}} is passed as the 'initial' argument for {{BaseFormSet.init()}} so that _initial_form_count and _total_form_count are set properly. Fixing that will probably require changes to {{BaseFormSet}}.

I think this patch fixes #8071 also.

[Jacob]

I'm going to stretch the term a little bit and call this a "feature" and postpone until after 1.0. It obviously needs a fix, but stuffing this in at the last moment is a recipe for other subtle bugs.

[Justin]

new patch that passes all but one test

[Justin]

changes the save_as_new test so the new patch passes it

[(none)]

Milestone post-1.0 deleted

[jkocherhans]

Taking this on with a few other related issues.

[Alan Justino da Silva]

Attached patch appears to target a very old revision. I will fix this bug in next 3 weeks (because I need it fixed), but for that I'm asking some guiedance to make not much mistakes. I will patch against 1.1-beta1.

Roadmap: (to source:django/forms/models.py#8691 )

• Compare rev8691 with 1.1-beta1
  
• Merge diff into rev8691 and study results
  
• Merge diff into 1.1-beta1 and run tests
  
• Force tests to pass
  

I found no tag for 1.1-beta1. So, if someone knows rev. number for this, please comment or msg me.

Thanks in advance.

[Alex Gaynor]

This is not a critical bug for 1.1, removing milestone.

[Alex Gaynor]

I'll also note that this looks like a dupe of #10208.

[Alan Justino da Silva]

It looks like fixed at 1.1.1, but I cant run unittests.

Replying to alanjds:

Attached patch appears to target a very old revision. I will fix this bug in next 3 weeks (because I need it fixed), but for that I'm asking some guiedance to make not much mistakes. I will patch against 1.1-beta1.

[Julien Phalip]

#8071 is very likely a symptom of the same problem. Both tickets should probably be addressed at the same time.

[Claude Paroz]

Test showing working functionality

[Claude Paroz]

This works on current trunk. I suggest to commit the attached test and then close this ticket.

[Ramiro Morales]

In [16918]:

Fixed #8160 -- Made sure modelformset_factory takes in account fields' and exclude` ModelForm options.

Thanks Andrew McMurry for the report and Claude Paroz for creating these tests.

(Actually, this had been fixed in r10619 but the tests added then exercise the
code in the context of ModelAdmin. This commit adds more generic tests.)