From b47e1f3fb5a876ad711cc5197818190dbef6eb69 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 Signed-off-by: Martin Renvoize --- 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 98422460d64..ab5d06c41b9 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 03d72afbb79..94acbe3e1c4 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -34,9 +34,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.45.2