From 58ea7286789e1fa72b61d24f55e9b252eb0917ca Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Fri, 23 Feb 2024 13:06:40 -0500 Subject: [PATCH] Bug 18317: (QA follow-up) Clean up code and add unit tests Signed-off-by: Kyle M Hall --- C4/SIP/ILS.pm | 16 +++---- installer/data/mysql/mandatory/sysprefs.sql | 2 +- t/db_dependent/SIP/Transaction.t | 47 ++++++++++++++++++++- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm index 480ccee285a..5baef0e719f 100644 --- a/C4/SIP/ILS.pm +++ b/C4/SIP/ILS.pm @@ -6,21 +6,19 @@ package C4::SIP::ILS; use warnings; use strict; -use C4::SIP::Sip qw( siplog ); -use Koha::DateUtils qw( dt_from_string output_pref ); -use Data::Dumper; +use C4::Context; use C4::SIP::ILS::Item; use C4::SIP::ILS::Patron; -use C4::SIP::ILS::Transaction; -use C4::SIP::ILS::Transaction::Checkout; use C4::SIP::ILS::Transaction::Checkin; +use C4::SIP::ILS::Transaction::Checkout; use C4::SIP::ILS::Transaction::FeePayment; use C4::SIP::ILS::Transaction::Hold; use C4::SIP::ILS::Transaction::Renew; use C4::SIP::ILS::Transaction::RenewAll; - -use C4::Context; #BZ 18317 +use C4::SIP::ILS::Transaction; +use C4::SIP::Sip qw( siplog ); +use Koha::DateUtils qw( dt_from_string output_pref ); my %supports = ( 'magnetic media' => 1, @@ -164,8 +162,8 @@ sub checkout { } elsif ( $item->{borrowernumber} - && !C4::Context->preference('AllowItemsOnLoanCheckoutSIP') #BZ 18317 - && !_ci_cardnumber_cmp( $item->{borrowernumber}, $patron->borrowernumber ) + && !C4::Context->preference('AllowItemsOnLoanCheckoutSIP') + && !_ci_cardnumber_cmp( $item->{borrowernumber}, $patron->borrowernumber ) ) { $circ->screen_msg("Item checked out to another patron"); diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql index 46ebaf747a8..d6264f0d624 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -33,9 +33,9 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('AllowHoldPolicyOverride','0',NULL,'Allow staff to override hold policies when placing holds','YesNo'), ('AllowHoldsOnDamagedItems','1','','Allow hold requests to be placed on damaged items','YesNo'), ('AllowHoldsOnPatronsPossessions','1',NULL,'Allow holds on records that patron have items of it','YesNo'), -('AllowItemsOnLoanCheckoutSIP','0','','Do not generate ISSUED_TO_ANOTHER warning when checking out items already checked out to someone else via SIP. This allows self checkouts for those items.','YesNo'), ('AllowItemsOnHoldCheckoutSCO','0','','Do not generate RESERVE_WAITING and RESERVED warning in the SCO module when checking out items reserved to someone else. This allows self checkouts for those items.','YesNo'), ('AllowItemsOnHoldCheckoutSIP','0','','Do not generate RESERVED warning when checking out items reserved to someone else via SIP. This allows self checkouts for those items.','YesNo'), +('AllowItemsOnLoanCheckoutSIP','0','','Do not generate ISSUED_TO_ANOTHER warning when checking out items already checked out to someone else via SIP. This allows self checkouts for those items.','YesNo'), ('AllowMultipleCovers','0','1','Allow multiple cover images to be attached to each bibliographic record.','YesNo'), ('AllowMultipleIssuesOnABiblio',1,'Allow/Don\'t allow patrons to check out multiple items from one biblio','','YesNo'), ('AllowNotForLoanOverride','0','','If ON, Koha will allow the librarian to loan a not for loan item.','YesNo'), diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t index 56a810a7360..b23b0025450 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -4,7 +4,7 @@ # Current state is very rudimentary. Please help to extend it! use Modern::Perl; -use Test::More tests => 18; +use Test::More tests => 19; use Test::Warn; use Koha::Database; @@ -1314,4 +1314,49 @@ subtest do_checkout_with_recalls => sub { is( $recall2->status, 'fulfilled', 'Recall is fulfilled by checked out item' ); }; +subtest 'Checkin message' => sub { + plan tests => 2; + + my $mockILS = Test::MockObject->new; + my $server = { ils => $mockILS }; + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); + + my $institution = { + id => $library->id, + implementation => "ILS", + policy => { + checkin => "true", + renewal => "true", + checkout => "true", + timeout => 100, + retries => 5, + } + }; + my $ils = C4::SIP::ILS->new($institution); + my $item = $builder->build_sample_item( + { + library => $library->branchcode, + location => 'My Location', + permanent_location => 'My Permanent Location', + } + ); + + t::lib::Mocks::mock_preference( 'AllowItemsOnLoanCheckoutSIP', '' ); + + my $circ = $ils->checkout( $patron1->cardnumber, $item->barcode, undef, undef, $server->{account} ); + + $circ = $ils->checkout( $patron2->cardnumber, $item->barcode, undef, undef, $server->{account} ); + is( + $circ->{screen_msg}, 'Item checked out to another patron', + "Checked out item was not checked out to the next patron" + ); + + t::lib::Mocks::mock_preference( 'AllowItemsOnLoanCheckoutSIP', '1' ); + + $circ = $ils->checkout( $patron2->cardnumber, $item->barcode, undef, undef, $server->{account} ); + is( $circ->{screen_msg}, '', "Checked out item was checked out to the next patron" ); +}; + $schema->storage->txn_rollback; -- 2.30.2