Add a hook for specifying whether a form in a formset should be deleted

[Brett Hoerner]

save_existing_objects currently uses form._raw_value to check the DELETION_FIELD_NAME,

 if self.can_delete:
     raw_delete_value = form._raw_value(DELETION_FIELD_NAME)
     should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value)
     if should_delete:
         self.deleted_objects.append(obj)
         obj.delete()
         continue

Is there any reason it can't use form.cleaned_data.get(DELETION_FIELD_NAME, False) instead? By using _raw_value we prevent users from deciding to flag an inline for deletion during form validation.

 if self.can_delete:
     should_delete = form.cleaned_data.get(DELETION_FIELD_NAME, False)
     if should_delete:
         self.deleted_objects.append(obj)
         obj.delete()
         continue

Use case: Inline FormSet in the admin that detects certain inlines _should_ be deleted based on other information, and there is no need to bug to admin user for confirmation.

Definitely exists in 1.0.x and trunk.

[jkocherhans]

Yes, there is a reason. If a form you are marking for deletion does not validate then form.cleaned_data will not exist. Are you getting an actual exception here? What isn't working? Code examples would help.

[jkocherhans]

I should also note that the very next line, should_delete = form.fields[DELETION_FIELD_NAME].clean(raw_delete_value) cleans the raw value, and that is what is used. You might be doing something I'm overlooking with the deletion field however, so if that's the case I'll see if I can figure out a way to accommodate your use case, but I'll need more specifics.

[Brett Hoerner]

No exception, but currently cleaned_data (while in the clean methods) displays the DELETION_FIELD_NAME as if the user can alter it with effect, ie:

print self.cleaned_data
[{'DELETE': False,
 'other_field': 0,
 ...},
 {'DELETE': False,
  'other_field': 1,
  ...}]

Now, in my clean method I'm detecting that certain inlines are no longer needed, and I want to flag them for deletion as if the user had,

for inline in self.cleaned_data:
    if inline.should_be_deleted():
        inline['DELETE'] = True

I just changed the value of DELETION_FIELD_NAME, but because form._raw_value(DELETION_FIELD_NAME) is used, nothing is actually deleted.

I don't understand why a form would be inside the save method, and thus the save_existing_objects method, if it weren't valid. But I'm new to 'newforms' (porting an old codebase now).

[jkocherhans]

Replying to bretthoerner:

No exception, but currently cleaned_data (while in the clean methods) displays the DELETION_FIELD_NAME as if the user can alter it with effect, ie:

You're depending on an implementation detail here. cleaned_data is not something you can expect to muck around with to cause any effects.

Now, in my clean method I'm detecting that certain inlines are no longer needed, and I want to flag them for deletion as if the user had,

for inline in self.cleaned_data:
    if inline.should_be_deleted():
        inline['DELETE'] = True

I just changed the value of DELETION_FIELD_NAME, but because form._raw_value(DELETION_FIELD_NAME) is used, nothing is actually deleted.

I'd need to see more context to be able to tell what self.cleaned_data actually refers to, but you might be better off using a custom subclass of BaseInlineFormSet, and change the save_existing_objects to do what you need. This is more of a feature request than a bug (even though I know it must not seem that way.) I'll put it in my queue for 1.2, but 1.1 is at a feature freeze right now, so it won't get into trunk for a little while. I need to think about it a bit more too.

I don't understand why a form would be inside the save method, and thus the save_existing_objects method, if it weren't valid. But I'm new to 'newforms' (porting an old codebase now).

Because save_existing_objects processes all the existing objects, don't read too much into the "save" part of that method.

You can check out #9587 and [10283] to see why things were changed.

[Brett Hoerner]

Replying to jkocherhans:

You're depending on an implementation detail here. cleaned_data is not something you can expect to muck around with to cause any effects.

Really? It's definitely an assumption on my part, but I think a reasonable one. Because my clean_foo methods are expected to return the value, and my clean method is expected to return a dict (for cleaned_data) I would think that means "change it how you want, return the value you want it to use." So if I change the DELETION_FIELD_NAME to True ... why isn't that respected? I'm not actually asking, but rather explaining the reasonable (I think) thoughts behind my assumption.

I'll override save_existing_objects as you said, but I think other people may run into this based on the above assumption.

If it helps to understand the problem,

from django.contrib import admin
from django.forms.formsets import DELETION_FIELD_NAME
from django.forms.models import BaseInlineFormSet

from myapp.bar import models

class FooFormSet(BaseInlineFormSet):
    def clean(self):
        # at this point self.cleaned_data is something like:
        # [{'DELETED': False,
        #   'field1': 'data1'},
        #  {'DELETED': False,
        #   'field1': 'data1'}]

        for i, foo in enumerate(self.cleaned_data):
            # in reality this is some logic to decide
            # whether we want to delete this inline for
            # some reason
            if foo:
                self.cleaned_data[i][DELETION_FIELD_NAME] = True

        # at this point self.cleaned_data is something like:
        # [{'DELETED': True,
        #   'field1': 'data1'},
        #  {'DELETED': True,
        #   'field1': 'data1'}]

        # And we return that, assuming the values we changed
        # will be used.
        return self.cleaned_data

class FooInline(admin.StackedInline):
    formset = FooFormSet
    model = models.Foo

class BarAdmin(admin.ModelAdmin):
    inlines = (FooInline,)
admin.site.register(models.Bar, BarAdmin)

Thanks so much for your quick help.

[jkocherhans]

Another feature that probably should have made it to 1.2. Punt! Yell at me about a week after 1.2 comes out.

[James Bennett]

1.3 is feature-frozen now. Also, per his comment, someone should yell at Joseph.

Hey, Joseph! Fix this (for 1.4)!

[Chris Beaven]

Hey Joseph, 1.3 is out! Fix this!

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Claude Paroz]

Use cleaned_data in _should_delete_form

[Claude Paroz]

Code has moved but now that cleaned_data is not deleted any more, we can use it in _should_delete_form.

[Claude Paroz]

I'm closing it as the original ticket subject is resolved (_should_delete_form). Cleaning up is tracked in #18751.