Question re django.forms.models.BaseModelFormSet.save_existing_objects()

[Mike Dewhirst]

I have just been digging into django.forms.models.py to debug my own
code and found something which confuses me in both 1.8 and 1.9

django.forms.models.BaseModelFormSet.save_existing_objects() puts
objects (child model instances) into the
BaseModelFormSet.deleted_objects list whether they are deleted or not.

Here is the 1.9 code (no need to show 1.8 because they are both more or
less the same)

741 def save_existing_objects(self, commit=True):
742 self.changed_objects = []
743 self.deleted_objects = []
744 if not self.initial_forms:
745 return []
746
747 saved_instances = []
748 forms_to_delete = self.deleted_forms
749 for form in self.initial_forms:
750 obj = form.instance
751 if form in forms_to_delete:
752 # If the pk is None, it means that the object can't be
753 # deleted again. Possible reason for this is that the
754 # object was already deleted from the DB. Refs #14877.
755 if obj.pk is None:
756 continue
757 self.deleted_objects.append(obj)
758 self.delete_existing(obj, commit=commit)
759 elif form.has_changed():
760 self.changed_objects.append((obj, form.changed_data))
761 saved_instances.append(self.save_existing(form, obj,
commit=commit))
762 if not commit:
763 self.saved_forms.append(form)
764 return saved_instances

Line 758 will fail and the object will not be deleted if commit == False
but line 757 has already added the object to a list.

Is this intended?

Thanks

Mike

[James Schneider]

Wouldn't L757 only get executed if L751 has already determined that the particular object/form is supposed to be deleted, meaning that self.deleted_objects would only contain objects/forms that were supposed to be deleted? 

-James 

[Mike Dewhirst]

On 21/12/2015 6:10 PM, James Schneider wrote:
> I have just been digging into django.forms.models.py

> <http://django.forms.models.py> to debug my own code and found

> something which confuses me in both 1.8 and 1.9
>
> django.forms.models.BaseModelFormSet.save_existing_objects() puts
> objects (child model instances) into the
> BaseModelFormSet.deleted_objects list whether they are deleted or not.
>
> Here is the 1.9 code (no need to show 1.8 because they are both more
> or less the same)
>
> 741 def save_existing_objects(self, commit=True):
> 742 self.changed_objects = []
> 743 self.deleted_objects = []
> 744 if not self.initial_forms:
> 745 return []
> 746
> 747 saved_instances = []
> 748 forms_to_delete = self.deleted_forms
> 749 for form in self.initial_forms:
> 750 obj = form.instance
> 751 if form in forms_to_delete:
> 752 # If the pk is None, it means that the
> object can't be
> 753 # deleted again. Possible reason for this
> is that the
> 754 # object was already deleted from the DB.
> Refs #14877.

> 755 if obj.pk <http://obj.pk> is None:

> 756 continue
> 757 self.deleted_objects.append(obj)
> 758 self.delete_existing(obj, commit=commit)
> 759 elif form.has_changed():
> 760 self.changed_objects.append((obj,
> form.changed_data))
> 761
> saved_instances.append(self.save_existing(form, obj, commit=commit))
> 762 if not commit:
> 763 self.saved_forms.append(form)
> 764 return saved_instances
>
> Line 758 will fail and the object will not be deleted if commit ==
> False but line 757 has already added the object to a list.
>
> Is this intended?
>
>
> Wouldn't L757 only get executed if L751 has already determined that the
> particular object/form is supposed to be deleted, meaning that
> self.deleted_objects would only contain objects/forms that were supposed
> to be deleted?

That is true. However, if commit was False that object wouldn't be
deleted but it would still appear in the self.deleted_objects list. My
concern - or more accurately lack of understanding - is that the object
would remain in existence AND appear in the deleted list. This was
precisely what happened in my project and which I was trying to debug.

The symptom in the Admin was a checked box for deleting a child record
which stubbornly stayed there despite the Admin reporting a successful
save.

Mike

>
> -James
>
> --
> You received this message because you are subscribed to the Google
> Groups "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to django-users...@googlegroups.com
> <mailto:django-users...@googlegroups.com>.
> To post to this group, send email to django...@googlegroups.com
> <mailto:django...@googlegroups.com>.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-users/CA%2Be%2BciWaSsBQ-5EvhpkT0MJmcGM1KTt2y%2Bnw2BQ6p%2BHEUd8Rqg%40mail.gmail.com
> <https://groups.google.com/d/msgid/django-users/CA%2Be%2BciWaSsBQ-5EvhpkT0MJmcGM1KTt2y%2Bnw2BQ6p%2BHEUd8Rqg%40mail.gmail.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

[James Schneider]

> That is true. However, if commit was False that object wouldn't be deleted but it would still appear in the self.deleted_objects list. My concern - or more accurately lack of understanding - is that the object would remain in existence AND appear in the deleted list. This was precisely what happened in my project and which I was trying to debug.
>

Why would you expect anything to be changed in the database when commit=False?

My interpretation is that self.deleted_items is referring to the items that were selected in the form[set] for deletion, and wouldn't refer to the state of the items per the DB, given that 'self' is a Form/Formset in this case. I totally understand the confusion, though.

I would suspect that you either need a super() call somewhere, or take care to also delete those items.

> The symptom in the Admin was a checked box for deleting a child record which stubbornly stayed there despite the Admin reporting a successful save.
>
> Mike
>

Yeah, been there on my own forms from time to time.

-James

[Tim Graham]

Hi Mike, see if the documentation at the bottom of https://docs.djangoproject.com/en/stable/topics/forms/formsets/#django.forms.formsets.BaseFormSet.can_delete helps.

[Mike Dewhirst]

On 22/12/2015 4:22 AM, Tim Graham wrote:
> Hi Mike, see if the documentation at the bottom of
> https://docs.djangoproject.com/en/stable/topics/forms/formsets/#django.forms.formsets.BaseFormSet.can_delete
> helps.

Yes it does. Thank you Tim.

"If you call formset.save(commit=False), objects will not be deleted
automatically. You’ll need to call delete() on each of the
formset.deleted_objects to actually delete them:"

I now understand the why of it. I'll see if I can do that in the Admin
after reimplementing my routine to update obj.modified_by with
request.user which - somehow - was causing commit to become False.

Many thanks

Mike

>
> On Monday, December 21, 2015 at 4:00:36 AM UTC-5, James Schneider wrote:
>
>
> > That is true. However, if commit was False that object wouldn't
> be deleted but it would still appear in the self.deleted_objects
> list. My concern - or more accurately lack of understanding - is
> that the object would remain in existence AND appear in the deleted
> list. This was precisely what happened in my project and which I was
> trying to debug.
> >
>
> Why would you expect anything to be changed in the database when
> commit=False?
>
> My interpretation is that self.deleted_items is referring to the
> items that were selected in the form[set] for deletion, and wouldn't
> refer to the state of the items per the DB, given that 'self' is a
> Form/Formset in this case. I totally understand the confusion, though.
>
> I would suspect that you either need a super() call somewhere, or
> take care to also delete those items.
>
> > The symptom in the Admin was a checked box for deleting a child
> record which stubbornly stayed there despite the Admin reporting a
> successful save.
> >
> > Mike
> >
>
> Yeah, been there on my own forms from time to time.
>
> -James
>

> --
> You received this message because you are subscribed to the Google
> Groups "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to django-users...@googlegroups.com
> <mailto:django-users...@googlegroups.com>.
> To post to this group, send email to django...@googlegroups.com
> <mailto:django...@googlegroups.com>.
> Visit this group at https://groups.google.com/group/django-users.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/django-users/8747c012-8398-44b8-b90c-ccbfcd020491%40googlegroups.com
> <https://groups.google.com/d/msgid/django-users/8747c012-8398-44b8-b90c-ccbfcd020491%40googlegroups.com?utm_medium=email&utm_source=footer>.