MattBroach / DjangoRestMultipleModels

View (and mixin) for serializing multiple models or querysets in Django Rest Framework
MIT License
549 stars 67 forks source link

Add `sort_descending` attribute and break out sort into own function #8

Closed schweickism closed 8 years ago

schweickism commented 8 years ago

A reverse option was already built in to sorted(), the separate function makes extending it easier.

New to Django (just hacking on an existing project) and I honestly couldn't get the test suite to run so I didn't bother writing one. If you're willing to accept this PR you may want to at least quickly confirm all tests still pass. (Not sure if a new one is needed since again reverse was built-in.)

Thanks!

MattBroach commented 8 years ago

Thanks for implementing this! I liked your approach but I made a few changes for the sake of consistency with other parts of django:

  1. instead of using a new option (sort_descending), I add the ability to add a '-' to the beginning of the sorting_field to specify reverse sorting. This is in line with how django handles ordering fields in other situations. Now, to reverse order by the field pub_date you would set sorting_field='-pub_date'
  2. naming the function sort made me nervous (since it's also the name of a built-in python function), so I renamed it queryList_sort
  3. implemented a test to make sure everything ran properly

I've merged those changes into master. Let me know if you have any questions/concerns!