From 5377bfc6240587b113f2b8ee26a8006f65e756a7 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 19 May 2020 08:35:06 -0400 Subject: [PATCH] Bug 25541: Add ability to prevent checkin via SIP of items with holds Some libraries would like patrons to be unable to return items with holds via SIP. Instead, the screen message should indicate that the patron should return that item at the circ desk so a librarian can use it to fill the next hold right away and place it on the hold shelf. Test Plan: 1) Apply this patch. 2) Place a hold for an item. 3) Enable the new SIP option no_holds_checkin for a SIP account. 4) Restart the SIP server. 5) Check in the item using the SIP CLI tool using the SIP account for which you set the new option. 6) Note the checkin fails with a screen message indicating you should return the item to the circulation desk. Signed-off-by: Peter Lau Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart --- C4/SIP/ILS.pm | 12 ++++++++++-- C4/SIP/ILS/Transaction/Checkin.pm | 15 +++++++++++---- C4/SIP/Sip/MsgType.pm | 2 +- etc/SIPconfig.xml | 3 ++- t/db_dependent/SIP/Message.t | 25 ++++++++++++++++++++++++- 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index 7cff9c0579..80c14de718 100644 --- a/C4/SIP/ILS.pm +++ b/C4/SIP/ILS.pm @@ -197,7 +197,12 @@ sub test_cardnumber_compare { } sub checkin { - my ( $self, $item_id, $trans_date, $return_date, $current_loc, $item_props, $cancel, $checked_in_ok, $cv_triggers_alert ) = @_; + my ( $self, $item_id, $trans_date, $return_date, $current_loc, $item_props, $cancel, $account ) = @_; + + my $checked_in_ok = $account->{checked_in_ok}; + my $cv_triggers_alert = $account->{cv_triggers_alert}; + my $no_holds_checkin = $account->{no_holds_checkin}; + my ( $patron, $item, $circ ); $circ = C4::SIP::ILS::Transaction::Checkin->new(); @@ -207,7 +212,7 @@ sub checkin { my $data; if ($item) { - $data = $circ->do_checkin( $current_loc, $return_date, $cv_triggers_alert, $checked_in_ok ); + $data = $circ->do_checkin( $current_loc, $return_date, $account ); } else { $circ->alert(1); @@ -220,6 +225,9 @@ sub checkin { if ( !$circ->ok && $circ->alert_type && $circ->alert_type == 98 ) { # data corruption $circ->screen_msg("Checkin failed: data problem"); siplog( "LOG_WARNING", "Problem with issue_id in issues and old_issues; check the about page" ); + } elsif ( $data->{messages}->{ResFound} && !$circ->ok && $no_holds_checkin ) { + $circ->screen_msg("Item is on hold, please return to circulation desk"); + siplog ("LOG_DEBUG", "C4::SIP::ILS::Checkin - item withdrawn"); } elsif ( $data->{messages}->{withdrawn} && !$circ->ok && C4::Context->preference("BlockReturnOfWithdrawnItems") ) { $circ->screen_msg("Item withdrawn, return not allowed"); siplog ("LOG_DEBUG", "C4::SIP::ILS::Checkin - item withdrawn"); diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm index 10cd87b5ad..8c54bfb6ec 100644 --- a/C4/SIP/ILS/Transaction/Checkin.pm +++ b/C4/SIP/ILS/Transaction/Checkin.pm @@ -48,8 +48,11 @@ sub do_checkin { my $self = shift; my $branch = shift; my $return_date = shift; - my $cv_triggers_alert = shift; - my $checked_in_ok = shift; + my $account = shift; + + my $checked_in_ok = $account->{checked_in_ok}; + my $cv_triggers_alert = $account->{cv_triggers_alert}; + my $no_holds_checkin = $account->{no_holds_checkin}; if (!$branch) { $branch = 'SIP2'; @@ -113,13 +116,17 @@ sub do_checkin { $self->alert_type('04'); # send to other branch } if ($messages->{ResFound}) { - $self->hold($messages->{ResFound}); - if ($branch eq $messages->{ResFound}->{branchcode}) { + if ($no_holds_checkin) { + $self->alert_type('99'); + $return = 0; + } elsif ($branch eq $messages->{ResFound}->{branchcode}) { + $self->hold($messages->{ResFound}); $self->alert_type('01'); ModReserveAffect( $messages->{ResFound}->{itemnumber}, $messages->{ResFound}->{borrowernumber}, 0, $messages->{ResFound}->{reserve_id}); } else { + $self->hold($messages->{ResFound}); $self->alert_type('02'); ModReserveAffect( $messages->{ResFound}->{itemnumber}, $messages->{ResFound}->{borrowernumber}, 1, $messages->{ResFound}->{reserve_id}); diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index a365107c8a..d9f8901c06 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -658,7 +658,7 @@ sub handle_checkin { siplog( "LOG_WARNING", "received no-block checkin from terminal '%s'", $account->{id} ); $status = $ils->checkin_no_block( $item_id, $trans_date, $return_date, $item_props, $cancel ); } else { - $status = $ils->checkin( $item_id, $trans_date, $return_date, $my_branch, $item_props, $cancel, $account->{checked_in_ok}, $account->{cv_triggers_alert} ); + $status = $ils->checkin( $item_id, $trans_date, $return_date, $my_branch, $item_props, $cancel, $account ); } $patron = $status->patron; diff --git a/etc/SIPconfig.xml b/etc/SIPconfig.xml index f140ff253e..570192614d 100644 --- a/etc/SIPconfig.xml +++ b/etc/SIPconfig.xml @@ -59,7 +59,8 @@ da_field_template="[% patron.surname %][% IF patron.firstname %], [% patron.firstname %][% END %]" av_field_template="[% accountline.description %] [% accountline.amountoutstanding | format('%.2f') %]" hide_fields="BD,BE,BF,PB" - register_id=''> + register_id='' + no_holds_checkin="1"> diff --git a/t/db_dependent/SIP/Message.t b/t/db_dependent/SIP/Message.t index 955652d33b..9690f93c73 100755 --- a/t/db_dependent/SIP/Message.t +++ b/t/db_dependent/SIP/Message.t @@ -29,6 +29,7 @@ use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; +use C4::Reserves qw(AddReserve); use Koha::Database; use Koha::AuthUtils qw(hash_password); use Koha::DateUtils; @@ -36,6 +37,7 @@ use Koha::Items; use Koha::Checkouts; use Koha::Old::Checkouts; use Koha::Patrons; +use Koha::Holds; use C4::SIP::ILS; use C4::SIP::ILS::Patron; @@ -67,7 +69,7 @@ subtest 'Testing Patron Info Request V2' => sub { subtest 'Checkin V2' => sub { my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; - plan tests => 29; + plan tests => 33; $C4::SIP::Sip::protocol_version = 2; test_checkin_v2(); $schema->storage->txn_rollback; @@ -596,6 +598,27 @@ sub test_checkin_v2 { is( substr($response,5,1), 'N', 'Alert flag is not set' ); is( Koha::Checkouts->find( $issue->issue_id ), undef, 'Issue record is gone now' ); + + # Test account option no_holds_check that prevents items on hold from being checked in via SIP + Koha::Old::Checkouts->search({ issue_id => $issue->issue_id })->delete; + $server->{account}->{no_holds_checkin} = 1; + my $reserve_id = AddReserve({ + branchcode => $branchcode, + borrowernumber => $patron1->{borrowernumber}, + biblionumber => $item_object->biblionumber, + priority => 1, + }); + my $hold = Koha::Holds->find( $reserve_id ); + is( $hold->id, $reserve_id, "Hold was created successfully" ); + undef $response; + $msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 ); + $msg->handle_checkin( $server ); + is( substr($response,2,1), '0', 'OK flag is false when we check in an item on hold and we do not allow it' ); + is( substr($response,5,1), 'Y', 'Alert flag is set' ); + check_field( $respcode, $response, FID_SCREEN_MSG, 'Item is on hold, please return to circulation desk', 'Screen message is correct' ); + $hold->delete(); + $server->{account}->{no_holds_checkin} = 0; + } sub test_hold_patron_bcode { -- 2.39.5