ModelFormSet.save() with a deleted form should work even if the model has already been deleted
|Reported by:||andornaut||Owned by:||Anssi Kääriäinen|
|Cc:||patrys@…, olau@…||Triage Stage:||Ready for checkin|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
This issue occurs when you have a
ModelFormSet that contains a deleted Form that has an already deleted instance. When trying to save the
ModelFormSet, Django will try and fail during
save() because it is not able to lookup the instance by the raw_pk_value.
An edit page with two formsets. The formset at the top allows you to edit or delete instances of model
Foo. The formset at the bottom allows you to edit or delete instances of model
Bar, each of which have a FK to one of the instance of
Foo in the top formset.
If the user deletes
Foo above, the the related Bars below are marked as deleted (
fields['DELETE']=True). If a
Foo is deleted and save() is called on the top formset before the bottom formset, then the
Bars that have a FK to the deleted
Foo will be deleted. Then when
ModelFormSet tries to delete the
Bar forms marked as deleted it will fail when it tries to lookup the deleted
Bar by its PK.
This may can also occur outside of a single request if a Foo is deleted, for example, in another browser tab (untested):
1) Bar edit page GET request.
2) Foo edit page GET request.
3) Foo edit page POST: Delete 1 Foo, which cascade deletes some Bars
4) Bar edit page POST: Delete 1 Bar (the same one deleted in #3) - fails on
save() during instance lookup
def save_existing_objects(self, commit=True): # ... snip ... 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)
It may make more sense to use
form.instance instead of looking up the instance by its
raw_pk_value again. This would also require fewer calls to the database.
Perhaps, consider replacing the code above with:
obj = self.instance
Of course, other changes may also be necessary to fix this use case.