Bugzilla – Attachment 181066 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) Extend test coverage and document
Bug-7376-follow-up-Extend-test-coverage-and-docume.patch (text/plain), 11.96 KB, created by
Mathieu Saby
on 2025-04-17 12:14:45 UTC
(
hide
)
Description:
Bug 7376: (follow-up) Extend test coverage and document
Filename:
MIME Type:
Creator:
Mathieu Saby
Created:
2025-04-17 12:14:45 UTC
Size:
11.96 KB
patch
obsolete
>From 6a2704d72de97ffcb34cd67c4057cefcd47d3e50 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 16 Dec 2024 12:15:20 +0100 >Subject: [PATCH] Bug 7376: (follow-up) Extend test coverage and document > >I'm not sure about the requirements for this bug. This patch extends the >tests to cover the new combinations introduced and highlights exactly >what changes in the logic.. which appears to be fairly little. > >Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> >Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> >Signed-off-by: Mathieu Saby <mathsabypro@gmail.com> >--- > t/db_dependent/Circulation.t | 169 ++++++++++++++++++++++++++++++----- > 1 file changed, 148 insertions(+), 21 deletions(-) > >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 983b445b45..403c9e5083 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -2843,46 +2843,173 @@ subtest 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub { > }; > > subtest 'CanBookBeReturned + UseBranchTransfertLimits' => sub { >- plan tests => 6; >+ plan tests => 31; >+ t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); > >- my $homebranch = $builder->build( { source => 'Branch' } ); >- my $otherbranch = $builder->build( { source => 'Branch' } ); >- my $patron = >- $builder->build( { source => 'Borrower', value => { categorycode => $patron_category->{categorycode} } } ); >+ 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 $item = $builder->build_sample_item( > { > homebranch => $homebranch->{branchcode}, >- holdingbranch => $homebranch->{branchcode}, >+ holdingbranch => $holdingbranch->{branchcode}, > } > ); > >- t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >- set_userenv($homebranch); >+ # Issue from holdingbranch >+ set_userenv($holdingbranch); > my $issue = AddIssue( $patron, $item->barcode ); >- my ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $otherbranch ); >- is( 1, $allowed, 'with AllowReturnToBranch = anywhere and no limits return to other is allowed' ); >+ >+ # 'Anywhere' + BranchTransferLimits >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >+ >+ # Attempt return at returnbranch (No limits defined) >+ my ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 1, $allowed, 'with AllowReturnToBranch = anywhere and no limits return to returnbranch is allowed' ); > is( undef, $message, 'without limits provides no message' ); > >- t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); >+ # Set limit (Holding -> Return denied) > t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); >- >- # set limit > my $limit = Koha::Item::Transfer::Limit->new( > { >- toBranch => $otherbranch->{branchcode}, >- fromBranch => $item->homebranch, >+ toBranch => $returnbranch->{branchcode}, >+ fromBranch => $holdingbranch->{branchcode}, > itemtype => $item->effective_itemtype, > } > )->store(); > >- ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $otherbranch ); >- is( 0, $allowed, 'With transfer limits cannot return to otherbranch' ); >- is( $homebranch->{branchcode}, $message, 'With transfer limits asks return to homebranch' ); >+ diag("Attempt return at returnbranch ('Homebranch' + Holding -> Return limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Holdingbranch' + Holding -> Return limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != holdingbranch' ); >+ is( $holdingbranch->{branchcode}, $message, 'Redirect to holdingbranch' ); >+ >+ diag("Attempt return at returnbranch ('Home Or Holding' + Holding -> Return limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch or holdingbranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Anywhere' + Holding -> Return limit)"); >+ >+ # NOTE: This prevents receiving an item from a branch that is listed in >+ # the branchtransferlimits as not allowed to send to our returnbranch. >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch has a branchtransfer limit from holdingbranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ # Set limit ( Return -> Home denied) >+ $limit->set( >+ { >+ toBranch => $homebranch->{branchcode}, >+ fromBranch => $returnbranch->{branchcode} >+ } >+ )->store()->discard_changes; >+ >+ diag("Attempt return at returnbranch ('Homebranch' + Return -> Home limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Holdingbranch' + Return -> Home limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != holdingbranch' ); >+ is( $holdingbranch->{branchcode}, $message, 'Redirect to holdingbranch' ); > >- ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $homebranch ); >- is( 1, $allowed, 'With transfer limits can return to homebranch' ); >- is( undef, $message, 'With transfer limits and homebranch provides no message' ); >+ diag("Attempt return at returnbranch ('Home Or Holding' + Return -> Home limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch or holdingbranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Anywhere' + Return -> Home limit)"); >+ >+ # NOTE: Should we prevent return here.. we cannot transfer from 'returnbranch' >+ # to 'homebranch' (But we can transfer back to 'holdingbranch'). >+ # NOTE: This is the ONLY change that bug 7376 introduces currently. >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 1, $allowed, 'Allow return where returnbranch can be transfered to from anywhere' ); >+ >+ # Set limit ( Return -> Holding denied) >+ $limit->set( >+ { >+ toBranch => $holdingbranch->{branchcode}, >+ fromBranch => $returnbranch->{branchcode} >+ } >+ )->store()->discard_changes; >+ >+ diag("Attempt return at returnbranch ('Homebranch' + Return -> Holding limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Holdingbranch' + Return -> Holding limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != holdingbranch' ); >+ is( $holdingbranch->{branchcode}, $message, 'Redirect to holdingbranch' ); >+ >+ diag("Attempt return at returnbranch ('Home Or Holding' + Return -> Holding limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch or holdingbranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Anywhere' + Return -> Holding limit)"); >+ >+ # NOTE: Should we prevent return here.. we cannot transfer from 'returnbranch' >+ # to 'holdingbranch' (But we can transfer back to 'homebranch'). >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 1, $allowed, 'Allow return where returnbranch can be transfered to from anywhere' ); >+ >+ # Set limit ( Holding -> Home denied) >+ $limit->set( >+ { >+ toBranch => $holdingbranch->{branchcode}, >+ fromBranch => $returnbranch->{branchcode} >+ } >+ )->store()->discard_changes; >+ >+ diag("Attempt return at returnbranch ('Homebranch' + Holding -> Home limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Holdingbranch' + Holding -> Home limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != holdingbranch' ); >+ is( $holdingbranch->{branchcode}, $message, 'Redirect to holdingbranch' ); >+ >+ diag("Attempt return at returnbranch ('Home Or Holding' + Holding -> Home limit)"); >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homeorholdingbranch' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 0, $allowed, 'Prevent return where returnbranch != homebranch or holdingbranch' ); >+ is( $homebranch->{branchcode}, $message, 'Redirect to homebranch' ); >+ >+ diag("Attempt return at returnbranch ('Anywhere' + Holding -> Home limit)"); >+ >+ # NOTE: A return here can subsequently be transfered to back to homebranch >+ # or holdingbranch. >+ t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); >+ ( $allowed, $message ) = C4::Circulation::CanBookBeReturned( $item, $returnbranch ); >+ is( 1, $allowed, 'Allow return where returnbranch can be transfered to from anywhere' ); > }; > > subtest 'Statistic patrons "X"' => sub { >-- >2.39.5
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