Optional client-side sorting of grids

[Mike Rylander]

For grids that either pull the full list of objects from the server, or that build their list based on user input (item status, checkin, checkout, renew, etc), and that use complex data structures in their data provider, there's no simple sorting mechanism. We can provide one, and a branch is forthcoming to do just that.

[Mike Rylander (mrylander)]

Here's the branch:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp-1697954-client_side_grid_sort

From the first commit:

There are several grids (items out, checkin, checkout, item status, etc) that
could benefit from the ability to sort their items, but either the data
provider uses a complex data structure or an API call that doesn't offer
sorting, or the item list is populated by user input rather than a call to the
server. In those cases, sorting is not available. However, if we know that
all the data in the grid is in client memory, it would be reasonable to offer
a client-side sort option.

This commit does that by teaching the grid to accept a "clientsort" feature
and teaching arrayNotifier how to sort the items currently stored. The sort
works over any mix of IDL objects, hashes, and flattened fields, supports
multisort, and pushes "nulls" to the end of the list.

------------

This branch enables client-side sorting on the following grids:
 * patron items out
 * patron holds (both grids)
 * checkin
 * checkout
 * renew
 * item status
 * pending patrons

It provides a custom comparator function to fix up mbts.balance_owed, which shows up as a string but should sort as a float.

[Andrea Neiman (aneiman)]

I have tested this code & consent to signing off on it with my name, Andrea Neiman & email address, <email address hidden>

[Bill Erickson (berick)]

Thanks for jumping on this much needed feature! For the most part, I'm ready to sign-off and merge, but I have a few comments/requests.

1. As noted in the commits, there are a couple of interfaces (patron items out, patron holds) where all rows of data are pre-fetched to support client-side sorting. I'd like to add some big, loud TODO comments suggesting that fetch-all client side sorting should be used with caution, since it can lead to the XUL-era "we can't render large buckets" class of problem for data sets that grow unbounded.

In the particular case of items out and holds, though, I imagine the impact will be minimal for the vast majority of patrons, and ultimately worth the trade-off.

2. I'd feel a lot better about the holds list in particular if circ/services/holds.js:local_flesh() had some basic caching support to prevent a slew of duplicate user fetching calls during grid rendering.

3. When testing a patron with 53 holds, it displays all 53 rows in the grid on first load, even though the page size is 25. I'm guessing having the grid display all of them is not intentional.

4. Branch needs a master rebase to resolve a minor merge conflict.

I can look at 1, 2, and 4 soon unless someone gets there first.

[Galen Charlton (gmc)]

Some spot-checks of largish consortial databases suggests that there are very few cases where patron records have more than 100 active loans, so if fetching everything performs acceptably for a hundred items out, we may be good to go. However, I've seen a couple cases where a patron record has more than 500 loans, so it might pay to be defensive, count 'em first, then choose whether to fetch everything or revert to a non-client-sorted view.

[Bill Erickson (berick)]

Staring on a branch here:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1697954-signoff-tweaks

I've addressed 1, 2, and 4 from my last comments here. Instead of local caching, it adds support to the hold details API for fleshing additional fields. This reduced the number API calls required to render a holds grid of 50 holds from 110 to 10. I also removed some no-longer relevant comments.

I still need to add some sign-offs. I have not addressed #3 from above.

I also haven't addressed Galen's suggestion of counting first. For what it's worth, I'm much less concerned about rendering the items-out interface, since that's a single pcrud query. The real beast is the holds list, which is a sluggish API, though the fleshing changes should help some.

[Mike Rylander (mrylander)]

Thanks, Bill. I've got #3 licked. Branch coming soon with signoffs on your commits.

[Mike Rylander (mrylander)]

And, here it is: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/miker/lp1697954-signoff-tweaks-notify-fix

[Bill Erickson (berick)]

Confirmed holds are now displaying only the first page as expected. Sorting still works fine there. It looks like the items out page needs the same treatment, though.

I also noticed that null / empty-string values always sort to the end, regardless of sort order. For example, sorting holds on current copy always results in holds with no copy sorting to the end. I'm not sure if this is intended or a bug.

[Mike Rylander (mrylander)]

Thanks, Bill. I'll look at the items out page, unless you want to spread the love/knowledge around and implement an analog.

The "nulls last" behavior is intentional, and noted in the commit message. If you look at ~ http://git.evergreen-ils.org/?p=working/Evergreen.git;a=blob;f=Open-ILS/web/js/ui/default/staff/services/grid.js;h=96c4a9eda9148d67d37d45072bad8b90e594d67f;hb=ddd3ccc0538e674e2d1ffc8ae62926a850058e95#l1585 you'll see that undefined values in JS are treated specially and go to the end. Empty strings, however, sort where "expected".

[Bill Erickson (berick)]

Thanks for clarifying, Mike. I'm looking at the circ list now.

[Bill Erickson (berick)]

New branch pushed:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1697954-items-out-paging

1. All sign-offs added for me and Andrea where appropriate.

2. Adds a fix for the items-out paging issues discussed above.

3. Adds a secondary fix to prevent items-out grids from refreshing themselves too aggressively, leading to redundant API calls and duplicate entries in the cache. Note this bug is part of current master, not a result of this feature, but the symptoms bubbled up working on this feature.

To see this one in action, navigate to patron items out, click on Noncataloged Circulations tab, and note in the console log there are 2 pcrud "ancc" search API calls. Ditto navigating back to the Items Out tab/grid. With the included patch, there's only one call per tab/grid.

For master merging, I believe we just need sign-offs on my last 2 commits.

[Mike Rylander (mrylander)]

So signed and merged. Thanks, Bill and Andrea, for the testing and improvement!