Allowing further overriding of save_formset on a ModelAdmin

Question

Pretty basic usage scenario here. I want to save the user who created an object and the user who last modified it. However, it's an inlined model so I, of course need to use save_formset. The Django docs have the following example code:

class ArticleAdmin(admin.ModelAdmin):
    def save_formset(self, request, form, formset, change):
        instances = formset.save(commit=False)
        for instance in instances:
            instance.user = request.user
            instance.save()
        formset.save_m2m()

The thing is, if you notice, since super is never called, this is a dead-end. If the ModelAdmin is subclassed and this method is overridden in the same way, you lose the functionality inherent in the parent. This matters because this is such a common usage scenario that I want to factor out the functionality, so I created the following:

class TrackableInlineAdminMixin(admin.ModelAdmin):
    def save_formset(self, request, form, formset, change):
        instances = formset.save(commit=False)
        for instance in instances:
            if hasattr(instance, 'created_by') and hasattr(instance, 'modified_by'):
                if not instance.pk:
                    instance.created_by = request.user
                instance.modified_by = request.user
            instance.save()
        formset.save_m2m()
        super(TrackableInlineAdminMixin, self).save_formset(request, form, formset, change)

I tacked on the call to super out of habit more than anything else, not thinking that it actually will cause the formset to save twice. Nevertheless, it still works in every scenario except one: deleting. As soon as you try to delete an inline in the admin, you get an error. The error's pretty vague and not really relavent to my question here, but I believe it's related to trying to save the formset again after you've just deleted one of the instances in it. The code works just fine when the call to super is removed.

Long and short, is there any way that I'm missing to both customize the formset saving behavior and allow subclasses to do their own overriding?

Answer 1

This is a doozie.

I had some fun poking around and it appears all of the action happens here in django.forms.models.BaseModelFormSet.

The problem is that ModelFormSet.save() deletes instances regardless of of the commit flag and does not modify the forms to reflect the deleted state.

If you call save() again it iterates over the forms and in ModelChoiceField cleaning tries to pull up the referenced ID and throws an invalid choice error.

def save_existing_objects(self, commit=True):
    self.changed_objects = []
    self.deleted_objects = []
    if not self.initial_forms:
        return []

    saved_instances = []
    for form in self.initial_forms:
        pk_name = self._pk_field.name
        raw_pk_value = form._raw_value(pk_name)

        # clean() for different types of PK fields can sometimes return
        # the model instance, and sometimes the PK. Handle either.
        pk_value = form.fields[pk_name].clean(raw_pk_value) 
        pk_value = getattr(pk_value, 'pk', pk_value)

        obj = self._existing_object(pk_value)
        if self.can_delete and self._should_delete_form(form):
            self.deleted_objects.append(obj)
            obj.delete()  
            # problem here causes `clean` 6 lines up to fail next round

            # patched line here for future save()
            # to not attempt a second delete
            self.forms.remove(form)

The only way I was able to fix this is to patch BaseModelFormset.save_existing_objects to remove the form from self.forms if an object is deleted.

Did some testing and there doesn't appear to be any ill effects.