Bugzilla – Attachment 153492 Details for
Bug 34281
Transfer limits have no impact on CanBookBeRenewed
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 34281: Add logic to CanBookBeRenewed function to check if item can be transferred
Bug-34281-Add-logic-to-CanBookBeRenewed-function-t.patch (text/plain), 7.85 KB, created by
Peter Vashchuk
on 2023-07-14 15:06:17 UTC
(
hide
)
Description:
Bug 34281: Add logic to CanBookBeRenewed function to check if item can be transferred
Filename:
MIME Type:
Creator:
Peter Vashchuk
Created:
2023-07-14 15:06:17 UTC
Size:
7.85 KB
patch
obsolete
>From a74d4ab26215f0c5b6837774b63c0406f1ec20e5 Mon Sep 17 00:00:00 2001 >From: Petro Vashchuk <stalkernoid@gmail.com> >Date: Fri, 14 Jul 2023 18:04:34 +0300 >Subject: [PATCH] Bug 34281: Add logic to CanBookBeRenewed function to check if > item can be transferred > >To reproduce: >1. Find or create a biblio that has two items of the same itemtype, one on branch A and another one on branch B >2. Checkout the item on branch A >3. Make a hold on that same branch >4. Checkout the item on branch B >5. Go to the sysprefs and enable UseBranchTransferLimits, after which go to the checkin and transfer policy page and disallow the transfer >6. Try to renew the item on branch B, the koha won't let you renew it >7. Apply the patch >8. Repeat step 6 but this time you should be able to renew an item > >Also run the tests prior to applying the patch and then after. >--- > C4/Circulation.pm | 13 ++++- > t/db_dependent/Circulation.t | 96 +++++++++++++++++++++++++++++++++++- > 2 files changed, 107 insertions(+), 2 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 0f1b6914aa..81f16826db 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -3047,7 +3047,9 @@ sub CanBookBeRenewed { > notforloan => 0, > -not => { itemnumber => $item->itemnumber } })->as_list; > >- return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds); >+ unless ( C4::Context->preference("UseBranchTransferLimits") ) { >+ return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds); >+ } > > my %matched_items; > foreach my $possible_hold (@possible_holds) { >@@ -3059,6 +3061,15 @@ sub CanBookBeRenewed { > }); > > # FIXME: We are not checking whether the item we are renewing can fill the hold >+ if ( C4::Context->preference("UseBranchTransferLimits") ) { >+ if ( C4::Context->preference("item-level_itypes") && C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ) { >+ if ( ! IsBranchTransferAllowed( $possible_hold->branchcode, $item->homebranch, $item->itype ) ) { >+ next; >+ } >+ } elsif ( ! IsBranchTransferAllowed( $possible_hold->branchcode, $item->homebranch, $item->ccode ) ) { >+ next; >+ } >+ } > > foreach my $other_item (@other_items) { > next if defined $matched_items{$other_item->itemnumber}; >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 1b625f90e4..032a85acc0 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > use utf8; > >-use Test::More tests => 67; >+use Test::More tests => 68; > use Test::Exception; > use Test::MockModule; > use Test::Deep qw( cmp_deeply ); >@@ -290,6 +290,9 @@ Koha::CirculationRules->set_rules( > subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers and items tests" => sub { > plan tests => 7; > >+ my $schema = Koha::Database->schema; >+ $schema->storage->txn_begin; >+ > #Can only reserve from home branch > Koha::CirculationRules->set_rule( > { >@@ -377,6 +380,97 @@ subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers a > ( $renewokay, $error ) = CanBookBeRenewed($patron, $issue); > is( $renewokay, 0, 'Cannot renew when there is an item specific hold'); > is( $error, 'on_reserve', 'Cannot renew, only this item can fill the reserve'); >+ $schema->storage->txn_rollback; >+}; >+ >+subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple branches (transfers are limited)" => sub { >+ plan tests => 10; >+ >+ my $schema = Koha::Database->schema; >+ $schema->storage->txn_begin; >+ >+ # Patrons from three different branches >+ my $patron_borrower_1 = $builder->build_object({ class => 'Koha::Patrons' }); >+ my $patron_borrower_2 = $builder->build_object({ class => 'Koha::Patrons' }); >+ my $patron_hold = $builder->build_object({ class => 'Koha::Patrons' }); >+ my $biblio = $builder->build_sample_biblio(); >+ >+ my $branch1 = $patron_borrower_1->branchcode; >+ my $branch2 = $patron_borrower_2->branchcode; >+ >+ # Item at each patron branch >+ my $item_1 = $builder->build_sample_item({ >+ biblionumber => $biblio->biblionumber, >+ homebranch => $branch1 >+ }); >+ my $item_2 = $builder->build_sample_item({ >+ biblionumber => $biblio->biblionumber, >+ homebranch => $branch2, >+ itype => $item_1->effective_itemtype >+ }); >+ >+ # checkout item_1 >+ my $issue1 = AddIssue( $patron_borrower_1->unblessed, $item_1->barcode); >+ my $datedue1 = dt_from_string( $issue1->date_due() ); >+ is (defined $issue1->date_due(), 1, "Item 1 checked out, due date: " . $issue1->date_due() ); >+ >+ # checkout item_2 >+ my $issue2 = AddIssue( $patron_borrower_2->unblessed, $item_2->barcode); >+ my $datedue2 = dt_from_string( $issue2->date_due() ); >+ is (defined $issue2->date_due(), 1, "Item 2 checked out, due date: " . $issue2->date_due() ); >+ >+ AddReserve( >+ { >+ branchcode => $branch1, >+ borrowernumber => $patron_hold->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ priority => 1, >+ reservation_date => dt_from_string(), >+ expiration_date => undef, >+ itemnumber => $item_1->itemnumber, >+ found => undef, >+ } >+ ); >+ >+ my $AllowRenewalIfOtherItemsAvailable = C4::Context->preference('AllowRenewalIfOtherItemsAvailable'); >+ my $UseBranchTransferLimits = C4::Context->preference('UseBranchTransferLimits'); >+ my $BranchTransferLimitsType = C4::Context->preference('BranchTransferLimitsType'); >+ >+ warn $UseBranchTransferLimits; >+ >+ t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', 1 ); >+ >+ my ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower_1->borrowernumber, $item_1->itemnumber); >+ is( $renewokay, 0, 'Cannot renew item1, no other items available for hold'); >+ is( $error, 'on_reserve', 'Cannot renew, reserved (returned error is on_reserve)'); >+ >+ t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); >+ t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); >+ >+ ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower_2->borrowernumber, $item_2->itemnumber); >+ is( $renewokay, 0, 'Cannot renew item2, no other items available for hold, transfers are not permitted'); >+ is( $error, 'on_reserve', 'Cannot renew, reserved (returned error is on_reserve)'); >+ >+ my $limit = Koha::Item::Transfer::Limit->new( >+ { >+ toBranch => $branch1, >+ fromBranch => $branch2, >+ itemtype => $item_1->effective_itemtype, >+ })->store(); >+ >+ ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower_1->borrowernumber, $item_1->itemnumber); >+ is( $renewokay, 0, 'Cannot renew item1, no other items available for hold'); >+ is( $error, 'on_reserve', 'Cannot renew, reserved (returned error is on_reserve)'); >+ >+ ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower_2->borrowernumber, $item_2->itemnumber); >+ is( $renewokay, 1, 'Can renew item2, it\'s on on different branch than hold, transfers are permitted between those two branches'); >+ is( $error, undef, 'Can renew, no holds on the same branch'); >+ >+ t::lib::Mocks::mock_preference( 'AllowRenewalIfOtherItemsAvailable', $AllowRenewalIfOtherItemsAvailable ); >+ t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', $UseBranchTransferLimits ); >+ t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', $BranchTransferLimitsType ); >+ >+ $schema->storage->txn_rollback; > }; > > subtest "GetIssuingCharges tests" => sub { >-- >2.41.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 34281
: 153492