Attachment 116494 Details for Bug 7376 – Bug 7376: (follow-up) Extend test coverage and document

[patch] Bug 7376: (follow-up) Extend test coverage and document

Bug-7376-follow-up-Extend-test-coverage-and-docume.patch (text/plain), 11.24 KB, created by Martin Renvoize (ashimema) on 2021-02-08 12:46:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-02-08 12:46:38 UTC

Size: 11.24 KB

patch

obsolete

>From 17a984c19d6c784d4e80d410b5b501c6fd11b6c2 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Mon, 8 Feb 2021 11:44:37 +0000
>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.
>---
> t/db_dependent/Circulation.t | 162 +++++++++++++++++++++++++++++++----
> 1 file changed, 143 insertions(+), 19 deletions(-)
>
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 6ae2f9af57..3724051c22 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -1820,43 +1820,167 @@ subtest 'AddIssue & AllowReturnToBranch' => 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 $holdingbranch = $builder->build( { source => 'Branch' } );
>+    my $returnbranch  = $builder->build( { source => 'Branch' } );
>     my $patron        = $builder->build( { source => 'Borrower', 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,
>-        itemtype => $item->effective_itemtype,
>+        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;
> 
>-    ($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 ('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 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 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub {
>-- 
>2.20.1

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