From 61a93b67d9df1359c228c46dfa7275866cd6ea34 Mon Sep 17 00:00:00 2001 From: Matthias Meusburger Date: Tue, 3 Nov 2020 14:49:31 +0000 Subject: [PATCH] Bug 26591: SIP option prevcheckout_block_checkout to block checkout of previously checked-out documents This patchs adds a new SIP option to block checkout of previously checked-out documents: prevcheckout_block_checkout See the CheckPrevCheckout system preference to enable previously checked-out verification in Koha. Test plan: 1) Apply this patch 2) Enable the CheckPrevCheckout syspref (on "Do" for instance) 3) Enable prevcheckout_block_checkout in the SIP server config file 4) Checkout and checkin an item for a user 5) Checkout the item again with the SIP CLI tool 6) Check that the SIP message is "This item was previously checked out by you" and that the item was not checked out 7) Disable prevcheckout_block_checkout in the SIP server config file 8) Checkout the item again with the SIP CLI tool 9) Check that the SIP message is "This item was previously checked out by you" and that the item was checked out. 10) Prove t/db_dependent/SIP/Message.t Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart (cherry picked from commit 86918091118d1e2ead99a943dfe783c7f1740789) Signed-off-by: Fridolin Somers --- C4/SIP/ILS.pm | 4 +- C4/SIP/ILS/Transaction/Checkout.pm | 3 ++ C4/SIP/Sip/MsgType.pm | 2 +- etc/SIPconfig.xml | 1 + t/db_dependent/SIP/Message.t | 76 +++++++++++++++++++++++++++++- 5 files changed, 82 insertions(+), 4 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index 2b71bcd176..3e5b6a4824 100644 --- a/C4/SIP/ILS.pm +++ b/C4/SIP/ILS.pm @@ -126,7 +126,7 @@ sub offline_ok { # the response. # sub checkout { - my ( $self, $patron_id, $item_id, $sc_renew, $fee_ack ) = @_; + my ( $self, $patron_id, $item_id, $sc_renew, $fee_ack, $account ) = @_; my ( $patron, $item, $circ ); $circ = C4::SIP::ILS::Transaction::Checkout->new(); @@ -153,7 +153,7 @@ sub checkout { $circ->screen_msg("Item checked out to another patron"); } else { - $circ->do_checkout(); + $circ->do_checkout($account); if ( $circ->ok ) { $debug and warn "circ is ok"; diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index aabe5e6830..e33a3c3f35 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -47,11 +47,13 @@ sub new { sub do_checkout { my $self = shift; + my $account = shift; siplog('LOG_DEBUG', "ILS::Transaction::Checkout performing checkout..."); my $shelf = $self->{item}->hold_attached; my $barcode = $self->{item}->id; my $patron = Koha::Patrons->find($self->{patron}->{borrowernumber}); my $overridden_duedate; # usually passed as undef to AddIssue + my $prevcheckout_block_checkout = $account->{prevcheckout_block_checkout}; $debug and warn "do_checkout borrower: . " . $patron->borrowernumber; my ($issuingimpossible, $needsconfirmation) = _can_we_issue($patron, $barcode, C4::Context->preference("AllowItemsOnHoldCheckoutSIP") @@ -95,6 +97,7 @@ sub do_checkout { } } elsif ($confirmation eq 'PREVISSUE') { $self->screen_msg("This item was previously checked out by you"); + $noerror = 0 if ($prevcheckout_block_checkout); last; } elsif ( $confirmation eq 'ADDITIONAL_MATERIALS' ) { $self->screen_msg('Item must be checked out at a circulation desk'); diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index 90896b2539..62d3d5fcfc 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -533,7 +533,7 @@ sub handle_checkout { # Does the transaction date really matter for items that are # checkout out while the terminal is online? I'm guessing 'no' - $status = $ils->checkout( $patron_id, $item_id, $sc_renewal_policy, $fee_ack ); + $status = $ils->checkout( $patron_id, $item_id, $sc_renewal_policy, $fee_ack, $account ); } $item = $status->item; diff --git a/etc/SIPconfig.xml b/etc/SIPconfig.xml index c03cd3e689..4277004f00 100644 --- a/etc/SIPconfig.xml +++ b/etc/SIPconfig.xml @@ -62,6 +62,7 @@ hide_fields="BD,BE,BF,PB" register_id='' holds_block_checkin="0" + prevcheckout_block_checkout="0" overdues_block_checkout="1"> diff --git a/t/db_dependent/SIP/Message.t b/t/db_dependent/SIP/Message.t index d4053672cd..8401e43421 100755 --- a/t/db_dependent/SIP/Message.t +++ b/t/db_dependent/SIP/Message.t @@ -21,7 +21,7 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 9; +use Test::More tests => 10; use Test::MockObject; use Test::MockModule; use Test::Warn; @@ -30,6 +30,7 @@ use t::lib::Mocks; use t::lib::TestBuilder; use C4::Reserves qw(AddReserve); +use C4::Circulation qw( AddReturn ); use Koha::Database; use Koha::AuthUtils qw(hash_password); use Koha::DateUtils; @@ -66,6 +67,15 @@ subtest 'Testing Patron Info Request V2' => sub { $schema->storage->txn_rollback; }; +subtest 'Checkout V2' => sub { + my $schema = Koha::Database->new->schema; + $schema->storage->txn_begin; + plan tests => 3; + $C4::SIP::Sip::protocol_version = 2; + test_checkout_v2(); + $schema->storage->txn_rollback; +}; + subtest 'Checkin V2' => sub { my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; @@ -504,6 +514,69 @@ sub test_request_patron_info_v2 { check_field( $respcode, $response, FID_SCREEN_MSG, '.+', 'But we have a screen msg', 'regex' ); } +sub test_checkout_v2 { + my $builder = t::lib::TestBuilder->new(); + my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; + my $branchcode2 = $builder->build({ source => 'Branch' })->{branchcode}; + my ( $response, $findpatron ); + my $mocks = create_mocks( \$response, \$findpatron, \$branchcode ); + + # create some data + my $patron1 = $builder->build({ + source => 'Borrower', + value => { + password => hash_password( PATRON_PW ), + }, + }); + my $card1 = $patron1->{cardnumber}; + my $sip_patron1 = C4::SIP::ILS::Patron->new( $card1 ); + $findpatron = $sip_patron1; + my $item_object = $builder->build_sample_item({ + damaged => 0, + withdrawn => 0, + itemlost => 0, + restricted => 0, + homebranch => $branchcode, + holdingbranch => $branchcode, + }); + + my $mockILS = $mocks->{ils}; + my $server = { ils => $mockILS, account => {} }; + $mockILS->mock( 'institution', sub { $branchcode; } ); + $mockILS->mock( 'supports', sub { return; } ); + $mockILS->mock( 'checkout', sub { + shift; + return C4::SIP::ILS->checkout(@_); + }); + my $today = dt_from_string; + t::lib::Mocks::mock_userenv({ branchcode => $branchcode, flags => 1 }); + t::lib::Mocks::mock_preference( 'CheckPrevCheckout', 'hardyes' ); + + my $issue = Koha::Checkout->new({ branchcode => $branchcode, borrowernumber => $patron1->{borrowernumber}, itemnumber => $item_object->itemnumber })->store; + my $return = AddReturn($item_object->barcode, $branchcode); + + my $siprequest = CHECKOUT . 'YN' . siprequestdate($today) . + siprequestdate( $today->clone->add( days => 1) ) . + FID_INST_ID . $branchcode . '|'. + FID_PATRON_ID . $sip_patron1->id . '|' . + FID_ITEM_ID . $item_object->barcode . '|' . + FID_TERMINAL_PWD . 'ignored' . '|'; + undef $response; + + my $msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 ); + $server->{account}->{prevcheckout_block_checkout} = 1; + $msg->handle_checkout( $server ); + my $respcode = substr( $response, 0, 2 ); + check_field( $respcode, $response, FID_SCREEN_MSG, 'This item was previously checked out by you', 'Check screen msg', 'equals' ); + + is( Koha::Checkouts->search({ itemnumber => $item_object->id })->count, 0, "Item was not checked out (prevcheckout_block_checkout enabled)"); + + $server->{account}->{prevcheckout_block_checkout} = 0; + $msg->handle_checkout( $server ); + $respcode = substr( $response, 0, 2 ); + is( Koha::Checkouts->search({ itemnumber => $item_object->id })->count, 1, "Item was checked out (prevcheckout_block_checkout disabled)"); +} + sub test_checkin_v2 { my $builder = t::lib::TestBuilder->new(); my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; @@ -774,5 +847,6 @@ sub fixed_length { #length of fixed fields including response code ( PATRON_STATUS_RESP ) => 37, ( PATRON_INFO_RESP ) => 61, ( CHECKIN_RESP ) => 24, + ( CHECKOUT_RESP ) => 24, }->{$_[0]}; } -- 2.39.5