vegu
commented
3 years ago @egfrank heads up there were many major changes on master as part of the permissioning refactor in #1.
I merged your pull request to master as well after resolving the conflicts that resulted from it. Please merge master to your dev branch and open a new PR once there is more code changes pushed for this.
Is that problematic? Definitely can be fixed but I would basically implement my own LowerCaseOrderingFilter.
Yeah we have to normalize everything to lowercase otherwise it's going to be annoying, especially in big lists - if it's just a pass through to django orm, would something like this this work?
from django.db.models.functions import Lower
InternetExchangeMember.objects.order_by(Lower('name'))Ordering itself seems to work well, however initial sorting doesn't seem to apply (it shows sorted by name but doesnt appear to be sorted by anything)
egfrank
@grizz @vegu there are some open questions in here, but also if you have comments on the code let me know.
Okay here is a work in progress PR for these changes.
Some questions / concerns:
1) We need to basically decide which of the columns we want to be sortable and which we don't want to be sortable. 2) Right now sorting is done with Django Rest Framework's OrderingFilter, which is basically just uses the ORM's order_by function. It seems like OrderingFilter does not coerces strings to lowercase before sorting meaning we get the following Descending sort on Names:
Is that problematic? Definitely can be fixed but I would basically implement my own LowerCaseOrderingFilter.
3) The backend actually supports sorting on multiple columns. Is this something we want for this data table? I don't have a good idea of what that would look like on the frontend.