From 5519811ae5e9be3476e36630ff33393fe6d0485a Mon Sep 17 00:00:00 2001 From: Baptiste Wojtkowski 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 Signed-off-by: Baptiste Wojtkowski Signed-off-by: Mathieu Saby --- C4/Circulation.pm | 7 ++- t/db_dependent/Circulation.t | 117 ++++++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index e285dfefda..fbbe1aba34 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1368,6 +1368,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: @@ -1386,11 +1388,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; @@ -1405,7 +1406,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 17c9985b5f..e6ede64c78 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -19,7 +19,7 @@ use Modern::Perl; use utf8; use Test::NoWarnings; -use Test::More tests => 77; +use Test::More tests => 78; use Test::Exception; use Test::MockModule; use Test::Deep qw( cmp_deeply ); @@ -2957,6 +2957,119 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub 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; @@ -3044,8 +3157,6 @@ 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 { -- 2.39.5