From b5052dca448a66bb84d0c81c8fdae340a45cbd66 Mon Sep 17 00:00:00 2001 From: Kyle Hall Date: Thu, 19 Jan 2023 13:45:07 -0500 Subject: [PATCH] Bug 32684: Implement SIP patron status field "too many items lost" The SIP patron status and information responses always return false foe "too many items lost". It would be reasonable to check the count of lost items still checked out to the patron and compare that to a threshold set in the sip config file. Though not all libraries operate in this way, it seems like a good and reasonable implementation as long is it is properly documented. This patch adds the ability to set the SIP "too many items lost" flag for a patron based on the number of lost checkouts the patron has where the lost flag on those items is greater than the given flag value. For example, one could specify that the flag be set if the patron has more than 2 items checked out where itemlost is greater than 3. By default the feature is disabled to retain the existing functionality. If enabled, the default itemlost minimum flag value is 1 unless specified. Test Plan: 1) Apply this patch 2) prove t/db_dependent/SIP/Message.t Signed-off-by: David Nind Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 877f8ed89890d5666608749781b7ac0958900fe8) Signed-off-by: Jacob O'Mara --- C4/SIP/Sip/MsgType.pm | 9 +++- etc/SIPconfig.xml | 2 + t/db_dependent/SIP/Message.t | 88 +++++++++++++++++++++++++++++++++++- 3 files changed, 96 insertions(+), 3 deletions(-) diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm index 7ba61959a3..658eb5eb91 100644 --- a/C4/SIP/Sip/MsgType.pm +++ b/C4/SIP/Sip/MsgType.pm @@ -1717,6 +1717,13 @@ sub patron_status_string { my $patron_status; + my $too_many_lost = 0; + if ( my $lost_block_checkout = $server->{account}->{lost_block_checkout} ) { + my $lost_block_checkout_value = $server->{account}->{lost_block_checkout_value} // 1; + my $lost_checkouts = Koha::Checkouts->search({ borrowernumber => $patron->borrowernumber, 'itemlost' => { '>=', $lost_block_checkout_value } }, { join => 'item'} )->count; + $too_many_lost = $lost_checkouts >= $lost_block_checkout; + } + siplog( "LOG_DEBUG", "patron_status_string: %s charge_ok: %s", $patron->id, $patron->charge_ok ); $patron_status = sprintf( '%s%s%s%s%s%s%s%s%s%s%s%s%s%s', @@ -1729,7 +1736,7 @@ sub patron_status_string { $server->{account}->{overdues_block_checkout} ? boolspace( $patron->too_many_overdue ) : q{ }, boolspace( $patron->too_many_renewal ), boolspace( $patron->too_many_claim_return ), - boolspace( $patron->too_many_lost ), + boolspace( $too_many_lost ), boolspace( $patron->excessive_fines ), boolspace( $patron->excessive_fees ), boolspace( $patron->recall_overdue ), diff --git a/etc/SIPconfig.xml b/etc/SIPconfig.xml index 8e0dcbef85..d64586b209 100644 --- a/etc/SIPconfig.xml +++ b/etc/SIPconfig.xml @@ -77,6 +77,8 @@ format_due_date="0" inhouse_item_types="" inhouse_patron_categories=""> + + diff --git a/t/db_dependent/SIP/Message.t b/t/db_dependent/SIP/Message.t index e985c3e27a..4c6fe1326c 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 => 15; +use Test::More tests => 16; use Test::Exception; use Test::MockObject; use Test::MockModule; @@ -31,7 +31,7 @@ use t::lib::Mocks; use t::lib::TestBuilder; use C4::Reserves qw( AddReserve ); -use C4::Circulation qw( AddReturn ); +use C4::Circulation qw( AddIssue AddReturn ); use Koha::Database; use Koha::AuthUtils qw(hash_password); use Koha::DateUtils qw( dt_from_string output_pref ); @@ -237,6 +237,90 @@ subtest 'Lastseen response' => sub { }; +subtest "Test patron_status_string" => sub { + my $schema = Koha::Database->new->schema; + $schema->storage->txn_begin; + + plan tests => 9; + + my $builder = t::lib::TestBuilder->new(); + my $branchcode = $builder->build({ source => 'Branch' })->{branchcode}; + my $patron = $builder->build({ + source => 'Borrower', + value => { + branchcode => $branchcode, + }, + }); + my $sip_patron = C4::SIP::ILS::Patron->new( $patron->{cardnumber} ); + + t::lib::Mocks::mock_userenv({ branchcode => $branchcode }); + + my $item1 = $builder->build_sample_item( + { + damaged => 0, + withdrawn => 0, + itemlost => 0, + restricted => 0, + homebranch => $branchcode, + holdingbranch => $branchcode, + permanent_location => "PERMANENT_LOCATION" + } + ); + AddIssue( $patron, $item1->barcode ); + + my $item2 = $builder->build_sample_item( + { + damaged => 0, + withdrawn => 0, + itemlost => 0, + restricted => 0, + homebranch => $branchcode, + holdingbranch => $branchcode, + permanent_location => "PERMANENT_LOCATION" + } + ); + AddIssue( $patron, $item2->barcode ); + + is( Koha::Checkouts->search({ borrowernumber => $patron->{borrowernumber} })->count, 2, "Found 2 checkouts for this patron" ); + + $item1->itemlost(1)->store(); + $item2->itemlost(2)->store(); + + is( Koha::Checkouts->search({ borrowernumber => $patron->{borrowernumber}, 'itemlost' => { '>', 0 } }, { join => 'item'} )->count, 2, "Found 2 lost checkouts for this patron" ); + + my $server->{account}->{lost_block_checkout} = undef; + my $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{ }, "lost_block_checkout = 0 does not block checkouts with 2 lost checkouts" );; + + $server->{account}->{lost_block_checkout} = 0; + $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{ }, "lost_block_checkout = 0 does not block checkouts with 2 lost checkouts" );; + + $server->{account}->{lost_block_checkout} = 1; + $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{Y}, "lost_block_checkout = 1 does block checkouts with 2 lost checkouts" );; + + $server->{account}->{lost_block_checkout} = 2; + $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{Y}, "lost_block_checkout = 2 does block checkouts with 2 lost checkouts" );; + + $server->{account}->{lost_block_checkout} = 3; + $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{ }, "lost_block_checkout = 3 does not block checkouts with 2 lost checkouts" );; + + $server->{account}->{lost_block_checkout} = 2; + $server->{account}->{lost_block_checkout_value} = 2; + $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{ }, "lost_block_checkout = 2, lost_block_checkout_value = 2 does not block checkouts with 2 lost checkouts where only 1 has itemlost = 2" ); + + $server->{account}->{lost_block_checkout} = 1; + $server->{account}->{lost_block_checkout_value} = 2; + $patron_status_string = C4::SIP::Sip::MsgType::patron_status_string( $sip_patron, $server ); + is( substr($patron_status_string, 9, 1), q{Y}, "lost_block_checkout = 2, lost_block_checkout_value = 2 does block checkouts with 2 lost checkouts where only 1 has itemlost = 2" ); + + $schema->storage->txn_rollback; +}; + subtest "Test build_additional_item_fields_string" => sub { my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; -- 2.39.5