search

nyergler / nested-formset

Nest Django formsets for multi-level editing.
BSD 3-Clause "New" or "Revised" License
87 stars 21 forks source link

Checking has_changed before calling save() #12

Closed aidanlister closed 10 years ago

aidanlister commented 10 years ago

I'm seeing 1100 queries (http://share.aidanlister.com/Vq3j) coming from here: https://github.com/nyergler/nested-formset/blob/master/src/nested_formset/__init__.py#L45

Can we check has_changed before calling .save() for performance maybe?

nyergler commented 10 years ago

My goal when writing this was to mimic "simple" form semantics as much as possible. For that reason, I'm resistant to checking has_changed in the save() method. I'd much rather override has_changed to return True if any nested form has changed (which I think is what #11 would do).

aidanlister commented 10 years ago

Ah, so a custom save() noop for the grandparent formset, and check has_changed and call save() manually in the CBV? Okay I can do that. Thanks!

aidanlister commented 10 years ago

Oh wait, how can I override the .save() method on the grandparent form generated by nestedformset_factory?

nyergler commented 10 years ago

I don't think you need to override save() at all. The idiomatic way to do this in Django (if you're trying to remove extraneous database calls) is to check has_changed() and only call save() if needed.

if form.is_valid() and form.has_changed():
    form.save()

Coupled with the fix to has_changed in #11, I think that'd do what you want.

If you really want to override save in the top level form generated by nestedformset_factory, you need to subclass BaseNestedFormset, and then pass your subclass in as the formset kwarg.

I'm going to close this since I don't intend to change the save method in the base package, but I'm happy to answer questions here that may arise as you're working through this.