From 71b2e0958bcc045875fcbbd510242f8ba98e5f78 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 17 Feb 2021 14:40:47 +0000 Subject: [PATCH] Bug 7376: Clarify tests further We want to prevent returns where the item cannot be transfered 'back to where it came from (be that Homebranch or Holdingbranch as defined respectively in the circulation rules). This patch clarifies the tests further by testing this transfer functionality distinctly from the 'AllowReturnToBranch' preference functionality. --- t/db_dependent/Circulation.t | 166 ++++++++++------------------------- 1 file changed, 45 insertions(+), 121 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 9d0f723a67..ed4c084e56 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -1836,8 +1836,8 @@ subtest 'AddIssue & AllowReturnToBranch' => sub { }; -subtest 'CanBookBeReturned + UseBranchTransfertLimits' => sub { - plan tests => 33; +subtest 'CanBookBeReturned + UseBranchTransfertLimits + CirculationRules' => sub { + plan tests => 22; t::lib::Mocks::mock_preference( 'UseBranchTransferLimits', '1' ); @@ -1857,133 +1857,72 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits' => sub { set_userenv($holdingbranch); my $issue = AddIssue( $patron, $item->barcode ); - # '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'); - - # Set limit (Holding -> Return denied) - 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(); - - diag("Attempt return at returnbranch ('Homebranch' + Holding -> Return limit)"); + # Attempt returns at returnbranch t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); - ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch); + my ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); 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); + ($allowed, $message) = C4::Circulation::CanBookBeReturned($item, $returnbranch->{branchcode}); 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); + ($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'); - 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'); - - 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 holding (homebranch forbidden) from returnbranch'); + ($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'); - my $limit_2 = Koha::Item::Transfer::Limit->new({ - toBranch => $holdingbranch->{branchcode}, - fromBranch => $returnbranch->{branchcode}, + # 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(); - diag("Attempt return at returnbranch ('Anywhere' + Return -> Home and Return -> Holding 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 ( 0 , $allowed , 'Prevent return where item can\'t be transferred to both homebranch and holding from returnbranch'); - is ( $homebranch->{branchcode} , $message , 'Redirect to homebranch'); + ($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'); - $limit_2->delete()->store()->discard_changes; + 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'); - # Set limit ( Return -> Holding denied) + # Set limit ( Return -> Home denied) + diag("Transfer limit: Return -> Home"); $limit->set( { - toBranch => $holdingbranch->{branchcode}, + toBranch => $homebranch->{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'); + 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'); - 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 transferred to homebranch (holdingbranch forbidden) from returnbranch'); + ($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 ( Holding -> Home denied) + # Set limit ( Return -> Holding denied) + diag("Transfer limit: Return -> Holding"); $limit->set( { toBranch => $holdingbranch->{branchcode}, @@ -1991,30 +1930,15 @@ subtest 'CanBookBeReturned + UseBranchTransfertLimits' => sub { } )->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'); + 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'); - diag("Attempt return at returnbranch ('Anywhere' + Holding -> Home limit)"); - # NOTE: A return here can subsequently be transferred 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 transferred to from 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'); }; subtest 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub { -- 2.20.1