TPAC > Hold doesn't save selection_depth

[Steve Callender]

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.

[James Fournie (jfournie)]

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)]

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.

[Jason Stephenson (jstephenson)]

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

backported to 2.2 and 2.3.