Attachment 91133 Details for Bug 23172 – Bug 23172: Unit tests

[patch] Bug 23172: Unit tests

Bug-23172-Unit-tests.patch (text/plain), 9.23 KB, created by Alex Buckley on 2019-07-01 07:53:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Buckley

Created: 2019-07-01 07:53:24 UTC

Size: 9.23 KB

patch

obsolete

>From a5b16e27b0a7f6c091a12c4baa9fc5c0fd85a607 Mon Sep 17 00:00:00 2001
>From: Alex Buckley <alexbuckley@catalyst.net.nz>
>Date: Mon, 1 Jul 2019 04:17:35 +0000
>Subject: [PATCH] Bug 23172: Unit tests
>
>Adding unit tests to check the output of the
>Koha::Hold->check_if_existing_hold_matches_issuingrules()
>
>Test plan:
>1. Run test plan in first patch
>
>2. Enter Koha shell:
>sudo koha-shell <instancename>
>
>3. Run: prove t/db_dependent/Hold.t
>
>4. All tests should pass
>
>Sponsored-By: Brimbank Library, Australia
>---
> Koha/Hold.pm          |   6 +--
> t/db_dependent/Hold.t | 101 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 103 insertions(+), 4 deletions(-)
>
>diff --git a/Koha/Hold.pm b/Koha/Hold.pm
>index 6fe8462..5185e15 100644
>--- a/Koha/Hold.pm
>+++ b/Koha/Hold.pm
>@@ -361,7 +361,7 @@ sub check_if_existing_hold_matches_issuingrules {
>          $branchcode  = $item->{homebranch};
>     }
>     elsif ( $controlbranch eq "PatronLibrary" ) {
>-         $branchcode  = $patron->{branchcode};
>+         $branchcode  = $patron->branchcode;
>     }
> 
>     my $issuingrule = Koha::IssuingRules->get_effective_issuing_rule({
>@@ -370,8 +370,8 @@ sub check_if_existing_hold_matches_issuingrules {
>         itemtype => $item->{itype},
>     });
> 
>-    #Check if the patron catgeory/item type combination is valid
>-    if ( ($issuingrule->reservesallowed || $issuingrule->holds_per_record) == 0 ) {
>+    #Check if the patron category/item type combination is valid
>+    if ( ($issuingrule->reservesallowed == 0 || $issuingrule->holds_per_record == 0 || $issuingrule->holds_per_day == 0 )) {
>         return 'InvalidHold';
>     } else {
>         return 'OK';
>diff --git a/t/db_dependent/Hold.t b/t/db_dependent/Hold.t
>index 2ba6f7b..57df922 100755
>--- a/t/db_dependent/Hold.t
>+++ b/t/db_dependent/Hold.t
>@@ -28,8 +28,9 @@ use Koha::Holds;
> use Koha::Item;
> use Koha::DateUtils;
> use t::lib::TestBuilder;
>+use Koha::IssuingRules;
> 
>-use Test::More tests => 29;
>+use Test::More tests => 30;
> use Test::Exception;
> use Test::Warn;
> 
>@@ -270,3 +271,101 @@ subtest 'suspend() tests' => sub {
> 
>     $schema->storage->txn_rollback;
> };
>+
>+subtest "check_if_existing_hold_matches_issuingrules tests" => sub {
>+    plan tests => 9;
>+
>+    $schema->storage->txn_begin();
>+
>+    #Create test data
>+    my $branchcode = $builder->build( { source => 'Branch' } )->{ branchcode };
>+    my $categorycode1 = $builder->build({ source => 'Category' })->{categorycode};
>+    my $categorycode2 = $builder->build({ source => 'Category' })->{categorycode};
>+    my $patron1 = $builder->build({source => 'Borrower', value => { branchcode => $branchcode }});
>+
>+    my $item1 = $builder->build({ source => 'Item', value => { homebranch => $branchcode }});
>+    my $item2 = $builder->build({ source => 'Item', value => { homebranch => $branchcode }});
>+
>+    my $itemtype1 = $item1->{'itype'};
>+    my $itemtype2 = $item2->{'itype'};
>+
>+    Koha::IssuingRules->delete;
>+
>+    #Get branchcode
>+    my $controlbranch = C4::Context->preference('ReservesControlBranch');
>+
>+    if ( $controlbranch eq "ItemHomeLibrary" ) {
>+       $branchcode  = $item->{homebranch};
>+    }
>+    elsif ( $controlbranch eq "PatronLibrary" ) {
>+       $branchcode  = $patron1->{branchcode};
>+    }
>+
>+    #Test all cases when the check_return
>+    #check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' when reservesallowed, holds_per_record and holds_per_day are all 0
>+    my $rule1 = Koha::IssuingRule->new({
>+        branchcode =>  $branchcode,
>+        categorycode => $patron1->{'categorycode'},
>+        itemtype => $item1->{'itype'},
>+        reservesallowed => 0,
>+        holds_per_record => 0,
>+        holds_per_day => 0,
>+    })->store;
>+    my $checkreserve1 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} );
>+    is( $checkreserve1, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules reservesallowed, holds_per_record and holds_per_day is 0 for this patron category/itemtype combination' );
>+
>+    #Confirm that when any of the 4 reserves related issuingrules fields (reservesallowed, holds_per_record, holds_per_day) are set to 0 then check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' and so a newly returned item is not allocated to a specific bib-level reserve
>+
>+    #check_if_existing_hold_matches_issuingrules() returns 'InvalidHold' when reservesallowed is 0 for the patron category/itemtype combination
>+    $rule1->set({ reservesallowed => 0, holds_per_record => 1, holds_per_day => 1 })->store;
>+    my $checkreserve2 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} );
>+    is( $checkreserve2, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules reservesallowed is 0 for this patron category/itemtype combination' );
>+
>+    #Now change to reservesallowed = 1, holds_per_record = 0, holds_per_day = 1
>+    $rule1->set({ reservesallowed => 1, holds_per_record => 0, holds_per_day => 1 })->store;
>+    my $checkreserve3 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} );
>+    is( $checkreserve3, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules holds_per_record is set to 0 for this patron category/itemtype combination' );
>+
>+   #Now change to reservesallowed = 1, holds_per_record = 1, holds_per_day = 0
>+   $rule1->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 0 })->store;
>+   my $checkreserve4 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} );
>+   is( $checkreserve4, 'InvalidHold', 'Allocating item1 to a bib-reserve is not allowed as the issuingrules holds_per_record is set to0 for this patron category/itemtype combination' );
>+
>+   #Now change to reservesallowed = 1, holds_per_record = 1, holds_per_day = 1 and confirm 'OK' is returned
>+   $rule1->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 1 })->store;
>+   my $checkreserve5 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item1->{'itemnumber'} );
>+   is( $checkreserve5, 'OK', 'Allocating item2 to a bib-reserve is allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which allows reserves' );
>+
>+   #Create a default rule (patron categories: ALL, itemtypes: ALL)
>+   my $rule3 = Koha::IssuingRule->new({
>+       branchcode => $branchcode,
>+       categorycode => '*',
>+       itemtype => '*',
>+       reservesallowed => 1,
>+       holds_per_record => 1,
>+       holds_per_day => 1,
>+    })->store;
>+
>+    #Check that when no specific patron category/item type issuingrule combo exists we revert to using the default ALL categories/ALL 
>+    #itemtypes issuingrule. When generic rule is reservesallowed, holds_per_record and holds_per_day = 1 then 'OK' is returned
>+    my $checkreserve6 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} );
>+    is( $checkreserve6, 'OK', 'Allocating item2 to a bib-reserve is allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which allows reserves' );
>+
>+    #When default rule is reservesallowed = 0, holds_per_record = 1, holds_per_day = 1 then 'InvalidHold is returned
>+    $rule3->set({ reservesallowed => 0, holds_per_record => 1, holds_per_day => 1 })->store;
>+    my $checkreserve7 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} );
>+    is( $checkreserve7, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as reservesallowed is 0' );
>+
>+    #When default rule is reservesallowed = 1, holds_per_record = 0, holds_per_day = 1 then 'InvalidHold' is returned
>+    $rule3->set({ reservesallowed => 1, holds_per_record => 0, holds_per_day => 1 })->store;
>+    my $checkreserve8 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} );
>+    is( $checkreserve8, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as holds_per_record is 0' );
>+
>+    #When default rule is reservesallowed = 1, holds_per_record = 1, holds_per_day = 0 then 'InvalidHold' is returned
>+    $rule3->set({ reservesallowed => 1, holds_per_record => 1, holds_per_day => 0 })->store;
>+    my $checkreserve9 = Koha::Hold->check_if_existing_hold_matches_issuingrules($patron1->{'borrowernumber'}, $item2->{'itemnumber'} );
>+    is( $checkreserve9, 'InvalidHold', 'Allocating item2 to a bib-reserve is not allowed as there is no specific issuingrule for this patron category/itemtype combination and so we default to the all patron category/all item type rule which doesnt allows reserves as holds_per_day is 0' );
>+
>+    $schema->storage->txn_rollback;
>+};
>+
>-- 
>2.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 23172: 90868 | 91133 | 91301 | 92250 | 92251 | 92252 | 102099 | 102100 | 102101 | 102169 | 118930 | 118931 | 118932 | 118933 | 128817 | 128857