Comment 3 for bug 1697954

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.