LP1076062: Hold overrides not working properly.
authorJason Stephenson <jstephenson@mvlc.org>
Mon, 10 Dec 2012 15:29:21 +0000 (10:29 -0500)
committerBill Erickson <berick@esilibrary.com>
Wed, 16 Jan 2013 15:26:33 +0000 (10:26 -0500)
commite6b376997ac078b422fee8785923ff1eac73b9c0
tree26b4345bc7c6179e833922156b5de20913f222e5
parent2474dff73de53a7f774950431fa2a34e5b717331
LP1076062: Hold overrides not working properly.

Edit the verify_copy_for_hold helper function to actually work.  There
were cases where it would permit a copy when it shouldn't and also
disallow a copy when it should have allowed it.  It now makes better
use of the oargs override argument.

If oargs has an events member, the method now removes the matching from
the list of failure events returned by the permit_copy_hold check.  It
was previously bugged, stopping on the first matching event, clearing
the list of failed and events, and returning a value to indicate that
the copy is permitted for the hold, even when it may not be.

If oargs has the "all" member and there are failure events remaining,
the function loops through those events checking if the requestor has
the override permission for the event.  If the requestor does have the
override permission, then the event is added to oargs->{events} to be
saved for future checks on future copies.  It the requestor does not
have the override permission, then the event is pushed onto a new array
of failed events and also onto oargs->{failed}.  The latter is kept
to avoid looking up events repeatedly on future copy checks.

The oargs->{failed} member is added as a shortcut to avoid repeatedly
looking up override permissions when the requestor does not have them.
It is actually checked, when present, before the user permission is
checked with a costly database lookup.

Since verify_copy_for_hold has no other way to determine that an override
was requested, it checks for the events and/or all members of oargs being
set and having a value that would evaluate to true.  If oargs is undefined,
a hashref with no members, or lacks a "true" events or all member, then
verify_copy_for_hold functions as though no overrides are requested.

Additionally, all functions that call verify_copy_for_hold, either directly
or indirectly, have had their intro logic modified to only set oargs when
an override is requested and oargs is not already set.  We make the assump-
tion that if oargs is set, it contains the events member.  (Perhaps that is
a poor assumption, but all of the code so far looks like it will work.)

Fix test_and_create_hold_batch so that it passes the oargs parameter into
open-ils.circ.title_hold.is_possible in the way that check_title_hold (the
implementation function) actually expects it to be passed.  This also means
that we need to delete oargs from the params hashref before creating the
hold objects, as that would blow up if it were present.

Signed-off-by: Jason Stephenson <jstephenson@mvlc.org>
Signed-off-by: Bill Erickson <berick@esilibrary.com>
Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm