From 39547dfaf09f713bce63a3b9c5060d5bae1a3538 Mon Sep 17 00:00:00 2001 From: Thomas Berezansky Date: Thu, 6 Oct 2011 09:13:38 -0400 Subject: [PATCH] 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 Signed-off-by: Mike Rylander --- .../lib/OpenILS/Application/Circ/Circulate.pm | 56 +++++++++++++------ .../lib/OpenILS/Application/Circ/Holds.pm | 19 ++++--- .../Application/Storage/Publisher/action.pm | 36 +++++++----- .../src/perlmods/lib/OpenILS/SIP/Patron.pm | 2 +- Open-ILS/src/sql/Pg/950.data.seed-values.sql | 6 +- .../src/sql/Pg/upgrade/XXXX.copy_holds.sql | 5 ++ 6 files changed, 81 insertions(+), 43 deletions(-) create mode 100644 Open-ILS/src/sql/Pg/upgrade/XXXX.copy_holds.sql 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 999ac689db..e5d572c989 100644 --- 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 49558b81ba..d083747114 100644 --- 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 d58e213627..10f309f018 100644 --- 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 68d8c13655..519382ff80 100644 --- 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 3d725ff473..d16b927d96 100644 --- 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 index 0000000000..3575dc3a0c --- /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' )); -- 2.43.2