Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Fix the sort list function in couchdb design docs #738

Merged
merged 4 commits into from
Oct 14, 2016

Conversation

gnowoel
Copy link
Contributor

@gnowoel gnowoel commented Oct 13, 2016

Fixes #45.

Seems PouchDB does not recognize the sortDesc query parameter correctly. So I replaced it with descending as per this doc. Please correct me!

cc @HospitalRun/core-maintainers

@jkleinsc
Copy link
Member

@gnowoel thanks for the PR. Unfortunately this PR causes the sorts to always be sorted ascending and this is because the "sortDesc" parameter is a special parameter used in our list functions (http://docs.couchdb.org/en/1.6.1/couchapp/ddocs.html#listfun) which is how we sort our views. Making the change to pass along "descending" causes the following to happen:

  1. The view gets sorted in descending order by the key for the via (in the case of patient it is sorted descending by id)
  2. The list function reverses the order of the results.

The TLDR is that the descending parameter doesn't help us because that parameter sorts on the views key, not on the order parameter passed. In order to do sorting on the order parameter, we have to pass it along to a list function which will reverse the order if sortDesc=true is passed.

@jkleinsc
Copy link
Member

@gnowoel I misspoke a bit above... The reason I was seeing the patients always sorted in ascending order is that the views didn't get updated to recognize the changing of the parameter from sortDesc to descending. However, my point is that passing along descending=true only affects the sort order of the keys (see http://docs.couchdb.org/en/1.6.1/api/ddoc/views.html), not the columns we are trying to sort by. the list functions are using sortDesc to determine sortOrder for a column.

@gnowoel
Copy link
Contributor Author

gnowoel commented Oct 14, 2016

@jkleinsc Thanks for your detailed explanation. It really helps for me to understand the problems. I'll try to figure out a real solution.

@gnowoel
Copy link
Contributor Author

gnowoel commented Oct 14, 2016

@jkleinsc PR updated. Hope this one would work :)

@gnowoel gnowoel changed the title Use descending instead of sortDesc Fix the sort list function in couchdb design docs Oct 14, 2016
Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnowoel -- I think you have found the root of the problem! Thanks for digging into this. I think the only other thing needed on this PR is to update the version on all the views that use generateSortFunction so that all of the views get the fix. The other affected views are:
appointments_by_date
inventory_by_name

@gnowoel
Copy link
Contributor Author

gnowoel commented Oct 14, 2016

@jkleinsc Yes, I missed those two views. Thanks for pointing out. PR updated.

@jkleinsc
Copy link
Member

@gnowoel thanks for updating! Everything looks good so I'll merge it in. Thanks again!

@jkleinsc jkleinsc merged commit 68e4121 into HospitalRun:master Oct 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants