Attachment 109534 Details for Bug 25541 – Bug 25541: Add ability to prevent checkin via SIP of items with holds

[patch] Bug 25541: Add ability to prevent checkin via SIP of items with holds

Bug-25541-Add-ability-to-prevent-checkin-via-SIP-o.patch (text/plain), 8.24 KB, created by Martin Renvoize (ashimema) on 2020-09-02 13:42:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2020-09-02 13:42:19 UTC

Size: 8.24 KB

patch

obsolete

>From 42608b3812da0fccc3f5229be5c878dba80a2f6e Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>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 <peter.lau@yccece.edu.hk>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> 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">
>           <screen_msg_regex find="Greetings from Koha." replace="Welcome to your library!" />
>           <screen_msg_regex find="Invalid patron barcode." replace="Barcode not found, are you sure this is your library card?" />
>           <patron_attribute field="XY" code="CODE" />
>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.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 25541: 105090 | 105098 | 107791 | 108626 | 109519 | 109534 | 109535 | 109536 | 109537