So I saw that #8160 has been pushed to post 1.0, which is understandable, but I've still been working on it since I need it fixed for a project. My previous patch was indeed failing all the model_formsets test, and it now passes all but one.
The one test it fails on seems a bit odd to me though. It's testing the save_as_new argument to InlineFormSet and changes formset.instance after the formset has been constructed: >>> formset = AuthorBooksFormSet(data, instance=Author(), save_as_new=True) >>> formset.is_valid() True >>> new_author = Author.objects.create(name='Charles Baudelaire') >>> formset.instance = new_author >>> [book for book in formset.save() if book.author.pk == new_author.pk] The problem is that in my patch I set the formset's child forms instance attribute during _construct_forms(), which is called from BaseFormSet.__init__(), and that instance already has the fk set to formset.instance. Changing formset.instance after it's been initialized causes the formset and its child forms to have a mismatch. In the case of this test the child forms won't save because their author_id is None. I see two ways to solve this: 1) Consider BaseInlineFormSet to be private and change the test to save the Author instance before saving the formset, like this: >>> new_author = Author.objects.create(name='Charles Baudelaire') >>> formset = AuthorBooksFormSet(data, instance=new_author, save_as_new=True) >>> formset.is_valid() True >>> [book for book in formset.save() if book.author.pk == new_author.pk] 2) Reassign the fk values on all the child form instances during save. This isn't really that bad, but I don't think it really mirrors how multiple ModelForms would be used in the absence of FormSets, which has kind of been my guideline. I think it's analogous to changing ModelForm.instance after it's been initialized, which will cause problems because obj_data will be empty, so right now I'd go for option 1. There might be other issues with the patch too, especially since I remove some methods that I'm still not sure are part of the API or not, so I understand it probably won't make it into 1.0. I uploaded the new patch along with a separate patch to change the test it's failing. -Justin On Wed, Aug 20, 2008 at 12:36 PM, Brian Rosner <[EMAIL PROTECTED]> wrote: > > On Wed, Aug 20, 2008 at 11:42 AM, Justin Fagnani > <[EMAIL PROTECTED]> wrote: >> >> On Wed, Aug 20, 2008 at 8:39 AM, Brian Rosner <[EMAIL PROTECTED]> wrote: >>> I am slightly unclear on what is allowed to >>> be broken in this phase of Django development. I suspect it is okay >>> since those methods are not explicitly documented and a quick note on >>> the wiki would be decent. Someone please correct me if I am wrong, but >>> will make this item post-1.0. >> >> I could rewrite the patch to preserve those methods, but it'd be a lot >> less elegant. I could see the argument that save_existing_objects(), >> and save_new_objects() are useful in some ways. > > One thing that I may have missed, but how are you dealing with > save_new in BaseInlineFormSet? From a quick glance that would seem > broken. > >> Yup, the tests pass. If I'm looking in the right places, the coverage >> seems pretty bad. ModelFormSets and InlineFormSets are not tested at >> all. > > Did you look at > http://code.djangoproject.com/browser/django/trunk/tests/modeltests/model_formsets/models.py? > >> >> Do you have a suggestion for not passing a queryset as the 'initial' >> argument to FormSet? I'm thinking that either the queryset is >> translated to a list of dicts like before, so the forms will have both >> an instance and initial data, or BaseFormSet.__init__() should be >> broken up so that _initial_form_count is set in another method that >> can be overridden. I like the later. > > I think the better solution here is to keep the __init__ code the same > but we need an instance level of the queryset persistent so we can use > the use the caching it would provide. Then use the the indexing like > you have with i local variable in _construct_form. > > -- > Brian Rosner > http://oebfare.com > > > > --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
