Rework form labels

[rpkilby]

Hey @carltongibson - wanted to take a stab at improving the form output. In brief, the PR allows filters to automatically generate more verbose, human friendly labels to use in forms. Given the following...

class ArticleFilter(FilterSet):
    exclude_author = CharFilter(name='author__name', lookup_expr='istartswith', exclude=True)

    class Meta:
        model = Article
        fields = {
            'author__name': ['exact'],
            'published': ['exact', 'date__lt', 'date__gt', ],
        }

This would generate default filter labels such as "Exclude author name starts with" and "Published date is less than". It's not perfect, but it should convey enough information and be suitable for DRF's filter integration.

---

Context:
The current form implementation works well for the Meta.fields list syntax, but kind of falls apart when it comes to the dict syntax. From the above example, there are a few issues:

• Field name translation does not traverse relationships for Meta.fields. Instead of 'author first name', we just get 'first name', which doesn't make sense as it is not an attribute on the book model. This is even worse for shared fields, such as 'id' - no way to differentiate between the book 'id' and author 'id' labels.
• Lookup expressions are not taken into account. This doesn't matter for the list syntax, since there is just the one 'exact' lookup, but this doesn't work with multiple lookups per field. All of the published filter labels just read 'published'.
• Oddly enough, these issues also affect the ordering field for both with the Meta.fields dict syntax and with declarative filters. Ordering is kind of borked - only really works with the Meta.fields list syntax.
• Additionally, not a fan with the current help_text for filters.
  • The 'Filter' help_text doesn't add any meaningful context. See Odd p tag #422 and DRF's default FilterSet, which comments them out.
  • The 'This is an exclusion filter' is useful, but could instead be handled in the label, as seen above.
  • help_text should provide additional context about usage. eg, CSV filters indicating that the expected input should be comma-separated.

Change list:

• Add default label generation to the Filter class.
• FilterSet no longer generates labels for filters.
• Existing 'Filter'/'Exclude' help_text has been removed.
• Added settings conf. Based on DRF's app settings, but a little simpler.
• FILTERS_VERBOSE_LABELS is a dict (or callable that resolves to a dict) of lookup expressions mapped to verbose expressions. eg, {'exact': 'is equal to'}.
• FILTERS_HELP_TEXT_FILTER and FILTERS_HELP_TEXT_EXCLUDE have been removed.
• Added 'Values should be comma-separated' help_text to the base CSV filter.

Misc notes:

• The 'exact' lookup has an empty verbose text. This was to maintain better consistency with the existing labels you'd see with the Meta.fields list syntax. eg, 'Author name' instead of 'Author name is equal to'.
• Case-insensitive lookups have the same verbose text as their case-sensitive counterparts.
  • I couldn't think of a terse way of differentiating the former.
  • Does it matter? Would you typically expose both lookups? If a user needs to differentiate, they can always override FILTERS_VERBOSE_LOOKUPS.

Questions:
At this point, the PR is 'feature complete' and tested, but there are a couple of issues/questions.

• The ordering field is somewhat borked (more details in Ordering filter problems #438), and the label changes exacerbate this. Currently, one test is failing. Should the PR temporarily mark the test as an expected failure (to be fixed in a separate PR), or should this PR grow in scope and fix ordering field? Separate PR merged.
• Should there be a usage warning that FILTERS_HELP_TEXT_FILTER and FILTERS_HELP_TEXT_EXCLUDE no longer have effect? Separate PR merged.
• Should LookupTypeField be integrated with the FILTERS_VERBOSE_LOOKUPS setting? (Probably could be handled as a separate PR).
• Are the verbose labels satisfactory for the current lookups?
  • Is the lack of differentiating text for case-insensitive lookups okay?

TODO:

• fix ordering field test
• remove DRF filterset help_text removal
• document settings deprecations / new settings (callable or dict-like).

[rpkilby]

Hey @carltongibson - this should be ready for review whenever. Any thoughts on the remaining questions/issues?

[carltongibson]

Any thoughts on the remaining questions/issues?

Yep. :-) To wit: It's probably OK as it is.

[eriktelepovsky]

Will there be an option to disable those "help-text" labels? I have actually set FILTERS_HELP_TEXT_FILTER to False and I would like to keep it the same way.

[rpkilby]

Hi @eriktelepovsky. I'm not entirely sure what you're asking here - could you clarify?

If you're concerned with help_texts - the existing "Filter" and "This is an exclusion filter" help_texts would be removed. The FILTERS_HELP_TEXT_FILTER and FILTERS_HELP_TEXT_EXCLUDE settings would also be defunct/removed. The PR does add a default BaseCSVFilter.help_text that would inform users of the CSV behavior, but this can be overridden to be blank.

If you're concerned with the more verbose labels, keep in mind that these are labels and not help_texts.

• Labels are only verbose when you have non-exact lookup_exprs.
• Verbose labels are only generated when no label has been provided. You can always prevent this behavior by setting the label.

You can also checkout the PR's branch and see if the form output is suitable.

[eriktelepovsky]

I was using older version of django-filters and after update, all my filter fields had "Filter" help text below the form field. So I had to use FILTERS_HELP_TEXT_FILTER to hide them. I don't want to display such information for any kind of filter even their lookup_expr is non-exact. Will it be possible to achieve that using simple approach/setting without necessarily updating all of my fields?

[rpkilby]

@eriktelepovsky - Here are some explanatory examples (before, after).

---

Whoops - forgot to add the code in question:

from tests import models
from django_filters import FilterSet, filters


class CharInFilter(filters.BaseInFilter, filters.CharFilter):
    pass


class UserFilter(FilterSet):
    username = filters.CharFilter(name='username', lookup_expr='exact')
    username__icontains = filters.CharFilter(name='username', lookup_expr='icontains')
    username_is_like = filters.CharFilter(name='username', lookup_expr='icontains', label='Username is like')
    status__in = CharInFilter(name='status', lookup_expr='in')

    class Meta:
        model = models.User
        fields = []


print(str(UserFilter().form))

[eriktelepovsky]

I don't want any "helptext" to be displayed by default. Neither "Values should be comma-separated". Can be all helptexts disabled using single setting, just like previous FILTERS_HELP_TEXT_FILTER?

[eriktelepovsky]

Thank you for DISABLE_HELP_TEXT ;)

[rpkilby]

Yep - I've added it since it was trivial to do. @carltongibson - thoughts on the DISABLE_HELP_TEXT setting?

@eriktelepovsky - if DISABLE_HELP_TEXT gets cut, you can always use a custom FilterSet class to disable field help_texts. Something like:

class FilterSet(django_filters.FilterSet):
    def __init__(self, *args, **kwargs):
        super(FilterSet, self).__init__(*args, **kwargs)

        for f in six.iteritems(self.filters.values()):
            f.extra.pop('help_text', '')

[eriktelepovsky]

Yes I know, but it seems to be unnecessary workaround for me. DISABLE_HELP_TEXT is definitely a better solution and I don't see a reason why it should be cut off 👍

[carltongibson]

... thoughts on the DISABLE_HELP_TEXT setting?

Grrr. I think it needs to stay. (I don't particularly like it, but I see the use-case.)

[carltongibson]

OK. Great. 👍

[carltongibson]

@rpkilby Can I just ask you to add

• A sub/section header grouping these help_text related settings
• A paragraph highlighting the out-of-the-box "Off but with BaseCSV default" behaviour, which leads to needing FILTERS_DISABLE_HELP_TEXT

Thanks.

[rpkilby]

A sub/section header grouping these help_text related settings

I've removed the docs for FILTERS_HELP_TEXT_FILTER and FILTERS_HELP_TEXT_EXCLUDE since they've been deprecated for the 1.0 release. Does that work?

A paragraph highlighting the out-of-the-box "Off but with BaseCSV default" behaviour, which leads to needing FILTERS_DISABLE_HELP_TEXT.

Done.

Also, added a section on FILTERS_VERBOSE_LABELS.

Preview docs here.