Optional client-side sorting of grids

Bug #1697954 reported by Mike Rylander on 2017-06-14
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

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) wrote :

Here's the branch:


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.

tags: added: pullrequest
Andrea Neiman (aneiman) wrote :

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

tags: added: signedoff
Galen Charlton (gmc) on 2017-06-19
Changed in evergreen:
status: New → Confirmed
Bill Erickson (berick) wrote :

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) wrote :

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) wrote :

Staring on a branch here:


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) wrote :

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

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers