webstaff sortable holds pull list

I can't see any reason to expend any effort to make it available and it's certainly nothing to hold sorting up over. (I'm also really not fond of wasting more server round trips on that number.)

New branch with 2 commits pushed:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1653001-webstaff-idl-pull-list

It turns out my question about queue position is moot... There is a significant amount of supporting code for holds that assumes the holds data exists in a certain format, namely the format returned from "open-ils.circ.hold.details.retrieve" and friends. Because of this, we have to fetch the extra details data for every hold regardless.

The grid now renders itself using the sort-able "ahopl" class with a canned grid query. Once all holds are retrieved, an additional batch/streaming call is made to fetch the extra details for all of the holds on display. This data is cached, so re-sorts, etc. will not cause another details fetch.

The 2nd commit in the branch resolves a cstore exhaustion problem with
open-ils.circ.hold.details.batch.retrieve.authoritative.

I know you said the queue position question is moot at this point, but in reviewing my notes from the Simplified Holds list development project, we determined this field as well as a few others (Estimated Wait, Top of Queue) were unnecessary because, by the time the hold is on the pull list, the queue position should be #1. Those fields are only valuable in holds interfaces displaying holds that have not yet been captured.

Thanks for tackling this one Bill! I noticed a couple of things with sorting in the pull list after loading this code.

In bug 1437104, you noted that the pull list is sorting by shelving location and then call number. However, I'm not seeing this as the default sort when I retrieve the pull list. I've attached a screenshot below of the BR1 pull list on a system with Concerto data.

Also, when I manually configure the sorting to go by shelving location, then call number, it's sorting shelving location alphabetically, when it should be using the order defined in asset.copy_location_order.

Also adding a note that the Call Number is not sorting by call number sortkey.

Thanks for testing, Kathy.

I have force-pushed changes to address all of the sorting issue, including a rebase to current master.

A word about sorting...

The grid is rendered from the "ahopl" IDL class, which has separate dedicated fields for the copy location sort position and call number sort key. We could handle these 2 additional fields in a couple of different ways. I have opted in the current code to lump each set of fields into a single user-friendly field that uses the sort key for the field definition, but renders the raw value(s) in the column cells.

For example, the "Call Number" column now uses the "call_number_sort_prefix" as its internal field, thus supporting the correct sort behavior, but uses the call number prefix/label/suffix values in the grid cell, so that it looks as expected to the user. I did something similar to the shelving location column.

Alternatively, we could display one raw column and one sort column for each, allowing the user to see both data sets and sort on both, depending on their needs. For example, you could sort by call number label ascibettically or or using its sort key value.

I opted for the combined fields approach, thinking it would be more user-friendly to staff.

And to anticipate the next question about the grid/sorting, the grid only allows one column per path (to prevent path.* paths from creating duplicates), so we can't as it currently stands have a raw field, plus a sort field, plus a combined sort/raw field. If there is interest in that, though, I'm sure we could make it work.

IMO, the sort key fields really shouldn't ever be exposed in the UI. It might be nice as a debuging aid, but the that's supporting developer needs above user needs, I think. So, I'm for the approach you've taken.

Agreed.

Should this simply be folded into the current webstaff branch, on the theory that that branch gets an exemption from the beta cut-off?

I rebased the working branch and gave it another quick test to confirm nothing broke.

Mike, if you want to point me at the webstaff branch, I can fold the (2) commits into that as well.

Sure thing, Bill. It's at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/kmlussier/webstaff-sprint-4-5 ... IMO, any webstaff fixes can go in there.

Thanks!

I just took a look at this branch. The default sort is now Copy Location -> Call number, and the call number appears to be sorting by sortkey. However, I found a few issues.

The Copy location sorting is a little wonky. I've attached a screenshot that shows it starts going by the copy location order defined in the local admin interface. Easy Reader is one of the first copy locations defined in that interface, which is why it shows first. However, if you look further down in the sort order, we see Easy Reader titles showing up again.

The barcode link is now embedding the barcode instead of the copy id in the URL, which leads to an empty item status screen.

I would consider merging the code anyway and filing bugs to address the above two issues later, but I'm also having trouble retrieving all of the titles that should be on the pull list. Logged into BR1 on a database with the Concerto data, I should see 41 titles in the pull list. However, I'm only retrieving 16 titles. I do see the 41 in the xul interface and also when I print the pull list using the Print button.

At first, I thought I was seeing a bug I already reported in bug 1643697. However, in that bug, I was always able to retrieve all holds after paging through the list. In this case, I'm unable to see more than the 16 holds.

I was able to confirm and repair the discrepancy between the number of holds in the web staff and XUL client versions. The query was looking for holds with a pickup lib of here, instead of holds whose copies had a circ lib of here.

Fix pushed to the tip of the working branch and branch rebased to current master.

I'm unable to reproduce the sorting problems. However, I bet the sorting issue is a result of the same bug... If the list was showing items from various circ libs, it's possible the variants of Easy Reader were actually different copy locations -- there are 3 versions of that location in the concerto data.

Thanks Bill. The sorting looks great!

We still have one small problem where the Current Copy link from the hold shelf is retrieving the item status page with the barcode number instead of the copy id.

I tried fixing it by adding what was previously used for the holds shelf list: {{item.hold.current_copy().id()}}
and then tried what I thought should work {{item.copy_id}} and {{current_copy.id}}

but none of these worked for me.

Otherwise, I think everything is ready to merge.

Thanks, Kathy. I took your {{item.copy_id}} approach and added the missing piece, a new grid field to fetch the current copy id. (Flattener-based autogrids only retrieve the fields they are told to fetch). That fix is squashed and the whole shebang is rebased.

However! The "Loading...0" indicator that appears and disappears along the top when the grid is working, which causes the grid to shuffle in the page, is now bugging me. The solution for this will be the progress dialog from bug #1522638. If we can get one more set of eyes / merging on that, then I'll migrate this code over to the progress dialog as well.

Here's a new branch, rebased over the progress dialog code from bug #1522638, plus a new commit to add a progress dialog for pull list grid and when collecting data for printing.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1653001-webstaff-idl-pull-list-progress

Once the code from bug #1522638 is merged, only the (3) LP#1653001-tagged commits from this branch will need to be merged.

Thank you Bill! Merged to master for inclusion in 2.12!