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

Bug #1712861 reported by Kathy Lussier
62
This bug affects 10 people
Affects Status Importance Assigned to Milestone
Evergreen
Confirmed
Medium
Unassigned

Bug Description

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.

Revision history for this message
Andrea Neiman (aneiman) wrote :

Confirmed 3.0 beta

Changed in evergreen:
status: New → Confirmed
milestone: none → 3.next
Kathy Lussier (klussier)
tags: added: webstaffcolumns
Revision history for this message
John Amundson (jamundson) wrote :

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.

Revision history for this message
Kathy Lussier (klussier) wrote :

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.

Revision history for this message
Kathy Lussier (klussier) wrote :

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)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
Revision history for this message
Kyle Huckins (khuckins) wrote :

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

Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
tags: added: pullrequest
Kyle Huckins (khuckins)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
tags: removed: pullrequest
Revision history for this message
Kyle Huckins (khuckins) wrote :

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

tags: added: pullrequest
Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

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)
tags: removed: pullrequest
Revision history for this message
Kyle Huckins (khuckins) wrote :

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)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
Kyle Huckins (khuckins)
tags: added: pullrequest
Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
tags: removed: pullrequest
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
Revision history for this message
Kyle Huckins (khuckins) wrote :

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').

Revision history for this message
Kyle Huckins (khuckins) wrote :

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.

tags: added: pullrequest
Changed in evergreen:
assignee: Kyle Huckins (khuckins) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

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.

Changed in evergreen:
milestone: 3.next → 3.4-beta1
Galen Charlton (gmc)
Changed in evergreen:
milestone: 3.4-beta1 → 3.4-beta2
Galen Charlton (gmc)
Changed in evergreen:
milestone: 3.4-beta2 → 3.4.1
Revision history for this message
Chris Sharp (chrissharp123) wrote :

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

Changed in evergreen:
milestone: 3.4.1 → 3.4.2
Revision history for this message
Terran McCanna (tmccanna) wrote :

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

tags: added: needsrepatch
Revision history for this message
Kyle Huckins (khuckins) wrote :

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

Revision history for this message
Kyle Huckins (khuckins) wrote :
tags: removed: needsrepatch
Revision history for this message
Kyle Huckins (khuckins) wrote :
Changed in evergreen:
milestone: 3.4.2 → 3.4.3
Revision history for this message
Ruth Frasur (rfrasur) wrote :

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

tags: added: signedoff
Changed in evergreen:
milestone: 3.4.3 → 3.5.0
assignee: nobody → Jason Stephenson (jstephenson)
Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → John Amundson (jamundson)
Revision history for this message
John Amundson (jamundson) wrote :

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.

Changed in evergreen:
assignee: John Amundson (jamundson) → nobody
Changed in evergreen:
milestone: 3.5.0 → 3.5.1
Changed in evergreen:
milestone: 3.5.1 → 3.5.2
Changed in evergreen:
milestone: 3.5.2 → 3.6.1
Changed in evergreen:
milestone: 3.6.1 → 3.6.2
Changed in evergreen:
milestone: 3.6.2 → 3.6.3
tags: added: holds
removed: webstaffclient
Revision history for this message
Kyle Huckins (khuckins) wrote :

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

Revision history for this message
Kyle Huckins (khuckins) wrote :

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

Changed in evergreen:
milestone: 3.6.3 → 3.6.4
Changed in evergreen:
milestone: 3.6.4 → 3.7.2
Jason Boyer (jboyer)
tags: removed: signedoff
Dan Briem (dbriem)
tags: added: circ-holds
removed: holds
no longer affects: evergreen/3.4
no longer affects: evergreen/3.5
Changed in evergreen:
milestone: 3.7.2 → 3.7.3
Revision history for this message
Blake GH (bmagic) wrote :

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

tags: added: needsrebase
tags: removed: pullrequest
no longer affects: evergreen/3.6
Changed in evergreen:
milestone: 3.7.3 → none
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.