From c7e1f4b564fc5cdf30752f1ac83928fa1ec02d87 Mon Sep 17 00:00:00 2001 From: Emily Lamancusa Date: Tue, 12 Aug 2025 08:34:01 -0400 Subject: [PATCH] Bug 21572: Adjust SIP and SCO checkout logic Handle the new multi-select values for AllowItemsOnHoldCheckoutSIP and AllowItemsOnHoldCheckoutSCO To test: 1. With patch applied, edit AllowItemsOnHoldCheckoutSIP and AllowItemsOnHoldCheckoutSCO to clear all options 2. Set HoldsNeedProcessingSIP to "Don't fulfill" 3. Place holds on 4 items: a. Hold A - place hold and leave as-is (pending status) b. Hold B - place hold for pickup at logged-in branch. Check in the item and confirm the hold (waiting status) c. Hold C - place hold for pickup at another branch. Check in the item and confirm the transfer (transferred status) d. Hold D - place hold for pickupt at the logged-in branch. 4. Use the SIP CLI emulator to check in the item from Hold D (this will set it to processing status) : misc/sip_cli_emulator.pl -a localhost -p 6001 -l -su term1 -sp term1 -m checkin --item 5. Use the SIP CLI emulator to attempt to check out each of the four items: misc/sip_cli_emulator.pl -a localhost -p 6001 -l CPL -su term1 -sp term1 -m checkout --patron koha --item --> Confirm that all checkouts are refused 6. Navigate to the SCO module and log in: http://localhost:8080/cgi-bin/koha/sco/sco-main.pl 7. Attempt to check out each of the four items --> Confirm that all checkouts are refused 8. Edit AllowItemsOnHoldCheckoutSIP and AllowItemsOnHoldCheckoutSCO and check all options 9. Attempt to check out each of the four items --> Confirm that all checkouts are now allowed 10. Set up four new holds as in step 3-4 with new items 11. Attempt to check out each of the four new items using the SIP CLI emulator as in step 5 --> Confirm that all checkouts are now allowed 12. prove t/db_dependent/SIP/Transaction.t --- C4/SIP/ILS/Transaction/Checkout.pm | 9 ++--- opac/sco/sco-main.pl | 10 +++--- t/db_dependent/SIP/Transaction.t | 55 ++++++++++++++++++++++++------ 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index 4909400493..c01f883586 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -50,7 +50,10 @@ sub do_checkout { my $overridden_duedate; # usually passed as undef to AddIssue my $prevcheckout_block_checkout = $account->{prevcheckout_block_checkout}; my $allow_additional_materials_checkout = $account->{allow_additional_materials_checkout}; - my ( $issuingimpossible, $needsconfirmation, $messages ) = _can_we_issue( $patron, $barcode, 0 ); + my @syspref_vals = split /,/, C4::Context->preference("AllowItemsOnHoldCheckoutSIP"); + my %ignore_reserves = map { $_ => 1 } @syspref_vals; + + my ( $issuingimpossible, $needsconfirmation, $messages ) = _can_we_issue( $patron, $barcode, \%ignore_reserves ); if ($no_block_due_date) { my $year = substr( $no_block_due_date, 0, 4 ); @@ -77,11 +80,9 @@ sub do_checkout { foreach my $confirmation ( keys %{$needsconfirmation} ) { if ( $confirmation eq 'RENEW_ISSUE' ) { $self->screen_msg("Item already checked out to you: renewing item."); - } elsif ( $confirmation eq 'RESERVED' and !C4::Context->preference("AllowItemsOnHoldCheckoutSIP") ) { + } elsif ( $confirmation eq 'RESERVED' ) { $self->screen_msg("Item is reserved for another patron upon return."); $noerror = 0; - } elsif ( $confirmation eq 'RESERVED' and C4::Context->preference("AllowItemsOnHoldCheckoutSIP") ) { - next; } elsif ( $confirmation eq 'RESERVE_WAITING' or $confirmation eq 'TRANSFERRED' or $confirmation eq 'PROCESSING' ) diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl index 9ecae7c76c..cc935cef16 100755 --- a/opac/sco/sco-main.pl +++ b/opac/sco/sco-main.pl @@ -190,15 +190,17 @@ if ( $patron && $op eq "cud-returnbook" && $allowselfcheckreturns ) { my @failed_checkouts; my @confirm_checkouts; foreach my $barcode (@$barcodes) { - my $item = Koha::Items->find( { barcode => $barcode } ); - my $impossible = {}; - my $needconfirm = {}; + my $item = Koha::Items->find( { barcode => $barcode } ); + my $impossible = {}; + my $needconfirm = {}; + my @syspref_vals = split /,/, C4::Context->preference("AllowItemsOnHoldCheckoutSCO"); + my %ignore_reserves = map { $_ => 1 } @syspref_vals; ( $impossible, $needconfirm ) = CanBookBeIssued( $patron, $barcode, undef, 0, - C4::Context->preference("AllowItemsOnHoldCheckoutSCO") + \%ignore_reserves, ); my $issue_error; if ( $confirm_required = scalar keys %$needconfirm ) { diff --git a/t/db_dependent/SIP/Transaction.t b/t/db_dependent/SIP/Transaction.t index b121b9c7c4..2256557b45 100755 --- a/t/db_dependent/SIP/Transaction.t +++ b/t/db_dependent/SIP/Transaction.t @@ -22,7 +22,8 @@ use C4::SIP::ILS::Transaction::Hold; use C4::SIP::ILS::Transaction::Checkout; use C4::SIP::ILS::Transaction::Checkin; -use C4::Reserves qw( AddReserve ModReserve ModReserveAffect RevertWaitingStatus ); +use C4::Reserves qw( AddReserve ModReserve ModReserveAffect RevertWaitingStatus ); +use C4::Circulation qw( AddReturn ); use Koha::CirculationRules; use Koha::Item::Transfer; use Koha::DateUtils qw( dt_from_string output_pref ); @@ -1055,7 +1056,7 @@ subtest do_checkout_with_noblock => sub { }; subtest do_checkout_with_holds => sub { - plan tests => 7; + plan tests => 10; my $library = $builder->build_object( { class => 'Koha::Libraries' } ); my $patron = $builder->build_object( @@ -1103,29 +1104,63 @@ subtest do_checkout_with_holds => sub { $sip_item, "Item assigned to transaction" ); - # Test attached holds + t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '' ); + + # Test attached holds with no hold statuses allowed to check out ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 0, $reserve ); # Mark waiting (W) my $hold = Koha::Holds->find($reserve); $co_transaction->do_checkout(); - is( $patron->checkouts->count, 0, 'Checkout was not done due to attached hold (W)' ); + is( + $patron->checkouts->count, 0, + 'Checkout was not done due to attached hold (W) and AllowItemsOnHoldCheckoutSIP' + ); $hold->set_transfer; $co_transaction->do_checkout(); - is( $patron->checkouts->count, 0, 'Checkout was not done due to attached hold (T)' ); + is( + $patron->checkouts->count, 0, + 'Checkout was not done due to attached hold (T) and AllowItemsOnHoldCheckoutSIP' + ); $hold->set_processing; $co_transaction->do_checkout(); - is( $patron->checkouts->count, 0, 'Checkout was not done due to attached hold (P)' ); + is( + $patron->checkouts->count, 0, + 'Checkout was not done due to attached hold (P) and AllowItemsOnHoldCheckoutSIP' + ); # Test non-attached holds C4::Reserves::RevertWaitingStatus( { itemnumber => $hold->itemnumber } ); - t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '0' ); + + $co_transaction->do_checkout(); + is( + $patron->checkouts->count, 0, + 'Checkout with non-attached hold refused due to hold and AllowItemsOnHoldCheckoutSIP' + ); + + t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', 'pending,processing,transferred,waiting' ); + $co_transaction->do_checkout(); - is( $patron->checkouts->count, 0, 'Checkout refused due to hold and AllowItemsOnHoldCheckoutSIP' ); + is( + $patron->checkouts->count, 1, + 'Checkout with non-attached hold allowed due to hold and AllowItemsOnHoldCheckoutSIP' + ); - t::lib::Mocks::mock_preference( 'AllowItemsOnHoldCheckoutSIP', '1' ); + # Test attached holds with all hold statuses allowed to check out + AddReturn( $item->barcode, $library->branchcode ); + $hold->set_waiting; + $co_transaction->do_checkout(); + is( $patron->checkouts->count, 1, 'Checkout allowed with attached hold (W) due to AllowItemsOnHoldCheckoutSIP' ); + + AddReturn( $item->barcode, $library->branchcode ); + $hold->set_transfer; + $co_transaction->do_checkout(); + is( $patron->checkouts->count, 1, 'Checkout allowed with attached hold (T) due to AllowItemsOnHoldCheckoutSIP' ); + + AddReturn( $item->barcode, $library->branchcode ); + $hold->set_processing; $co_transaction->do_checkout(); - is( $patron->checkouts->count, 1, 'Checkout allowed due to hold and AllowItemsOnHoldCheckoutSIP' ); + is( $patron->checkouts->count, 1, 'Checkout allowed with attached hold (P) due to AllowItemsOnHoldCheckoutSIP' ); }; subtest checkin_lost => sub { -- 2.39.5