AndrewIngram / django-extra-views

Django's class-based generic views are awesome, let's have more of them.
MIT License
1.39k stars 172 forks source link

Add note to documentation about `empty_permitted` when using `initial` #191

Closed BigglesZX closed 4 years ago

BigglesZX commented 5 years ago

Just ran into a persnickety issue when using multiple initial data to prepopulate my formset. Only the first form in the formset was being saved – the rest had empty cleaned_data.

It turns out that Django's detection of new/changed data was responsible – because the values in the formset's forms had not changed from their initial, Django didn't include them in the form data and so didn't try to save them.

I eventually found this SO answer which pointed to the solution of setting empty_permitted = False on the forms. However, I was not able to use formset_kwargs to set this property (see below).

Simplified code for illustration:

# models
class Product(models.Model):
    pass

class Reservation(models.Model):
    products = models.ManyToManyField(Product, blank=True)
# view
class ReservationCreateView(CreateWithInlinesView):
    model = Reservation
    form_class = ReservationCreateForm  # not included, for brevity, nothing special about it
    inlines = [ReservationProductInline]

    def dispatch(self, request, *args, **kwargs):
        self.initial_product_pks = [1, 2, 3]  # these would be set dynamically in production
        return super(ReservationCreateView, self).dispatch(request, *args, **kwargs)

    def forms_valid(self, form, inlines):
        response = self.form_valid(form)
        for formset in inlines:
            for _form in formset:
                if _form.cleaned_data:
                    instance = _form.save(commit=False)
                    instance.reservation_id = instance.reservation.pk
                    instance.save()
        return response
# form
class ReservationProductForm(forms.ModelForm):
    product = forms.ModelChoiceField(queryset=Product.objects.all())

    def __init__(self, *args, **kwargs):
        super(ReservationProductForm, self).__init__(*args, **kwargs)
        self.empty_permitted = False
# formset
class ReservationProductInline(InlineFormSetFactory):
    model = Reservation.products.through
    factory_kwargs = {'extra': 0, 'min_num': 1, 'max_num': 20,
                      'can_order': False, 'can_delete': False}
    fields = '__all__'
    form_class = ReservationProductForm

    def get_factory_kwargs(self):
        kwargs = super(ReservationProductInline, self).get_factory_kwargs()
        kwargs['extra'] = len(self.view.initial_product_pks) - 1
        return kwargs

    def get_initial(self):
        return [{'product': pk} for pk in self.view.initial_product_pks]

I was not able to use formset_kwargs with the form_kwargs key on my InlineFormSetFactory subclass to set empty_permitted as is described in the documentation on formset customization. When I tried that I got a TypeError: ModelFormMetaclass object got multiple values for keyword argument 'empty_permitted'. Hence the use of __init__ above. Not sure if I was doing something wrong, but the alternative works nonetheless.

Perhaps the documentation could be updated to make reference to this form property? Hope this is helpful to someone encountering the same problem. Thanks for a very useful library.

sdolemelipone commented 4 years ago

@BigglesZX sorry for taking a while to get back to you. It looks like Django does not permit empty_permitted to be passed in through form_kwargs. In Django 2.2 empty forms are constructed as below:

django/forms/formsets.py

class BaseFormSet:
    ...
    @property
    def empty_form(self):
        form = self.form(
            auto_id=self.auto_id,
            prefix=self.add_prefix('__prefix__'),
            empty_permitted=True,
            use_required_attribute=False,
            **self.get_form_kwargs(None)
        )
        self.add_fields(form, None)
        return form

I don't think this library should document what fields Django will or won't allow as part of form_kwargs as this may change between Django versions. Nevertheless thanks for reporting this. The below change in Django would address this, but would need to be made in Django:

class BaseFormSet:
    ...
    @property
    def empty_form(self):
        form_kwargs = self.get_form_kwargs(None).copy()
        form_kwargs.update({
            'auto_id': self.auto_id,
            'prefix': self.add_prefix('__prefix__'),
            'empty_permitted': True,
            'use_required_attribute': False,
        })
        form = self.form(**form_kwargs)
        self.add_fields(form, None)
        return form

You could still use form_kwargs if you also override the empty_form method of your model formset class as above.