izimobil / django-rest-framework-datatables

Seamless integration between Django REST framework and Datatables.
http://django-rest-framework-datatables.readthedocs.io/en/latest/
MIT License
395 stars 89 forks source link

Sorting a datatable by a related field fails #107

Closed Microserf closed 3 years ago

Microserf commented 3 years ago

I have a datatable loading server-side JSON, which includes a model with a related field. For instance, a row might look like this:

{
  "id": 1,
  "status": "Active",
  "record": {
    "id": 10,
    "name": "Laszlo"
  }
}

The datatable configuration includes the following two column definitions:

{
  name: 'status',
  data: 'status',
  title: 'Status',
  orderable: true
},
{
  name: 'record_name',
  data: 'record.name',
  title: 'Record Name',
  orderable: true
},

Display of both columns works fine. Server-side sorting of the status column works fine, but unfortunately, it fails for the record_name column.

I believe the problem may be due to the condition on line 84 of backends.py. In the case above, the value of field['data'] would be record.name, which isn't found within the list of filterable fields, which includes only ['status', 'record'].

By subclassing the DatatablesFilterBackend class, I managed to fix this issue thusly:

class WorkaroundDatatablesFilterBackend(DatatablesFilterBackend);
    def get_ordering_fields(self, request, view, fields):
        fields = super(WorkaroundDatatablesFilterBackend, self).get_ordering_fields(request, view, fields)

        ret = []
        for field_tuple in fields:
            # Correct an issue preventing sorting from being applied to related fields
            if '.' in field_tuple[0]['data']:
                field_tuple[0]['data'] = field_tuple[0]['data'].split('.', 1)[0]

            ret.append(field_tuple)

        return ret

This modification changes the value of field['data'] to record, allowing sorting to proceed as expected.

matthewhegarty commented 3 years ago

I don't know if it will help, but another workaround is to declare a subclass of DatatablesFilterSet which includes your field as an attribute. So you would declare something like:

record_name = filters.CharFilter()

There is an example here.

Once you have declared the DatatablesFilterSet, you have to reference it from your view (example)

izimobil commented 3 years ago

As stated in the docs, the correct code should be:

{
  name: 'status',
  data: 'status',
  title: 'Status',
  orderable: true
},
{
  name: 'record.name', // or 'record__name', both are valid
  data: 'record.name',
  title: 'Record Name',
  orderable: true
},

I agree this part of the documentation is not perfect though. If one of you is native english, feel free to submit a PR for improving this part.

Microserf commented 3 years ago

Thank you for your responses, and for pointing me to the relevant sections in the documentation, and to the example project that implements sorting on related fields.

Looking into the issue a bit more, I can see that there are two version of the DatatablesFilterBackend class which one can use. The one I was using (which is affected by the bug) is in rest_framework_datatables.django_filters.backends.DatatablesFilterBackend.

There is an identically-named class in rest_framework_datatables.filters.DatatablesFilterBackend. This version of the class is the one used in the sample project, and does not contain the bug.

Can you help me understand what the difference is between these two classes? When would it be appropriate to use the first one, vs. the second one?

matthewhegarty commented 3 years ago

Can you help me understand what the difference is between these two classes? When would it be appropriate to use the first one, vs. the second one?

You have the option to use django-filter, which gives more flexibility with the underlying queries. To use django-filter, you have to use the appropriate backend. There's more information here.

Do you still see the reported bug after making the changes suggested by David? ie. use record.name instead of record_name.

izimobil commented 3 years ago

@Microserf please close this issue if it's resolved by my comment (it should be), thanks ;)