Bugzilla – Attachment 175697 Details for
Bug 7376
Transfer limits should be checked at check-in
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 7376: (follow-up) Add tests on no_return
Bug-7376-follow-up-Add-tests-on-noreturn.patch (text/plain), 16.29 KB, created by
Baptiste Wojtkowski (bwoj)
on 2024-12-19 10:51:24 UTC
(
hide
)
Description:
Bug 7376: (follow-up) Add tests on no_return
Filename:
MIME Type:
Creator:
Baptiste Wojtkowski (bwoj)
Created:
2024-12-19 10:51:24 UTC
Size:
16.29 KB
patch
obsolete
>From 9c02c2be9420992d15dd734744a18e194d39951a Mon Sep 17 00:00:00 2001 >From: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> >Date: Thu, 19 Dec 2024 11:29:17 +0100 >Subject: [PATCH] Bug 7376: (follow-up) Add tests on no_return > >Note: tests have to be performed on addReturn and have therefore been >added in a separate test >--- > C4/Circulation.pm | 7 +- > t/db_dependent/Circulation.t | 207 +++++++++++++++++++++++++++-------- > 2 files changed, 166 insertions(+), 48 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index ed4a9835..9fd6e004 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -1347,6 +1347,8 @@ Check whether the item can be returned to the provided branch > > =item C<$branch> is the branchcode where the return is taking place > >+=item C<$transferbranch> the branch where the book should be transferred after return, if the transfer is impossible, the book will be prevented from returning. >+ > =back > > Returns: >@@ -1365,11 +1367,10 @@ sub CanBookBeReturned { > my ($item, $returnbranch, $transferbranch) = @_; > my $allowreturntobranch = C4::Context->preference("AllowReturnToBranch") || 'anywhere'; > >- # assume return is allowed to start > my $allowed = 1; > my $message; > >- # identify all cases where return is forbidden >+ # Refuse check-in if it does not respect AllowReturnToBranch rules > if ($allowreturntobranch eq 'homebranch' && $returnbranch ne $item->homebranch) { > $allowed = 0; > $message = $item->homebranch; >@@ -1381,7 +1382,7 @@ sub CanBookBeReturned { > $message = $item->homebranch; # FIXME: choice of homebranch is arbitrary > } > >- # identify cases where transfer rules prohibit return >+ # Refuse check-in if book cannot be transferred to $transferbranch due to transfer limits > if ( defined($transferbranch) && $allowed ) { > my $from_library = Koha::Libraries->find($returnbranch); > my $to_library = Koha::Libraries->find($transferbranch); >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index cce67e4f..946a8831 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 => 76; >+use Test::More tests => 77; > use Test::Exception; > use Test::MockModule; > use Test::Deep qw( cmp_deeply ); >@@ -2600,7 +2600,6 @@ subtest 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub { > is( $error->{USERBLOCKEDNOENDDATE}, '9999-12-31', 'USERBLOCKEDNOENDDATE should be 9999-12-31 for unlimited debarments' ); > }; > >- > subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub { > plan tests => 22; > t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); >@@ -2608,7 +2607,8 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub > my $homebranch = $builder->build( { source => 'Branch' } ); > my $holdingbranch = $builder->build( { source => 'Branch' } ); > my $returnbranch = $builder->build( { source => 'Branch' } ); >- my $patron = $builder->build_object( { class => 'Koha::Patrons', value => { categorycode => $patron_category->{categorycode} } } ); >+ my $patron = $builder->build_object( >+ { class => 'Koha::Patrons', value => { categorycode => $patron_category->{categorycode} } } ); > > my $item = $builder->build_sample_item( > { >@@ -2623,48 +2623,52 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub > > # Attempt returns at returnbranch > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); >- my ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); >- is ( 0 , $allowed , 'Prevent return where returnbranch != homebranch'); >- is ( $homebranch->{branchcode} , $message , 'Redirect to homebranch'); >+ my ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode} ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); >- is ( 0 , $allowed , 'Prevent return where returnbranch != holdingbranch'); >- is ( $holdingbranch->{branchcode} , $message , 'Redirect to holdingbranch'); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode} ); >+ is( 0, $allowed, 'Prevent return where returnbranch != holdingbranch' ); >+ is( $holdingbranch->{branchcode}, $message, 'Redirect to holdingbranch' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); >- is ( 0 , $allowed , 'Prevent return where returnbranch != homebranch or holdingbranch'); >- is ( $homebranch->{branchcode} , $message , 'Redirect to homebranch'); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode} ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch or holdingbranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); >- is ( 1 , $allowed , 'with AllowReturnToBranch = anywhere and no limits return to returnbranch is allowed'); >- is ( undef , $message , 'without limits provides no message'); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode} ); >+ is( 1, $allowed, 'with AllowReturnToBranch = anywhere and no limits return to returnbranch is allowed' ); >+ is( undef, $message, 'without limits provides no message' ); > > # Set limit (Holding -> Return denied) > diag("Transfer limit: Holding -> Return"); > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); >- my $limit = Koha::Item::Transfer::Limit->new({ >- toBranch => $returnbranch->{branchcode}, >- fromBranch => $holdingbranch->{branchcode}, >- itemtype => $item->effective_itemtype, >- })->store(); >+ my $limit = Koha::Item::Transfer::Limit->new( >+ { >+ toBranch => $returnbranch->{branchcode}, >+ fromBranch => $holdingbranch->{branchcode}, >+ itemtype => $item->effective_itemtype, >+ } >+ )->store(); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); >- is ( 1 , $allowed , 'Allow return where transferbranch is not passed'); >- is ( undef , $message , 'without limits provides no message'); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode} ); >+ is( 1, $allowed, 'Allow return where transferbranch is not passed' ); >+ is( undef, $message, 'without limits provides no message' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}, $homebranch->{branchcode}); >- is ( 1 , $allowed , 'Allow return where there is no transfer limit between returnbranch and homebranch'); >- is ( undef , $message , 'without limits provides no message'); >+ ( $allowed, $message ) = >+ C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode}, $homebranch->{branchcode} ); >+ is( 1, $allowed, 'Allow return where there is no transfer limit between returnbranch and homebranch' ); >+ is( undef, $message, 'without limits provides no message' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}, $holdingbranch->{branchcode}); >- is ( 1 , $allowed , 'Allow return where there is no transfer limit between returnbranch and holdingbranch'); >- is ( undef , $message , 'without limits provides no message'); >+ ( $allowed, $message ) = >+ C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode}, $holdingbranch->{branchcode} ); >+ is( 1, $allowed, 'Allow return where there is no transfer limit between returnbranch and holdingbranch' ); >+ is( undef, $message, 'without limits provides no message' ); > > # Set limit ( Return -> Home denied) > diag("Transfer limit: Return -> Home"); >@@ -2676,15 +2680,17 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub > )->store()->discard_changes; > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}, $homebranch->{branchcode}); >- is ( 0 , $allowed , 'Prevent return where returnbranch cannot transfer to homebranch'); >- is ( $homebranch->{branchcode} , $message , 'Redirect to homebranch'); >+ ( $allowed, $message ) = >+ C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode}, $homebranch->{branchcode} ); >+ is( 0, $allowed, 'Prevent return where returnbranch cannot transfer to homebranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}, $holdingbranch->{branchcode}); >- is ( 1 , $allowed , 'Allow return where there is no transfer limit between returnbranch and holdingbranch'); >- is ( undef , $message , 'without limits provides no message'); >- >+ ( $allowed, $message ) = >+ C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode}, $holdingbranch->{branchcode} ); >+ is( 1, $allowed, 'Allow return where there is no transfer limit between returnbranch and holdingbranch' ); >+ is( undef, $message, 'without limits provides no message' ); >+ > # Set limit ( Return -> Holding denied) > diag("Transfer limit: Return -> Holding"); > >@@ -2697,16 +2703,130 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub > )->store()->discard_changes; > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}, $homebranch->{branchcode}); >- is ( 1 , $allowed , 'Allow return where there is no transfer limit between returnbranch and homebranch'); >- is ( undef , $message , 'without limits provides no message'); >+ ( $allowed, $message ) = >+ C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode}, $homebranch->{branchcode} ); >+ is( 1, $allowed, 'Allow return where there is no transfer limit between returnbranch and homebranch' ); >+ is( undef, $message, 'without limits provides no message' ); > > t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}, $holdingbranch->{branchcode}); >- is ( 0 , $allowed , 'Prevent return where returnbranch cannot transfer to holdingbranch'); >- is ( $holdingbranch->{branchcode} , $message , 'Redirect to holdingbranch'); >+ ( $allowed, $message ) = >+ C4::Circulation::CanBookBeReturned( $item, $returnbranch->{branchcode}, $holdingbranch->{branchcode} ); >+ is( 0, $allowed, 'Prevent return where returnbranch cannot transfer to holdingbranch' ); >+ is( $holdingbranch->{branchcode}, $message, 'Redirect to holdingbranch' ); > }; > >+subtest 'AddReturn + TransferLimits' => sub { >+ plan tests => 3; >+ >+ ######################################################################## >+ # >+ # Prepare test >+ # >+ ######################################################################## >+ >+ my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); >+ my $homebranch = $builder->build( { source => 'Branch' } ); >+ my $holdingbranch = $builder->build( { source => 'Branch' } ); >+ my $returnbranch = $builder->build( { source => 'Branch' } ); >+ my $item = $builder->build_sample_item( >+ { >+ homebranch => $homebranch->{branchcode}, >+ holdingbranch => $holdingbranch->{branchcode}, >+ } >+ ); >+ >+ # Default circulation rules >+ Koha::CirculationRules->set_rules( >+ { >+ categorycode => undef, >+ branchcode => undef, >+ itemtype => $item->itype, >+ rules => { >+ maxissueqty => 1, >+ reservesallowed => 25, >+ issuelength => 7, >+ lengthunit => 'days', >+ renewalsallowed => 5, >+ renewalperiod => 7, >+ norenewalbefore => undef, >+ auto_renew => 0, >+ fine => .10, >+ chargeperiod => 1, >+ } >+ } >+ ); >+ t::lib::Mocks::mock_userenv( { branchcode => $holdingbranch->{branchcode} } ); >+ >+ # Each transfer from returnbranch is forbidden >+ my $limit = Koha::Item::Transfer::Limit->new( >+ { >+ fromBranch => $returnbranch->{branchcode}, >+ toBranch => $holdingbranch->{branchcode}, >+ itemtype => $item->effective_itemtype, >+ } >+ )->store(); >+ my $limit2 = Koha::Item::Transfer::Limit->new( >+ { >+ fromBranch => $returnbranch->{branchcode}, >+ toBranch => $homebranch->{branchcode}, >+ itemtype => $item->effective_itemtype, >+ } >+ )->store(); >+ >+ ######################################################################## >+ # >+ # Begin test >+ # >+ # Each transfer is forbidden by transfer limits >+ # Checkin must be forbidden except if there is no transfer to perform >+ # >+ ######################################################################## >+ >+ # Case 1: There is a transfer to to do homebranch >+ Koha::CirculationRules->set_rules( >+ { >+ branchcode => undef, >+ itemtype => undef, >+ rules => { >+ returnbranch => 'homebranch', >+ }, >+ } >+ ); >+ my $issue = AddIssue( $patron, $item->barcode ); >+ my ( $doreturn, $messages, $iteminfo, $borrowerinfo ) = AddReturn( $item->barcode, $returnbranch->{branchcode} ); >+ is( $doreturn, 0, "Item cannot be returned if there is a transfer to do" ); >+ >+ # Case 2: There is a transfer to do to holdingbranch >+ Koha::CirculationRules->set_rules( >+ { >+ branchcode => undef, >+ itemtype => undef, >+ rules => { >+ returnbranch => 'holdingbranch', >+ }, >+ } >+ ); >+ $issue->delete; >+ $item->holdingbranch( $holdingbranch->{branchcode} )->store(); >+ $issue = AddIssue( $patron, $item->barcode ); >+ ( $doreturn, $messages, $iteminfo, $borrowerinfo ) = AddReturn( $item->barcode, $returnbranch->{branchcode} ); >+ is( $doreturn, 0, "Item cannot be returned if there is a transfer to do (item is floating)" ); >+ >+ # Case 3: There is no transfer to do >+ Koha::CirculationRules->set_rules( >+ { >+ branchcode => undef, >+ itemtype => undef, >+ rules => { >+ returnbranch => 'noreturn', >+ }, >+ } >+ ); >+ $issue->delete; >+ $issue = AddIssue( $patron, $item->barcode ); >+ ( $doreturn, $messages, $iteminfo, $borrowerinfo ) = AddReturn( $item->barcode, $returnbranch->{branchcode} ); >+ is( $doreturn, 1, "Item cannot be returned if there is a transfer to do" ); >+}; > > subtest 'Statistic patrons "X"' => sub { > plan tests => 15; >@@ -2795,11 +2915,8 @@ subtest 'Statistic patrons "X"' => sub { > Koha::Statistics->search( { itemnumber => $item_4->itemnumber } )->count, 3, > 'Issue, return, and localuse should be recorded in statistics table for item 4.' > ); >- >- # TODO There are other tests to provide here > }; > >- > subtest "Bug 27753 - Add AutoClaimReturnStatusOnCheckin" => sub { > plan tests => 8; > >-- >2.30.2
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 7376
:
6954
|
21715
|
21805
|
22016
|
22017
|
22323
|
22357
|
22696
|
23168
|
76559
|
76866
|
76876
|
92740
|
92774
|
93190
|
93191
|
93192
|
93193
|
93194
|
93195
|
93196
|
107077
|
107078
|
107079
|
115369
|
115370
|
116493
|
116494
|
116927
|
116928
|
116937
|
116938
|
120897
|
120898
|
120899
|
120900
|
120901
|
120902
|
120903
|
144435
|
144436
|
144437
|
144438
|
144439
|
144440
|
144441
|
175690
|
175691
|
175692
|
175693
|
175694
|
175695
|
175696
|
175697
|
180069
|
180070
|
180071
|
180072
|
180073
|
180074
|
180075
|
180076
|
180590
|
180591
|
180592
|
180593
|
180594
|
180595
|
180596
|
180597
|
181055
|
181056
|
181057
|
181058
|
181059
|
181060
|
181061
|
181062
|
181064
|
181065
|
181066
|
181067
|
181068
|
181069
|
181070
|
181071