izimobil/django-rest-framework-datatables

Django-Filter Quickstart Documentation: Subclassing of DatatablesFilterSet

banagale opened this issue ยท 10 comments

First, thanks to the folks who work on this package I really needed it today.

To those trying to implement the Django-filter subpackage and finding it missing, it is not in the current release, but it is in master. See #82

Once you have that, and are following the Django-Filter Quickstart, you might run into some confusion on what gets subclassed.

From my experience trying to repeat the docs, this line:

class AlbumGlobalFilter(GlobalFilter):

triggers the following error:

AttributeError: type object 'AlbumGlobalFilter' has no attribute '_meta'

I believe instead AlbumGlobalFilter should subclass DatatablesFilterSet, which is imported in the example but never implemented. i.e.:

class AlbumGlobalFilter(DatatablesFilterSet):

Which makes some sense given the idea of using the DataTables version of various imports otherwise handled by django-filter i.e. filters.FilterSet.

This may be correct for the example in the docs, but I was also stumped up by the Meta class in the current docs. The fields = '__all__' resulted in an error, I believe because I did not declare all of the fields from my model:

TypeError: 'Meta.fields' contains fields that are not defined on this FilterSet: __all__

I was able to get django-rest-framework-datatables and the django-filter to ultimately work for my subset of model fields. In case someone else is might appreciate an example, it is simply something like:

Declare the field-level global filter

class GlobalCharFilter(GlobalFilter, filters.CharFilter):
    pass

Then declare the Filterset with only the necessary fields:

class FacilityFilterCrud(DatatablesFilterSet):
    name = GlobalCharFilter(lookup_expr='icontains')

    class Meta:
        model = Facility
        fields = ['__name__']

A partial example like this in the docs, might be helpful to help demonstrate some of the more fine-grained control hinted at in the introduction.

Thank you for the explanation. I was already wondering if I had wrong version of the package!

Thank you for the explanation. I was already wondering if I had wrong version of the package!

You're welcome. Hope your build goes well!

@banagale Maybe this would be a nice addition to the documentation, can you work on a PR if you have time ? thanks.

@izimobil Sure. I typically look for a go-ahead from a maintainer before investing the time, which was why I created the issue.

I have another outstanding docs PR request in scrapy. I'll try and handle them both at the same time.

Feel free to assign this to me.

@izimobil It has been a while since I filed this, just to be sure: can you confirm that my thinking in the original comment is correct? For example, my definition of FacilityFilterCrud's Meta class is the most correct way to gain control?

@banagale Sorry I'm not sure about that, I didn't implement django filter integration (and didn't have a chance to try it on a real project). Maybe @matthewhegarty or @TauPan can answer you. Thanks.

@izimobil It has been a while since I filed this, just to be sure: can you confirm that my thinking in the original comment is correct? For example, my definition of FacilityFilterCrud's Meta class is the most correct way to gain control?

Uhm... almost I think.

I'm confused about the __name__ in the filters declaration. I see __all__ is a magic value that would work here and so would ['name'] but I can't find anything about __name__ in the django filters documentation. Wouldn't that be the classname?

(I actually use this in production code... unfortunately it's not a public repository:)

    class Meta:
        model = Match
        fields = '__all__'

See tests/test_django_filter_backend.py

class AlbumFilter(DatatablesFilterSet):
       """Filter name, artist and genre by name with icontains"""
       name = filters.CharFilter(lookup_expr='icontains')
       genres = filters.CharFilter(lookup_expr='name__icontains', distinct=True)
       artist = filters.CharFilter(lookup_expr='name__icontains')
   
       class Meta:
           model = Album
           fields = '__all__'

And if we put the model from the doc together it makes sense.

  class AlbumGlobalFilter(AlbumFilter):
    """Filter name, artist and genre by name with icontains"""
      name = GlobalCharFilter(lookup_expr='icontains')
      genres = GlobalCharFilter(field_name='genres__name', lookup_expr='icontains')
      artist = GlobalCharFilter(field_name='artist__name', lookup_expr='icontains')
  
      class Meta:
          model = Album
          fields = '__all__

Fixed by #117 , thanks to all.

Thank you, @johnazedo