Set the default ordering on OrderableListMixin #198

Merged
merged 6 commits into from Feb 1, 2017

Projects

None yet

3 participants

@hanneshapke
Contributor

Allow a change in the default ordering for OrderableListMixin if desc is required as default

Pull request includes:

  • ordering_default can be set, if None than asc is assumed
  • check for correct option if option is given otherwise
    ImproperlyConfigured raised
  • Added tests (check for the correct order if ordering_default is set
    to asc and desc, as well as test to check for ImproperlyConfigured)
  • updated docs and changelog
hanneshapke added some commits Oct 30, 2015
@hanneshapke hanneshapke Allow a change in the default ordering for OrderableListMixin
- ordering_default can be set, if None than `asc` is assumed
- check for correct option if option is given otherwise
`ImproperlyConfigured` raised
- Added tests (check for the correct order if ordering_default is set
to `asc` and `desc`, as well as test to check for ImproperlyConfigured)
- updated docs and changelog
55eeb24
@hanneshapke hanneshapke Merge pull request #1 from hanneshapke/feature/ordering-default
Allow a change in the default ordering for OrderableListMixin
cb6e99e
@hanneshapke hanneshapke Add test to check that the ordering_default is not overwriting query …
…params
ee5ed89
@panizza
panizza commented Nov 15, 2015

This patch is bugged. I cloned your fork and set the ordering_default flag to "desc" and obviously the initial order is correctly set to "desc".
But after that, my queryset it will always ordered ascending even if in my queryset I explicitly set the ordering flag to "asc".
I tracked down the issue, and I found out that the problem is that whatever is the ordering value in the querystring, you always overwrite the ordering value with the default_ordering value, and because of the following statement you never assign the correct value to the ordering flag.

    self.order_by = order_by
    self.ordering = self.get_ordering_default()
    if order_by and self.request.GET.get("ordering", self.ordering) == "desc":
        order_by = "-" + order_by
        self.ordering = "desc"

Just try by setting the default_ordering to "desc" and then by "asc".
Then change the code with the following one:

    self.order_by = order_by
    self.ordering = self.get_ordering_default()
    if order_by and self.request.GET.get("ordering", self.ordering) == "desc":
        order_by = "-" + order_by
    self.ordering = self.request.GET.get("ordering", self.ordering)
@hanneshapke
Contributor

Hi @panizza

Thank you for your reply and for tracking down the bug. I am testing your suggestion and will push the fix later.

-Hannes

@hanneshapke
Contributor

@panizza Added your suggestion to the PR.

@kennethlove
Member

Hey, can you fix the minor conflict here? I'll get this in the next release.

@hanneshapke
Contributor
@kennethlove kennethlove merged commit b0fb4a7 into brack3t:master Feb 1, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment