Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_form() hook #1654

Open
bblanchon opened this issue Apr 25, 2024 · 5 comments
Open

get_form() hook #1654

bblanchon opened this issue Apr 25, 2024 · 5 comments

Comments

@bblanchon
Copy link

Hi there,

Thank you for this awesome library ❤️

As far as I understand, the commonly accepted way to use crispy-forms with django-filter is to override the form property like so:

import django_filters
from crispy_forms.helper import FormHelper
from crispy_forms.layout import Layout, Submit, Row, Column

class ProductFilter(django_filters.FilterSet):
    name = django_filters.CharFilter(lookup_expr='iexact')

    @property
    def form(self):
        form = super().form
        form.helper = FormHelper(form)
        form.helper.layout = Layout(
            Row(
                Column('price'),
                Column('release_date'),
            ),
            Submit("submit", "Apply filter")
        )
        return form

    class Meta:
        model = Product
        fields = ['price', 'release_date']

This solution works, but it recreates a new FormHelper and Layout every time the property is accessed.
I know I could cache the values, but since I need to do this in most filters, I wish there were a DRYer solution.

I understand I could also create a new Form derivative and specify in Meta.form but I think creating a class for this is overkill. Besides, the definition of this class, and therefore the construction of the layout, would have to be above the definition of the filters, which feels very unnatural.

I suggest adding a new hook, get_form() that would be called when the form is created:

class BaseFilterSet:
    ...
    
    @property
    def form(self):
        if not hasattr(self, "_form"):
            self._form = self.get_form()
        return self._form

    def get_form(self):
        Form = self.get_form_class()
        if self.is_bound:
            return Form(self.data, prefix=self.form_prefix)
        else:
            return Form(prefix=self.form_prefix)

I mentioned FormHelper and Layout as examples, but I'm sure there are other last-minute form customizations that users could do with this hook.

I know you've been reluctant to add hooks in the past (e.g., #1630), but I think this one perfectly aligns with the design of the library and with the rest of the Django ecosystem; for example, FormView.get_form() and ModelAdmin.get_form().

I can write a PR if you want.

Best regards,
Benoit

@bblanchon
Copy link
Author

I'm willing to write a PR for this if you want.

@carltongibson
Copy link
Owner

Hi, yes. @bblanchon It's not clear to me that I want to add this. 🤔

@bblanchon
Copy link
Author

How would you recommend using django-filter with crispy-forms then?

@carltongibson
Copy link
Owner

carltongibson commented Oct 15, 2024

I'm not sure why you're not encapsulating these items in your Form class, as shown in the Crispy Forms docs. (Cache them on the class if you don't want per-instance copies.)

Layouts — Django-Crispy-Forms 2.3 Documentation

@bblanchon
Copy link
Author

That's what I do, but this means I must create a new Form class for each FilterSet class.
Not only is it repetitive, but it also makes you build the crispy-forms layout on fields that are not defined in the Form class.

Here is an example:

import django_filters as filters
from django import forms
from crispy_forms.helper import FormHelper
from crispy_forms.layout import Layout, Row, Column, Submit

from .models import Product

class ProductFilterForm(forms.Form):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.helper = FormHelper(self)
        self.helper.layout = Layout(
            Row(
                Column("name"),
                Column('price'),
                Column('release_date'),
            ),
            Submit("filter", "Filter"),
        )

class ProductFilter(filters.FilterSet):
    name = filters.CharFilter(lookup_expr='iexact')

    class Meta:
        model = Product
        fields = ['price', 'release_date']
        form = ProductFilterForm

I believe it would be more intuitive to define the layout inside the FilterSet like so:

from typing import override
import django_filters as filters
from crispy_forms.helper import FormHelper
from crispy_forms.layout import Column, Layout, Row, Submit

from .models import Product

class ProductFilter(filters.FilterSet):
    name = filters.CharFilter(lookup_expr="iexact")

    @override
    def get_form(self):
        form = super().get_form()
        form.helper = FormHelper(form)
        form.helper.layout = Layout(
            Row(
                Column("name"),
                Column("price"),
                Column("release_date"),
            ),
            Submit("filter", "Filter"),
        )
        return form

    class Meta:
        model = Product
        fields = ["price", "release_date"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants