web client: notices column picker option not available in holds interfaces

[Kathy Lussier]

The column picker option that displays the number of notices sent for a hold is not available in the web client's hold interfaces. It is mostly needed in the Patron Holds and Holds Shelf interfaces, but is also useful in the Record Holds interface.

[Andrea Neiman (aneiman)]

Confirmed 3.0 beta

[John Amundson (jamundson)]

Tested 3.0.6

I'd like to add that the Last Notify Time column is also not available. I agree these are most needed in the Holds Shelf and Patron Holds interfaces.

[Kathy Lussier (klussier)]

Adding a note that the code in bug 1712854 adds the notice count and the last notify time columns in the holds shelf interface. I haven't tested a notification to see if they actually work, but they are there.

[Kathy Lussier (klussier)]

It also adds it to record holds. The code doesn't touch patron holds, so we still have a need to add those columns in that interface.

[Kyle Huckins (khuckins)]

I've pushed a branch here, migrating the Patron Holds UI over to the wide_holds API introduced in bug 1712854, allowing us access to the notice count and last notify time columns: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1712861-notices-column-picker-patron-holds

[Kyle Huckins (khuckins)]

I added the pullrequest prematurely previously, re-adding it now after a bit more thought has been put into the code changes.

[Bill Erickson (berick)]

Thanks, Kyle. +1 to wide_holds migration. Did some testing and code review and have a few comments.

1. The Recently Canceled holds tab needs to honor the 'circ.holds.canceled.display_count' and 'circ.holds.canceled.display_age' org unit settings.

In the current code, this is addressed on the API side (see open-ils.circ.holds.retrieve and related APIs).

2. It looks like the default sort order is changing in the new code. Some special sort handling exists in open-ils.circ.holds.retrieve, etc. It looks like the wide_holds API will support these sort additions with some syntax modification (e.g. sorting on "clear_me").

[Kyle Huckins (khuckins)]

Thanks for the feedback Bill! I've pushed a small commit that honors the display_count - the best way to do this here seemed to be on the JS side of things, making use of the existing limit parameter. 'display_age' looks like it'll want to be done on the API side of things, but I have an idea on a slight expansion of the restrictions parameter. There's handling for {not: blah}, handling for {greaterThan/lessThan} may be useful in this and other cases as well, so unless there's objection there, that's where I'm thinking of starting.

[Kyle Huckins (khuckins)]

I've made some changes and squashed them into a single commit(and a second for release notes) - Jumped the gun on the pullrequest above, as the sorting issues still persist, however there are some changes to the wide_holds that are included in the latest version of this code - it will now support multiple conditional restrictions(previously it only supported 'not').

[Kyle Huckins (khuckins)]

Okay, one last squash and now it should be ready. No changes to the API were needed(I was hesitant to modify the API for this as it's so context-specific), but instead making sure not to overwrite order_by, and ensuring its rules matched the rules set in the holds.retrieve API seems to have done the trick.

[Bill Erickson (berick)]

I have pushed a follow-up branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1712861-notices-column-picker-patron-holds

Includes sign off and a batch of minor repairs. Main fix related to how the order_by's are defined to match the requirements of the API.

I'm going to target this for 3.4 only since this is a fairly big change to the UI that requires the grid display settings be reapplied / saved after updating.

[Chris Sharp (chrissharp123)]

Tested this and found an error that Bill corrected. I added that correction in the top commit of my signoff branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1712861-notices-column-picker-patron-holds

[Terran McCanna (tmccanna)]

Blake attempted to put this on a master sandbox and got a perl compilation error.

[Kyle Huckins (khuckins)]

Thanks everyone! This branch should get that Perl compiling correctly, an additional syntax correction needed to be made: user/khuckins/lp1712861-notices-column-picker-patron-holds-perl-corrections

[Kyle Huckins (khuckins)]

Oops, somehow managed to only copy the branch name and not the full URL: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1717366-patron-summary-autocollapse-rebase

[Kyle Huckins (khuckins)]

And wrong link too, oops:

Third time's the charm:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1712861-notices-column-picker-patron-holds-perl-corrections

[Ruth Frasur Davis (redavis)]

I have tested this code and consent to signing off on it with my name, rfrasur, and my email address, <email address hidden>.

[John Amundson (jamundson)]

I have just tested this, as well, and was about to add my sign off into the mix.

However, one thing I noticed is that my hold tab grid customization reverted after installing the patch. I didn't think too much of this initially, but I thought I'd check stored preferences just to see what happened.

There is a new preference called eg.grid.circ.patron.wide_holds that takes precedence over the existing eg.grid.circ.patron.holds.

That makes sense based on the conversations on this bug.

However, eg.grid.circ.patron.wide_holds is stored as an in-browser preference. It should be stored as a server pref to follow other grids.

[Kyle Huckins (khuckins)]

I've pushed up an initial rebase to this here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1712861-notices-column-picker-patron-holds-perl-corrections-v2

It does not address the feedback given in #18, and _may_ blow away some changes that have been made since, but should be a good start at getting these changes current. Will be looking at making sure nothing got lost in the rebase next, and then will move that preference over

[Kyle Huckins (khuckins)]

Potential issues could be in Open-ILS/src/templates/staff/circ/patron/t_holds_list.tt2, where I'll need to triple-check the fields being removed and added, in case of any changes in the past two years

[Blake GH (bmagic)]

The post branch link (assumed to be THE branch) - does not merge with master at the moment.