open-ils.circ.hold.change_title(.specific_holds) APIs cancel previously captured holds at other locations, confusing staff and patrons

Bug #1312824 reported by Jason Boyer on 2014-04-25
18
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Evergreen
Medium
Unassigned
2.11
Medium
Unassigned

Bug Description

Evergreen: All currently supported
OpenSRF: *
PgSQL: *
OS: *

Both of these APIs call search_action_hold_request in this manner:

my $holds = $e->search_action_hold_request(
         [
             {
                 cancel_time => undef,
                 fulfillment_time => undef,
                 hold_type => 'T',
                 target => $bib_ids
             },
             {
                 flesh => 1,
                 flesh_fields => { ahr => ['usr'] }
             }
         ],
         { substream => 1 }
     );

The worst issue with these calls is that they don't check for capture_time => undef, meaning that items currently waiting for pickup can suddenly "vanish" from the holds shelf. This is an easy fix and fixes an infuriating issue.

More subtly, however, is that transits are also canceled as part of this process. This is nightmarish if your courier system isn't highly accurate. Items show up at their destinations already "available" if they show up at all. This one I don't know how to manage since ahr doesn't "know" that copies are in transit.

Branch incoming to at least take care of the capture_time check, I'm less sure how best to not cancel transits in-flight.

Jason Boyer (jboyer) on 2014-04-25
tags: added: holds
summary: - open-ils.circ.hold.change_title(.specific_holds) APIs casuse havoc when
+ open-ils.circ.hold.change_title(.specific_holds) APIs cause havoc when
used
Jeff Godin (jgodin) on 2014-06-10
Changed in evergreen:
status: New → Confirmed
importance: Undecided → Medium
assignee: nobody → Jeff Godin (jgodin)
milestone: none → 2.next
milestone: 2.next → 2.5.6
Mike Rylander (mrylander) wrote :

Jason,

Your branch seems like it should address the transit issue, since capture time (but not shelf time) should be set when the item is captured for the hold, regardless of the transit. Or, have you confirmed that that's not the case for some reason?

Jason Boyer (jboyer) wrote :

Mike, that was a misunderstanding on my part. I assumed (but did not check :/ ) that capture time wasn't set until the item actually arrived at its transit destination.

Jason Boyer (jboyer) on 2014-07-01
summary: - open-ils.circ.hold.change_title(.specific_holds) APIs cause havoc when
- used
+ open-ils.circ.hold.change_title(.specific_holds) APIs cancel previously
+ captured holds at other locations, confusing staff and patrons
Galen Charlton (gmc) on 2014-09-09
Changed in evergreen:
milestone: 2.5.7 → 2.5.8
Jason Boyer (jboyer) on 2015-05-08
tags: added: pullrequest

Jason, I'd like to test this but I don't fully understand the problem. Could you create a rough script for showing the problem and then testing your fix, something I can use to setup the problem situation on a test machine. Along the lines of Galen's QA proposal?

Jason Boyer (jboyer) wrote :

Josh, it's been a while so the particulars may be a little fuzzy, but I believe this should demonstrate the issue:

1. Create a bib with multiple holdings
2. Add multiple holds to this title and capture one of them - Item will have a status of On Holds Shelf
3. Open another bib record, open Actions for this Record menu, Mark as Hold Transfer Destination
4. Return to the record that has a captured hold, open Actions for this Record, Transfer all Title Holds
5. Check Captured hold, Item status is Available, hold has been reset and now points to the new bib record.

With the patch applied, steps 2-5 above should be repeated, but this time in step 5 the hold will still be on the original record and the item's status will still be On Holds Shelf.

Jeff Godin (jgodin) on 2016-05-31
Changed in evergreen:
assignee: Jeff Godin (jgodin) → nobody
Kathy Lussier (klussier) on 2016-05-31
Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
Jason Boyer (jboyer) on 2016-05-31
Changed in evergreen:
milestone: 2.5.8 → 2.next
Galen Charlton (gmc) on 2017-05-19
Changed in evergreen:
assignee: Kathy Lussier (klussier) → nobody
Kathy Lussier (klussier) on 2017-05-19
Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
Kathy Lussier (klussier) wrote :

This looks good to me and worked well for the Transfer all holds and transfer specific holds in the xul client. It also works in the web client.

I signed off on Jason's commit and added my own commit to change the holds transferred success message. I know it's not a good idea to introduce string changes for a bug fix, but I think it's important that success message say that the holds that were transferred are only ones that are not available for pickup. Otherwise, I can see lots of instances where staff getting annoyed when they see holds remaining on the bib.

The working branch with my signoff and the new commit is available at http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kmlussier/LP1312824-change_title_capture_time.

Changed in evergreen:
assignee: Kathy Lussier (klussier) → nobody
Galen Charlton (gmc) on 2017-05-25
tags: added: signedoff
Galen Charlton (gmc) wrote :

Pushed to master, rel_2_12, and rel_2_11. Thanks, Jason and Kathy!

Changed in evergreen:
milestone: 3.next → 2.12.3
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers