Add optional support for django filters

[TauPan]

While trying to use yadcf (https://github.com/vedmack/yadcf) for (date) range filters and multiselect-dropdowns I noticed in the resulting prototype that the queries where too slow for our database.

This is because icontains or iregex is always used, regardless of the type of field.

Instead of trying to make a specific backend for our application, I decided it would be worthwhile to try to integrate django-filters into django-rest-framework-datatables.

This is a complex task and it took me three (calendar) months to get to this point.

See https://github.com/TauPan/django-rest-framework-datatables/blob/add-django-filters/docs/django-filters.rst for documentation.

(It looks to me like the task is more or less finished, but I'm still integrating it into a project, so more fixes and modifications might come up.)

Notable points:

• I did not modify the example app or create an example app for the django-filter integration. I did so in my first prototype, but it's not very useful as a demonstration, since the real point of using django-filters if you need to optimize things. And the dataset for the example app is very small, so there's no need for optimization here.

• I had to drop python 2.7 support, since django-filter 2.2.0 requires python 3. See Planned timeline for python 2 support? #71

• I had to refactor the main backend in order to keep things DRY.

• The api and behaviour is a bit different than plain django-rest-framework-datatables. This is covered in the documentation under "Differences and Limitations".

I understand you don't have a lot of time to review things at the moment, so I'll just integrate this code into our django application and see how far I get and if there are problems or new use-cases, I'll update this pull request.

[TauPan]

I've managed to use the django filters based implementation as a drop-in replacement[1] for django-rest-framework-datatables (prior to this PR) but right now it requires that:

• The data attribute contains the exact lookup (i.e. 'related_model__field')
• The filter field_name should be exactly identical to that.
• The fields on the serializer should also be exactly identical.

Only then will the combination of column-search and global search work correctly.

This is a bit more restrictive (and repetitive) than before, which is certainly unintended. I'm not sure what's the best way to resolve this. It does work, however ;)

[1] By this I mean that icontains and iregex is used at all times and there are no optimized queries so far whatsoever.

[TauPan]

About that coverage decrease:

Impacted Files Coverage Δ
[...]
rest_framework_datatables/pagination.py 96.42% <0%> (-3.58%)

This is only because there's no ImportError any more, as django.utils.six is always imported. Maybe that's because I removed Python 2?

[TauPan]

As an appetizer (see #26) , this is an example of a (working) filter for a numerical range from https://github.com/vedmack/yadcf/

class YADCFRangeWidget(widgets.RangeWidget):

    def value_from_datadict(self, data, files, name):
        if name not in data:
            return None
        vals = data[name].split('-yadcf_delim-')
        new_data = data.copy()
        del new_data[name]
        for val, suffix in zip(vals, self.suffixes):
            new_data[self.suffixed(name, suffix)] = val
        return super().value_from_datadict(new_data, files, name)


class YADCFRangeField(fields.RangeField):

    widget = YADCFRangeWidget


class YADCFRangeFilter(filters.RangeFilter):

    field_class = YADCFRangeField


class MyFilter(DatatablesFilterSet):
    range = YADCFRangeFilter()

(I wasn't sure if it's ok to modify data, otherwise value_from_datadict might be a lot shorter.)

This will generate queries like WHERE range BETWEEN 1 AND 10 ...

[TauPan]

Also filtering datetime ranges works with very little code:

class YADCFDateTimeRangeWidget(YADCFRangeWidget):

    suffixes = ['after', 'before']


class YADCFIsoDateTimeRangeField(fields.IsoDateTimeRangeField):

    widget = YADCFDateTimeRangeWidget


class YADCFIsoDateTimeFromToRangeFilter(filters.IsoDateTimeFromToRangeFilter):

    field_class = YADCFIsoDateTimeRangeField

(preferably with eonasdan-bootstrap-datetimepicker ... not sure if others will work)

[TauPan]

Ok... I've noticed a problem in my code and fixed it above. The problem is that if I want to use a different lookup, e.g. 'in' for efficiency reasons, I need to override it in my filter, because the filterset will blatantly override everything with 'iregex' if [search_regex] is set on the column.

Part of the problem is that YADCF will always set search_regex e.g. on a multiselect filter.

So the responsibility to switch lookups must be on the filter class, not the filterset directly.

Now if you want to switch your lookups, you need to mixin SwitchRegexFilter, which comes with the added bonus that you can set lookup_regex_replacements to a different mapping on your filter.

(This means you could switch between arbitrary lookup types by toggling the regex feature on the datatables column.)

I guess I need to document this. I'll do that after some more testing.

[TauPan]

The limitation that the 'data' attribute needs to be set to the exact lookup (e.g. my_relation__field) in order for global search to work is caused by the way I construct the global search lookup from the datatables query.

The GlobalFilter mixin could be a little smarter and look at Filter-Object for the correct way to construct a global lookup.

[TauPan]

By now I've fixed all three of those issues. If you know django-filters, the filters will work as expected without any special magic and my code will infer the correct ordering based on the filter configuration.

This follows the principle of least astonishment if you come from django-filters, however it behaves quite differently compared with django-rest-framework-datatables without django-filter support. I've documented the differences in "Differences and limitations".

This feels like the correct design choice, since filter configuration via django-filters is way more flexible and powerful and it also means that django-rest-framework-datatables will behave more like vanilla Datatables (without any special magic behaviour of column name attributes).

I've managed to use the django filters based implementation as a drop-in replacement[1] for django-rest-framework-datatables (prior to this PR) but right now it requires that:

• The data attribute contains the exact lookup (i.e. 'related_model__field')
• The filter field_name should be exactly identical to that.
• The fields on the serializer should also be exactly identical.

Only then will the combination of column-search and global search work correctly.

This is a bit more restrictive (and repetitive) than before, which is certainly unintended. I'm not sure what's the best way to resolve this. It does work, however ;)

[1] By this I mean that icontains and iregex is used at all times and there are no optimized queries so far whatsoever.

[izimobil]

@TauPan Really good work, I will merge this soon, but I want to do a bugfix release before that.

[izimobil]

I will fix the conflicts and merge manually.
We'll improve doc examples later. Thank @TauPan !

[izimobil]

It would be great to add a HTML/JS example here.

[vrbrito]

Can you please do it?
I'm a bit lost on how to do this. :(

[matthewhegarty]

see #79

[izimobil]

@TauPan : I've fixed the conflicts and one issue (with order_by), all tests pass ! You PR is now merged, but for some reason (I did the merge locally), your commits do not appear in the commit log, really sorry about that !

[TauPan]

@TauPan : I've fixed the conflicts and one issue (with order_by), all tests pass ! You PR is now merged,

Good news! :)

Can you elaborate on which issue you did fix with order_by? Is there a testcase for that? Since you put everything into one large commit (understandable, since you had to rebase), it's a bit hard to see what changed in comparison with my branch.

but for some reason (I did the merge locally), your commits do not appear in the commit log, really sorry about that !

That's no issue at all :) Thanks for merging!

[izimobil]

The queryset was always ordered, even when get_ordering() returned an empty list, and this was causing warnings when running the test cases:

/home/izi/VirtualEnvs/drf-datatables/django-rest-framework-datatables/rest_framework_datatables/pagination.py:69: UnorderedObjectListWarning: Pagination may yield inconsistent results with an unordered object_list: <class 'albums.models.Album'> QuerySet.
  super(CachedCountPaginator, self).__init__(*args, **kwargs)

Not a big deal :) Thanks again.