TPAC > Hold doesn't save selection_depth

Bug #1064651 reported by Steve Callender on 2012-10-09
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Undecided
Unassigned
2.2
Undecided
Unassigned
2.3
Undecided
Unassigned

Bug Description

I tested this in 2.2.2 TPAC.

In sub test_and_create_hold_batch in Holds.pm, it doesn't look like the selection_depth is getting passed through to save with the hold. The selection depth on the hold is being passed as null and therefore defaulting to 0 no matter what the hold boundary is set at. It does look like it will initially use the correct hold boundary when checking for copies, but things such as the hold queue position are not working properly due to the boundaries not saving correctly with the hold. I suspect any subsequent hold targeting is also failing.

Looks like the depth needs to be specified i the subroutine.

Changed in evergreen:
status: New → Triaged
James Fournie (jfournie) wrote :

It looks like test_and_create_hold_batch() is calling a separate method, open-ils.circ.title_hold.is_possible. That method captures the hold boundary from the settings and returns it, however it looks like test_and_create_hold_batch doesn't pay much heed as to what title_hold.is_possible returns at all, and instead just passes the same parameters it passes into title_hold.is_possible into open-ils.circ.holds.create. It's a reasonable assumption that title_hold.is_possible doesn't enrich the hold request, but unfortunately it does.

I have a fix to be pushed to user/jamesrf/lp1064651_selection_depth_fix. This fix is critical for Sitka in order to use the TPAC in the staff client -- without it, copies outside of boundaries will be targetted, which causes confusion for patrons and libraries.

However, the fix is not ideal, it simply makes test_and_create_hold_batch() check for and use the value returned by title_hold.is_possible, some code refactoring is probably needed.

Jason Stephenson (jstephenson) wrote :

I definitely agree that refactoring of the holds code is called for. Maybe we can hammer away at it during the hackfest at the conference in April.

I'll take a look at your branch and see how it plays with my changes in bug 1076062.

Thanks.

Changed in evergreen:
status: Triaged → In Progress
assignee: nobody → Jason Stephenson (jstephenson)
Changed in evergreen:
status: In Progress → Fix Committed
milestone: none → 2.4.0-alpha
assignee: Jason Stephenson (jstephenson) → nobody
Jason Stephenson (jstephenson) wrote :

Fix is trivial and works for me! Thanks, James.

backported to 2.2 and 2.3.

Ben Shum (bshum) on 2013-05-19
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