Completing Force/Recall Holds

Add support for the Force and Recall hold types to be something other than
just aliases of Copy holds.

Both holds are made to cut in line over *all* other holds and both ignore
all hold rules, including copy, location, and status holdable flags.

Recall holds, when they reach their destination, will be flagged to be sent
to cataloging.

In addition, to place either kind of hold you need a new permission at the
copy's circulation library:

COPY_HOLD_FORCE for force holds
COPY_HOLD_RECALL for recall holds

Also, some re-ordering of logic should result in a fix for issues with
capturing holds as transits and/or suppressing holds and the hold shelf
logic.

Signed-off-by: Thomas Berezansky <tsbere@mvlc.org>
Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
index 999ac68..e5d572c 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Circulate.pm
@@ -2511,6 +2511,17 @@ sub do_checkin {
                 $self->fake_hold_dest(0);
                 return if $self->bail_out;
 
+            } elsif ($hold and $hold->hold_type eq 'R') {
+
+                $self->copy->status(OILS_COPY_STATUS_CATALOGING);
+                $self->notify_hold(0); # No need to notify
+                $self->fake_hold_dest(0);
+                $self->noop(1); # Don't try and capture for other holds/transits now
+                $self->update_copy();
+                $hold->fulfillment_time('now');
+                $self->bail_on_events($self->editor->event)
+                    unless $self->editor->update_action_hold_request($hold);
+
             } else {
 
                 # hold transited to correct location
@@ -2911,12 +2922,24 @@ sub attempt_checkin_hold_capture {
 
     $self->retarget($retarget);
 
+    my $suppress_transit = 0;
+    if( $hold->pickup_lib != $self->circ_lib and not $self->hold_as_transit ) {
+        my $suppress_transit_circ = $U->ou_ancestor_setting($self->circ_lib, 'circ.transit.suppress_hold');
+        if($suppress_transit_circ && $suppress_transit_circ->{value}) {
+            my $suppress_transit_pickup = $U->ou_ancestor_setting($hold->pickup_lib, 'circ.transit.suppress_hold');
+            if($suppress_transit_pickup && $suppress_transit_circ->{value} eq $suppress_transit_pickup->{value}) {
+                $suppress_transit = 1;
+                $self->hold->pickup_lib($self->circ_lib);
+            }
+        }
+    }
+
     $logger->info("circulator: found permitted hold ".$hold->id." for copy, capturing...");
 
     $hold->current_copy($copy->id);
     $hold->capture_time('now');
     $self->put_hold_on_shelf($hold) 
-        if $hold->pickup_lib == $self->circ_lib;
+        if ($suppress_transit || ($hold->pickup_lib == $self->circ_lib and not $self->hold_as_transit) );
 
     # prevent DB errors caused by fetching 
     # holds from storage, and updating through cstore
@@ -2934,26 +2957,22 @@ sub attempt_checkin_hold_capture {
 
     return 0 if $self->bail_out;
 
-    my $suppress_transit = 0;
-    if( $hold->pickup_lib != $self->circ_lib and not $self->hold_as_transit ) {
-        my $suppress_transit_circ = $U->ou_ancestor_setting($self->circ_lib, 'circ.transit.suppress_hold');
-        if($suppress_transit_circ && $suppress_transit_circ->{value}) {
-            my $suppress_transit_pickup = $U->ou_ancestor_setting($hold->pickup_lib, 'circ.transit.suppress_hold');
-            if($suppress_transit_pickup && $suppress_transit_circ->{value} eq $suppress_transit_pickup->{value}) {
-                $suppress_transit = 1;
-                $self->hold->pickup_lib($self->circ_lib);
-            }
-        }
-    }
-
     if( $suppress_transit or ( $hold->pickup_lib == $self->circ_lib && not $self->hold_as_transit ) ) {
 
-        # This hold was captured in the correct location
-        $copy->status(OILS_COPY_STATUS_ON_HOLDS_SHELF);
-        $self->push_events(OpenILS::Event->new('SUCCESS'));
+        if ($hold->hold_type eq 'R') {
+            $copy->status(OILS_COPY_STATUS_CATALOGING);
+            $hold->fulfillment_time('now');
+            $self->noop(1); # Block other transit/hold checks
+            $self->bail_on_events($self->editor->event)
+                unless $self->editor->update_action_hold_request($hold);
+        } else {
+            # This hold was captured in the correct location
+            $copy->status(OILS_COPY_STATUS_ON_HOLDS_SHELF);
+            $self->push_events(OpenILS::Event->new('SUCCESS'));
 
-        #$self->do_hold_notify($hold->id);
-        $self->notify_hold($hold->id);
+            #$self->do_hold_notify($hold->id);
+            $self->notify_hold($hold->id);
+        }
 
     } else {
     
@@ -2967,6 +2986,7 @@ sub attempt_checkin_hold_capture {
 
     # make sure we save the copy status
     $self->update_copy;
+    return 0 if $copy->status == OILS_COPY_STATUS_CATALOGING;
     return 1;
 }
 

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
index 49558b8..d083747 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Circ/Holds.pm
@@ -288,10 +288,14 @@ sub create_hold {
         return $e->die_event unless $e->allowed('ISSUANCE_HOLDS', $porg);
     } elsif ( $t eq OILS_HOLD_TYPE_COPY ) {
         return $e->die_event unless $e->allowed('COPY_HOLDS',   $porg);
-    } elsif ( $t eq OILS_HOLD_TYPE_FORCE ) {
-        return $e->die_event unless $e->allowed('COPY_HOLDS',   $porg);
-    } elsif ( $t eq OILS_HOLD_TYPE_RECALL ) {
-        return $e->die_event unless $e->allowed('COPY_HOLDS',   $porg);
+    } elsif ( $t eq OILS_HOLD_TYPE_FORCE || $t eq OILS_HOLD_TYPE_RECALL ) {
+               my $copy = $e->retrieve_asset_copy($hold->target)
+                       or return $e->die_event;
+        if ( $t eq OILS_HOLD_TYPE_FORCE ) {
+            return $e->die_event unless $e->allowed('COPY_HOLDS_FORCE',   $copy->circ_lib);
+        } elsif ( $t eq OILS_HOLD_TYPE_RECALL ) {
+            return $e->die_event unless $e->allowed('COPY_HOLDS_RECALL',   $copy->circ_lib);
+        }
     }
 
     if( @events ) {
@@ -2285,6 +2289,7 @@ sub do_possibility_checks {
         return $e->event unless $volume = $e->retrieve_asset_call_number($copy->call_number);
         return $e->event unless $title  = $e->retrieve_biblio_record_entry($volume->record);
 
+        return (1, 1, []) if( $hold_type eq OILS_HOLD_TYPE_RECALL || $hold_type eq OILS_HOLD_TYPE_FORCE);
         return verify_copy_for_hold( 
             $patron, $e->requestor, $title, $copy, $pickup_lib, $request_lib
         );
@@ -2848,10 +2853,6 @@ sub find_nearest_permitted_hold {
                } 
        );
 
-       # hold->type "R" means we need this copy
-       for my $h (@$old_holds) { return ($h) if $h->hold_type eq 'R'; }
-
-
     my $hold_stall_interval = $U->ou_ancestor_setting_value($user->ws_ou, OILS_SETTING_HOLD_SOFT_STALL);
 
        $logger->info("circulator: searching for best hold at org ".$user->ws_ou.
@@ -3519,7 +3520,7 @@ sub hold_has_copy_at {
         limit => 1
     };
 
-    if($hold_type eq 'C') {
+    if($hold_type eq 'C' or $hold_type eq 'F' or $hold_type eq 'R') {
 
         $query->{where}->{'+acp'}->{id} = $hold_target;
 

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
index d58e213..10f309f 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/Publisher/action.pm
@@ -307,7 +307,7 @@ sub nearest_hold {
                        AND h.cancel_time IS NULL
                        AND (h.expire_time IS NULL OR h.expire_time > NOW())
             AND h.frozen IS FALSE
-               ORDER BY $holdsort
+               ORDER BY CASE WHEN h.hold_type IN ('R','F') THEN 0 ELSE 1 END, $holdsort
                LIMIT $limit
        SQL
        
@@ -1039,7 +1039,7 @@ sub new_hold_copy_targeter {
                                                          frozen => 'f',
                                                          prev_check_time => { '<=' => $expire_threshold },
                                                        },
-                                                       { order_by => 'CASE WHEN hold_type = \'F\' THEN 0 ELSE 1 END, selection_depth DESC, request_time,prev_check_time' } ) ];
+                                                       { order_by => 'selection_depth DESC, request_time,prev_check_time' } ) ];
 
                        # find all the holds holds needing first time targeting
                        push @$holds, action::hold_request->search(
@@ -1048,7 +1048,7 @@ sub new_hold_copy_targeter {
                                                        prev_check_time => undef,
                                                        frozen => 'f',
                                                        cancel_time => undef,
-                                                       { order_by => 'CASE WHEN hold_type = \'F\' THEN 0 ELSE 1 END, selection_depth DESC, request_time' } );
+                                                       { order_by => 'selection_depth DESC, request_time' } );
                } else {
 
                        # find all the holds holds needing first time targeting ONLY
@@ -1058,7 +1058,7 @@ sub new_hold_copy_targeter {
                                                        prev_check_time => undef,
                                                        cancel_time => undef,
                                                        frozen => 'f',
-                                                       { order_by => 'CASE WHEN hold_type = \'F\' THEN 0 ELSE 1 END, selection_depth DESC, request_time' } ) ];
+                                                       { order_by => 'selection_depth DESC, request_time' } ) ];
                }
        } catch Error with {
                my $e = shift;
@@ -1230,14 +1230,17 @@ sub new_hold_copy_targeter {
                                push @$all_copies, $_cp if $_cp;
                        }
 
-                       # trim unholdables
-                       @$all_copies = grep {   isTrue($_->status->holdable) && 
-                                               isTrue($_->location->holdable) && 
-                                               isTrue($_->holdable) &&
-                                               !isTrue($_->deleted) &&
-                                               (isTrue($hold->mint_condition) ? isTrue($_->mint_condition) : 1) &&
-                                               ($hold->hold_type ne 'P' ? $_->part_maps->count == 0 : 1)
-                                       } @$all_copies;
+            # Force and recall holds bypass pretty much everything
+            if ($hold->hold_type ne 'R' && $hold->hold_type ne 'F') {
+                       # trim unholdables
+                       @$all_copies = grep {   isTrue($_->status->holdable) && 
+                                               isTrue($_->location->holdable) && 
+                                               isTrue($_->holdable) &&
+                                               !isTrue($_->deleted) &&
+                                               (isTrue($hold->mint_condition) ? isTrue($_->mint_condition) : 1) &&
+                                                   ($hold->hold_type ne 'P' ? $_->part_maps->count == 0 : 1)
+                                       } @$all_copies;
+            }
 
                        # let 'em know we're still working
                        $client->status( new OpenSRF::DomainObject::oilsContinueStatus );
@@ -1351,8 +1354,13 @@ sub new_hold_copy_targeter {
 
                        $all_copies = [grep { $_->status == 0 || $_->status == 7 } grep {''.$_->circ_lib ne $pu_lib } @good_copies];
                        # $all_copies is now a list of copies not at the pickup library
-
-                       my $best = choose_nearest_copy($hold, $prox_list);
+                       
+            my $best;
+            if  ($hold->hold_type eq 'R' || $hold->hold_type eq 'F') { # Recall/Force holds bypass hold rules.
+                $best = $good_copies[0] if(scalar @good_copies);
+            } else {
+                $best = choose_nearest_copy($hold, $prox_list);
+            }
                        $client->status( new OpenSRF::DomainObject::oilsContinueStatus );
 
                        if (!$best) {

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm b/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
index 68d8c13..519382f 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/SIP/Patron.pm
@@ -461,7 +461,7 @@ sub __hold_to_title {
 
        return __copy_to_title($e, 
                $e->retrieve_asset_copy($hold->target)) 
-               if $hold->hold_type eq 'C';
+               if $hold->hold_type eq 'C' or $hold->hold_type eq 'F' or $hold->hold_type eq 'R';
 
        return __volume_to_title($e, 
                $e->retrieve_asset_call_number($hold->target))

diff --git a/Open-ILS/src/sql/Pg/950.data.seed-values.sql b/Open-ILS/src/sql/Pg/950.data.seed-values.sql
index 3d725ff..d16b927 100644 (file)
--- a/Open-ILS/src/sql/Pg/950.data.seed-values.sql
+++ b/Open-ILS/src/sql/Pg/950.data.seed-values.sql
@@ -1448,7 +1448,11 @@ INSERT INTO permission.perm_list ( id, code, description ) VALUES
  ( 515, 'UPDATE_PATRON_PRIMARY_CARD', oils_i18n_gettext( 515,
     'Allows a user to manually adjust a patron''s primary card', 'ppl', 'description')),
  ( 516, 'CREATE_REPORT_TEMPLATE', oils_i18n_gettext( 516,
-    'Allows a user to create report templates', 'ppl', 'description' ));
+    'Allows a user to create report templates', 'ppl', 'description' )),
+ ( 517, 'COPY_HOLDS_FORCE', oils_i18n_gettext( 517, 
+    'Allow a user to place a force hold on a specific copy', 'ppl', 'description' )),
+ ( 518, 'COPY_HOLDS_RECALL', oils_i18n_gettext( 518, 
+    'Allow a user to place a recall hold on a specific copy', 'ppl', 'description' ));
 
 SELECT SETVAL('permission.perm_list_id_seq'::TEXT, 1000);
 

diff --git a/Open-ILS/src/sql/Pg/upgrade/XXXX.copy_holds.sql b/Open-ILS/src/sql/Pg/upgrade/XXXX.copy_holds.sql
new file mode 100644 (file)
index 0000000..3575dc3
--- /dev/null
+++ b/Open-ILS/src/sql/Pg/upgrade/XXXX.copy_holds.sql
@@ -0,0 +1,5 @@
+INSERT INTO permission.perm_list ( id, code, description ) VALUES
+ ( 517, 'COPY_HOLDS_FORCE', oils_i18n_gettext( 517, 
+    'Allow a user to place a force hold on a specific copy', 'ppl', 'description' )),
+ ( 518, 'COPY_HOLDS_RECALL', oils_i18n_gettext( 518, 
+    'Allow a user to place a recall hold on a specific copy', 'ppl', 'description' ));