Bug 17600: Standardize our EXPORT_OK
[srvgit] / t / db_dependent / Holds.t
index fde87fe..0710eda 100755 (executable)
@@ -7,20 +7,21 @@ use t::lib::TestBuilder;
 
 use C4::Context;
 
-use Test::More tests => 61;
+use Test::More tests => 71;
 use MARC::Record;
 
 use C4::Biblio;
 use C4::Calendar;
 use C4::Items;
-use C4::Reserves;
+use C4::Reserves qw( AddReserve CalculatePriority ModReserve ToggleSuspend AutoUnsuspendReserves SuspendAll ModReserveMinusPriority AlterPriority CanItemBeReserved CheckReserves );
+use C4::Circulation qw( CanBookBeRenewed );
 
 use Koha::Biblios;
 use Koha::CirculationRules;
 use Koha::Database;
 use Koha::DateUtils qw( dt_from_string output_pref );
 use Koha::Holds;
-use Koha::IssuingRules;
+use Koha::Checkout;
 use Koha::Item::Transfer::Limits;
 use Koha::Items;
 use Koha::Libraries;
@@ -59,8 +60,7 @@ $insert_sth->execute('ONLY1');
 my $biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
 
 # Create item instance for testing.
-my ($item_bibnum, $item_bibitemnum, $itemnumber)
-    = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
+my $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber })->itemnumber;
 
 # Create some borrowers
 my @borrowernumbers;
@@ -77,17 +77,13 @@ foreach (1..$borrowers_count) {
 # Create five item level holds
 foreach my $borrowernumber ( @borrowernumbers ) {
     AddReserve(
-        $branch_1,
-        $borrowernumber,
-        $biblio->biblionumber,
-        my $bibitems = q{},
-        my $priority = C4::Reserves::CalculatePriority( $biblio->biblionumber ),
-        my $resdate,
-        my $expdate,
-        my $notes = q{},
-        'a title',
-        my $checkitem = $itemnumber,
-        my $found,
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => C4::Reserves::CalculatePriority( $biblio->biblionumber ),
+            itemnumber     => $itemnumber,
+        }
     );
 }
 
@@ -153,7 +149,7 @@ is( $hold->suspend_until, '2013-01-01 00:00:00', "Test ModReserve, suspend until
 
 ModReserve({ # call without reserve_id
     rank          => '3',
-    biblionumber  => $item_bibnum,
+    biblionumber  => $biblio->biblionumber,
     itemnumber    => $itemnumber,
     borrowernumber => $borrowernumber,
 });
@@ -192,25 +188,20 @@ is( $hold->suspend, 0, "Test resuming with SuspendAll()" );
 is( $hold->suspend_until, undef, "Test resuming with SuspendAll(), should have no suspend until date" );
 
 # Add a new hold for the borrower whose hold we canceled earlier, this time at the bib level
-AddReserve(
-    $branch_1,
-    $borrowernumbers[0],
-    $biblio->biblionumber,
-    my $bibitems = q{},
-    my $priority,
-    my $resdate,
-    my $expdate,
-    my $notes = q{},
-    'a title',
-    my $checkitem,
-    my $found,
-);
+    AddReserve(
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumbers[0],
+            biblionumber   => $biblio->biblionumber,
+        }
+    );
+
 $patron = Koha::Patrons->find( $borrowernumber );
 $holds = $patron->holds;
 my $reserveid = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $borrowernumbers[0] })->next->reserve_id;
 ModReserveMinusPriority( $itemnumber, $reserveid );
 $holds = $patron->holds;
-is( $holds->next->itemnumber, $itemnumber, "Test ModReserveMinusPriority()" );
+is( $holds->search({ itemnumber => $itemnumber })->count, 1, "Test ModReserveMinusPriority()" );
 
 $holds = $biblio->holds;
 $hold = $holds->next;
@@ -242,13 +233,12 @@ is( $hold->priority, '6', "Test AlterPriority(), move to bottom" );
 # IndependentBranches is OFF.
 
 my $foreign_biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
-my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber)
-  = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_biblio->biblionumber);
+my $foreign_itemnumber = $builder->build_sample_item({ library => $branch_2, biblionumber => $foreign_biblio->biblionumber })->itemnumber;
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed  => 25,
             holds_per_record => 99,
@@ -257,8 +247,8 @@ Koha::CirculationRules->set_rules(
 );
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        branchcode   => undef,
         itemtype     => 'CANNOT',
         rules        => {
             reservesallowed  => 0,
@@ -297,12 +287,36 @@ ok(
 {
     # Regression test for bug 11336 # Test if ModReserve correctly recalculate the priorities
     $biblio = $builder->build_sample_biblio({ itemtype => 'DUMMY' });
-    ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
-    my $reserveid1 = AddReserve($branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1);
-    ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
-    my $reserveid2 = AddReserve($branch_1, $borrowernumbers[1], $biblio->biblionumber, '', 2);
-    ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1 } , $biblio->biblionumber);
-    my $reserveid3 = AddReserve($branch_1, $borrowernumbers[2], $biblio->biblionumber, '', 3);
+    $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber })->itemnumber;
+    my $reserveid1 = AddReserve(
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumbers[0],
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1
+        }
+    );
+
+    $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber })->itemnumber;
+    my $reserveid2 = AddReserve(
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumbers[1],
+            biblionumber   => $biblio->biblionumber,
+            priority       => 2
+        }
+    );
+
+    $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber })->itemnumber;
+    my $reserveid3 = AddReserve(
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumbers[2],
+            biblionumber   => $biblio->biblionumber,
+            priority       => 3
+        }
+    );
+
     my $hhh = Koha::Holds->search({ biblionumber => $biblio->biblionumber });
     my $hold3 = Koha::Holds->find( $reserveid3 );
     is( $hold3->priority, 3, "The 3rd hold should have a priority set to 3" );
@@ -311,7 +325,7 @@ ok(
     is( $hold3->discard_changes->priority, 1, "After ModReserve, the 3rd reserve becomes the first on the waiting list" );
 }
 
-ModItem({ damaged => 1 }, $item_bibnum, $itemnumber);
+Koha::Items->find($itemnumber)->damaged(1)->store; # FIXME The $itemnumber is a bit confusing here
 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 );
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" );
 ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" );
@@ -320,7 +334,7 @@ $hold = Koha::Hold->new(
     {
         borrowernumber => $borrowernumbers[0],
         itemnumber     => $itemnumber,
-        biblionumber   => $item_bibnum,
+        biblionumber   => $biblio->biblionumber,
     }
 )->store();
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status},
@@ -332,32 +346,270 @@ t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 );
 ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
 ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" );
 
+# Items that are not for loan, but holdable should not be trapped until they are available for loan
+t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 0 );
+Koha::Items->find($itemnumber)->damaged(0)->notforloan(-1)->store;
+Koha::Holds->search({ biblionumber => $biblio->id })->delete();
+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can place hold on item that is not for loan but holdable ( notforloan < 0 )" );
+$hold = Koha::Hold->new(
+    {
+        borrowernumber => $borrowernumbers[0],
+        itemnumber     => $itemnumber,
+        biblionumber   => $biblio->biblionumber,
+        found          => undef,
+        priority       => 1,
+        reservedate    => dt_from_string,
+        branchcode     => $branch_1,
+    }
+)->store();
+ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" );
+t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 1 );
+ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold is trapped for item that is not for loan but holdable ( notforloan < 0 )" );
+t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1' );
+ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" );
+t::lib::Mocks::mock_preference( 'SkipHoldTrapOnNotForLoanValue', '-1|1' );
+ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item with notforloan value matching SkipHoldTrapOnNotForLoanValue" );
+is(
+    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'itemAlreadyOnHold',
+    "cannot request item that you have already reservedd"
+);
+is(
+    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber, undef, { ignore_hold_counts => 1 })->{status}, 'OK',
+    "can request item if we are not checking holds counts, but only if policy allows or forbids it"
+);
+$hold->delete();
+
 # Regression test for bug 9532
 $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblio->biblionumber);
+$item = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber});
 AddReserve(
-    $branch_1,
-    $borrowernumbers[0],
-    $biblio->biblionumber,
-    '',
-    1,
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumbers[0],
+        biblionumber   => $biblio->biblionumber,
+        priority       => 1,
+    }
 );
 is(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'tooManyReserves',
+    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber)->{status}, 'noReservesAllowed',
     "cannot request item if policy that matches on item-level item type forbids it"
 );
-ModItem({ itype => 'CAN' }, $item_bibnum, $itemnumber);
-ok(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'OK',
-    "can request item if policy that matches on item type allows it"
+is(
+    CanItemBeReserved( $borrowernumbers[0], $item->itemnumber, undef, { ignore_hold_counts => 1 })->{status}, 'noReservesAllowed',
+    "cannot request item if policy that matches on item-level item type forbids it even if ignoring counts"
 );
 
-t::lib::Mocks::mock_preference('item-level_itypes', 0);
-ModItem({ itype => undef }, $item_bibnum, $itemnumber);
-ok(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'tooManyReserves',
-    "cannot request item if policy that matches on bib-level item type forbids it (bug 9532)"
-);
+subtest 'CanItemBeReserved' => sub {
+    plan tests => 2;
+
+    my $itemtype_can         = $builder->build({source => "Itemtype"})->{itemtype};
+    my $itemtype_cant        = $builder->build({source => "Itemtype"})->{itemtype};
+    my $itemtype_cant_record = $builder->build({source => "Itemtype"})->{itemtype};
+
+    Koha::CirculationRules->set_rules(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => $itemtype_cant,
+            rules        => {
+                reservesallowed  => 0,
+                holds_per_record => 99,
+            }
+        }
+    );
+    Koha::CirculationRules->set_rules(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => $itemtype_can,
+            rules        => {
+                reservesallowed  => 2,
+                holds_per_record => 2,
+            }
+        }
+    );
+    Koha::CirculationRules->set_rules(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => $itemtype_cant_record,
+            rules        => {
+                reservesallowed  => 0,
+                holds_per_record => 0,
+            }
+        }
+    );
+
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode => $branch_1,
+            itemtype   => $itemtype_cant,
+            rules => {
+                holdallowed => 0,
+                returnbranch => 'homebranch',
+            }
+        }
+    );
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode => $branch_1,
+            itemtype   => $itemtype_can,
+            rules => {
+                holdallowed => 1,
+                returnbranch => 'homebranch',
+            }
+        }
+    );
+
+    subtest 'noReservesAllowed' => sub {
+        plan tests => 5;
+
+        my $biblionumber_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant })->biblionumber;
+        my $biblionumber_can = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
+        my $biblionumber_record_cannot = $builder->build_sample_biblio({ itemtype => $itemtype_cant_record })->biblionumber;
+
+        my $itemnumber_1_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_cannot })->itemnumber;
+        my $itemnumber_1_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_cannot })->itemnumber;
+        my $itemnumber_2_can = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_can })->itemnumber;
+        my $itemnumber_2_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant, biblionumber => $biblionumber_can })->itemnumber;
+        my $itemnumber_3_cannot = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_cant_record, biblionumber => $biblionumber_record_cannot })->itemnumber;
+
+        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
+
+        t::lib::Mocks::mock_preference('item-level_itypes', 1);
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'noReservesAllowed',
+            "With item level set, rule from item must be picked (CANNOT)"
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'OK',
+            "With item level set, rule from item must be picked (CAN)"
+        );
+        t::lib::Mocks::mock_preference('item-level_itypes', 0);
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_1_can)->{status}, 'noReservesAllowed',
+            "With biblio level set, rule from biblio must be picked (CANNOT)"
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_2_cannot)->{status}, 'OK',
+            "With biblio level set, rule from biblio must be picked (CAN)"
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_3_cannot)->{status}, 'noReservesAllowed',
+            "When no holds allowed and no holds per record allowed should return noReservesAllowed"
+        );
+    };
+
+    subtest 'tooManyHoldsForThisRecord + tooManyReserves + itemAlreadyOnHold' => sub {
+        plan tests => 7;
+
+        my $biblionumber_1 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
+        my $itemnumber_11 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
+        my $itemnumber_12 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_1 })->itemnumber;
+        my $biblionumber_2 = $builder->build_sample_biblio({ itemtype => $itemtype_can })->biblionumber;
+        my $itemnumber_21 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
+        my $itemnumber_22 = $builder->build_sample_item({ homebranch => $branch_1, holdingbranch => $branch_1, itype => $itemtype_can, biblionumber => $biblionumber_2 })->itemnumber;
+
+        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
+
+        # Biblio-level hold
+        AddReserve({
+            branch => $branch_1,
+            borrowernumber => $borrowernumbers[0],
+            biblionumber => $biblionumber_1,
+        });
+        for my $item_level ( 0..1 ) {
+            t::lib::Mocks::mock_preference('item-level_itypes', $item_level);
+            is(
+                # FIXME This is not really correct, but CanItemBeReserved does not check if biblio-level holds already exist
+                CanItemBeReserved( $borrowernumbers[0], $itemnumber_11)->{status}, 'OK',
+                "A biblio-level hold already exists - another hold can be placed on a specific item item"
+            );
+        }
+
+        Koha::Holds->search({borrowernumber => $borrowernumbers[0]})->delete;
+        # Item-level hold
+        AddReserve({
+            branch => $branch_1,
+            borrowernumber => $borrowernumbers[0],
+            biblionumber => $biblionumber_1,
+            itemnumber => $itemnumber_11,
+        });
+
+        $dbh->do('DELETE FROM circulation_rules');
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
+                rules        => {
+                    reservesallowed  => 5,
+                    holds_per_record => 1,
+                }
+            }
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
+            "A item-level hold already exists and holds_per_record=1, another hold cannot be placed on this record"
+        );
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
+                rules        => {
+                    reservesallowed  => 1,
+                    holds_per_record => 1,
+                }
+            }
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'tooManyHoldsForThisRecord',
+            "A item-level hold already exists and holds_per_record=1 - tooManyHoldsForThisRecord has priority over tooManyReserves"
+        );
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
+                rules        => {
+                    reservesallowed  => 5,
+                    holds_per_record => 2,
+                }
+            }
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_12)->{status}, 'OK',
+            "A item-level hold already exists but holds_per_record=2- another item-level hold can be placed on this record"
+        );
+
+        AddReserve({
+            branch => $branch_1,
+            borrowernumber => $borrowernumbers[0],
+            biblionumber => $biblionumber_2,
+            itemnumber => $itemnumber_21
+        });
+        Koha::CirculationRules->set_rules(
+            {
+                categorycode => undef,
+                branchcode   => undef,
+                itemtype     => undef,
+                rules        => {
+                    reservesallowed  => 2,
+                    holds_per_record => 2,
+                }
+            }
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_21)->{status}, 'itemAlreadyOnHold',
+            "A item-level holds already exists on this item, itemAlreadyOnHold should be raised"
+        );
+        is(
+            CanItemBeReserved( $borrowernumbers[0], $itemnumber_22)->{status}, 'tooManyReserves',
+            "This patron has already placed reservesallowed holds, tooManyReserves should be raised"
+        );
+    };
+};
 
 
 # Test branch item rules
@@ -365,9 +617,9 @@ ok(
 $dbh->do('DELETE FROM circulation_rules');
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
-        itemtype     => '*',
+        categorycode => undef,
+        branchcode   => undef,
+        itemtype     => undef,
         rules        => {
             reservesallowed  => 25,
             holds_per_record => 99,
@@ -378,9 +630,8 @@ Koha::CirculationRules->set_rules(
     {
         branchcode => $branch_1,
         itemtype   => 'CANNOT',
-        categorycode => undef,
         rules => {
-            holdallowed => 0,
+            holdallowed => 'not_allowed',
             returnbranch => 'homebranch',
         }
     }
@@ -389,22 +640,19 @@ Koha::CirculationRules->set_rules(
     {
         branchcode => $branch_1,
         itemtype   => 'CAN',
-        categorycode => undef,
         rules => {
-            holdallowed => 1,
+            holdallowed => 'from_home_library',
             returnbranch => 'homebranch',
         }
     }
 );
 $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem(
-    { homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $biblio->biblionumber);
+$itemnumber = $builder->build_sample_item({ library => $branch_1, itype => 'CANNOT', biblionumber => $biblio->biblionumber})->itemnumber;
 is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, 'notReservable',
     "CanItemBeReserved should return 'notReservable'");
 
 t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'PatronLibrary' );
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem(
-    { homebranch => $branch_2, holdingbranch => $branch_1, itype => 'CAN' } , $biblio->biblionumber);
+$itemnumber = $builder->build_sample_item({ library => $branch_2, itype => 'CAN', biblionumber => $biblio->biblionumber})->itemnumber;
 is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status},
     'cannotReserveFromOtherBranches',
     "CanItemBeReserved should use PatronLibrary rule when ReservesControlBranch set to 'PatronLibrary'");
@@ -413,8 +661,7 @@ is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status},
     'OK',
     "CanItemBeReserved should use item home library rule when ReservesControlBranch set to 'ItemsHomeLibrary'");
 
-($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem(
-    { homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CAN' } , $biblio->biblionumber);
+$itemnumber = $builder->build_sample_item({ library => $branch_1, itype => 'CAN', biblionumber => $biblio->biblionumber})->itemnumber;
 is(CanItemBeReserved($borrowernumbers[0], $itemnumber)->{status}, 'OK',
     "CanItemBeReserved should return 'OK'");
 
@@ -428,13 +675,12 @@ $dbh->do('DELETE FROM items');
 $dbh->do('DELETE FROM biblio');
 
 $biblio = $builder->build_sample_biblio({ itemtype => 'ONLY1' });
-( $item_bibnum, $item_bibitemnum, $itemnumber )
-    = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $biblio->biblionumber );
+$itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber;
 
 Koha::CirculationRules->set_rules(
     {
-        categorycode => '*',
-        branchcode   => '*',
+        categorycode => undef,
+        branchcode   => undef,
         itemtype     => 'ONLY1',
         rules        => {
             reservesallowed  => 1,
@@ -445,10 +691,19 @@ Koha::CirculationRules->set_rules(
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status},
     'OK', 'Patron can reserve item with hold limit of 1, no holds placed' );
 
-my $res_id = AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
+my $res_id = AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumbers[0],
+        biblionumber   => $biblio->biblionumber,
+        priority       => 1,
+    }
+);
 
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber )->{status},
     'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' );
+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber, undef, { ignore_hold_counts => 1 } )->{status},
+    'OK', 'Patron can reserve item if checking policy but not counts' );
 
     #results should be the same for both ReservesControlBranch settings
 t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' );
@@ -461,26 +716,31 @@ subtest 'Test max_holds per library/patron category' => sub {
     plan tests => 6;
 
     $dbh->do('DELETE FROM reserves');
-    $dbh->do('DELETE FROM circulation_rules');
 
-    $biblio = $builder->build_sample_biblio({ itemtype => 'TEST' });
-    ( $item_bibnum, $item_bibitemnum, $itemnumber ) =
-      AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 },
-        $biblio->biblionumber );
+    $biblio = $builder->build_sample_biblio;
+    $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber;
     Koha::CirculationRules->set_rules(
         {
-            categorycode => '*',
-            branchcode   => '*',
-            itemtype     => 'TEST',
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => $biblio->itemtype,
             rules        => {
                 reservesallowed  => 99,
                 holds_per_record => 99,
             }
         }
     );
-    AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
-    AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
-    AddReserve( $branch_1, $borrowernumbers[0], $biblio->biblionumber, '', 1, );
+
+    for ( 1 .. 3 ) {
+        AddReserve(
+            {
+                branchcode     => $branch_1,
+                borrowernumber => $borrowernumbers[0],
+                biblionumber   => $biblio->biblionumber,
+                priority       => 1,
+            }
+        );
+    }
 
     my $count =
       Koha::Holds->search( { borrowernumber => $borrowernumbers[0] } )->count();
@@ -493,7 +753,6 @@ subtest 'Test max_holds per library/patron category' => sub {
         {
             categorycode => $category->{categorycode},
             branchcode   => undef,
-            itemtype     => undef,
             rule_name    => 'max_holds',
             rule_value   => 3,
         }
@@ -503,7 +762,6 @@ subtest 'Test max_holds per library/patron category' => sub {
         {
             branchcode   => $branch_1,
             categorycode => $category->{categorycode},
-            itemtype     => undef,
             rule_name    => 'max_holds',
             rule_value   => 5,
         }
@@ -537,14 +795,19 @@ subtest 'Pickup location availability tests' => sub {
     plan tests => 4;
 
     $biblio = $builder->build_sample_biblio({ itemtype => 'ONLY1' });
-    my ( $item_bibnum, $item_bibitemnum, $itemnumber )
-    = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $biblio->biblionumber );
+    $itemnumber = $builder->build_sample_item({ library => $branch_1, biblionumber => $biblio->biblionumber})->itemnumber;
     #Add a default rule to allow some holds
-    $dbh->do(
-        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
-          VALUES (?, ?, ?, ?, ?)},
-        {},
-        '*', '*', '*', 25, 99
+
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
+            rules        => {
+                reservesallowed  => 25,
+                holds_per_record => 99,
+            }
+        }
     );
     my $item = Koha::Items->find($itemnumber);
     my $branch_to = $builder->build({ source => 'Branch' })->{ branchcode };
@@ -583,50 +846,30 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
 
     $schema->storage->txn_begin;
 
-    Koha::Holds->search->delete;
-    $dbh->do('DELETE FROM issues');
-    $dbh->do('DELETE FROM issuingrules');
-    $dbh->do('DELETE FROM circulation_rules');
-    Koha::Items->search->delete;
-    Koha::Biblios->search->delete;
-
     my $itemtype = $builder->build_object( { class => 'Koha::ItemTypes' } );
     my $library  = $builder->build_object( { class => 'Koha::Libraries' } );
     my $patron   = $builder->build_object( { class => 'Koha::Patrons' } );
 
     # Create 3 biblios with items
     my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype->itemtype });
-    my ( undef, undef, $itemnumber_1 ) = AddItem(
-        {   homebranch    => $library->branchcode,
-            holdingbranch => $library->branchcode
-        },
-        $biblio_1->biblionumber
-    );
+    my $itemnumber_1 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_1->biblionumber})->itemnumber;
     my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype->itemtype });
-    my ( undef, undef, $itemnumber_2 ) = AddItem(
-        {   homebranch    => $library->branchcode,
-            holdingbranch => $library->branchcode
-        },
-        $biblio_2->biblionumber
-    );
+    my $itemnumber_2 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_2->biblionumber})->itemnumber;
     my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype->itemtype });
-    my ( undef, undef, $itemnumber_3 ) = AddItem(
-        {   homebranch    => $library->branchcode,
-            holdingbranch => $library->branchcode
-        },
-        $biblio_3->biblionumber
-    );
-
-    Koha::IssuingRules->search->delete;
-    my $issuingrule = Koha::IssuingRule->new(
-        {   categorycode     => '*',
-            branchcode       => '*',
-            itemtype         => $itemtype->itemtype,
-            reservesallowed  => 1,
-            holds_per_record => 99,
-            holds_per_day    => 2
+    my $itemnumber_3 = $builder->build_sample_item({ library => $library->branchcode, biblionumber => $biblio_3->biblionumber})->itemnumber;
+
+    Koha::CirculationRules->set_rules(
+        {
+            categorycode => '*',
+            branchcode   => '*',
+            itemtype     => $itemtype->itemtype,
+            rules        => {
+                reservesallowed  => 1,
+                holds_per_record => 99,
+                holds_per_day    => 2
+            }
         }
-    )->store;
+    );
 
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_1 ),
@@ -634,7 +877,14 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
         'Patron can reserve item with hold limit of 1, no holds placed'
     );
 
-    AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_1->biblionumber, '', 1, );
+    AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio_1->biblionumber,
+            priority       => 1,
+        }
+    );
 
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_1 ),
@@ -643,7 +893,16 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
     );
 
     # Raise reservesallowed to avoid tooManyReserves from it
-    $issuingrule->set( { reservesallowed => 3 } )->store;
+    Koha::CirculationRules->set_rule(
+        {
+
+            categorycode => '*',
+            branchcode   => '*',
+            itemtype     => $itemtype->itemtype,
+            rule_name  => 'reservesallowed',
+            rule_value => 3,
+        }
+    );
 
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ),
@@ -652,7 +911,14 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
     );
 
     # Add a second reserve
-    my $res_id = AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_2->biblionumber, '', 1, );
+    my $res_id = AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio_2->biblionumber,
+            priority       => 1,
+        }
+    );
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ),
         { status => 'tooManyReservesToday', limit => 2 },
@@ -671,7 +937,17 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
     );
 
     # Set holds_per_day to 0
-    $issuingrule->set( { holds_per_day => 0 } )->store;
+    Koha::CirculationRules->set_rule(
+        {
+
+            categorycode => '*',
+            branchcode   => '*',
+            itemtype     => $itemtype->itemtype,
+            rule_name  => 'holds_per_day',
+            rule_value => 0,
+        }
+    );
+
 
     # Delete existing holds
     Koha::Holds->search->delete;
@@ -681,21 +957,53 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
         'Patron cannot reserve if holds_per_day is 0 (i.e. 0 is 0)'
     );
 
-    $issuingrule->set( { holds_per_day => undef } )->store;
+    Koha::CirculationRules->set_rule(
+        {
+
+            categorycode => '*',
+            branchcode   => '*',
+            itemtype     => $itemtype->itemtype,
+            rule_name  => 'holds_per_day',
+            rule_value => undef,
+        }
+    );
+
     Koha::Holds->search->delete;
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_2 ),
         { status => 'OK' },
         'Patron can reserve if holds_per_day is undef (i.e. undef is unlimited daily cap)'
     );
-    AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_1->biblionumber, '', 1, );
-    AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_2->biblionumber, '', 1, );
+    AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio_1->biblionumber,
+            priority       => 1,
+        }
+    );
+    AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio_2->biblionumber,
+            priority       => 1,
+        }
+    );
+
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ),
         { status => 'OK' },
         'Patron can reserve if holds_per_day is undef (i.e. undef is unlimited daily cap)'
     );
-    AddReserve( $library->branchcode, $patron->borrowernumber, $biblio_3->biblionumber, '', 1, );
+    AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio_3->biblionumber,
+            priority       => 1,
+        }
+    );
     is_deeply(
         CanItemBeReserved( $patron->borrowernumber, $itemnumber_3 ),
         { status => 'tooManyReserves', limit => 3 },
@@ -717,20 +1025,15 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
 
     $schema->storage->txn_begin;
 
-    # Cleanup database
-    Koha::Holds->search->delete;
-    $dbh->do('DELETE FROM issues');
-    $dbh->do('DELETE FROM issuingrules');
-    $dbh->do(
-        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
-        VALUES (?, ?, ?, ?)},
-        {},
-        '*', '*', '*', 25
+    Koha::CirculationRules->set_rule(
+        {
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
+            rule_name    => 'reservesallowed',
+            rule_value   => 25,
+        }
     );
-    $dbh->do('DELETE FROM circulation_rules');
-
-    Koha::Items->search->delete;
-    Koha::Biblios->search->delete;
 
     # Create item types
     my $itemtype1 = $builder->build_object( { class => 'Koha::ItemTypes' } );
@@ -752,25 +1055,26 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
 
     # Create 3 biblios with items
     my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
-    my ( undef, undef, $itemnumber_1 ) = AddItem(
-        {   homebranch    => $library1->branchcode,
-            holdingbranch => $library1->branchcode
-        },
-        $biblio_1->biblionumber
+    my $item_1   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio_1->biblionumber,
+            library      => $library1->branchcode
+        }
     );
     my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype2->itemtype });
-    my ( undef, undef, $itemnumber_2 ) = AddItem(
-        {   homebranch    => $library2->branchcode,
-            holdingbranch => $library2->branchcode
-        },
-        $biblio_2->biblionumber
+    my $item_2   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio_2->biblionumber,
+            library      => $library2->branchcode
+        }
     );
+    my $itemnumber_2 = $item_2->itemnumber;
     my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
-    my ( undef, undef, $itemnumber_3 ) = AddItem(
-        {   homebranch    => $library1->branchcode,
-            holdingbranch => $library1->branchcode
-        },
-        $biblio_3->biblionumber
+    my $item_3   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio_3->biblionumber,
+            library      => $library1->branchcode
+        }
     );
 
     # Test 1: Patron 3 can place hold
@@ -785,9 +1089,8 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
         {
             branchcode => undef,
             itemtype   => undef,
-            categorycode => undef,
             rules => {
-                holdallowed => 3,
+                holdallowed => 'from_local_hold_group',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -808,17 +1111,13 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
         'Patron cannot place hold because patron\'s home library is not part of hold group'
     );
 
-    # Cleanup default_cirt_rules
-    $dbh->do('DELETE FROM circulation_rules');
-
     # Insert default circ rule to "any" for library 2
     Koha::CirculationRules->set_rules(
         {
             branchcode => $library2->branchcode,
             itemtype   => undef,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -837,9 +1136,8 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
         {
             branchcode => $library2->branchcode,
             itemtype   => undef,
-            categorycode => undef,
             rules => {
-                holdallowed => 3,
+                holdallowed => 'from_local_hold_group',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -853,17 +1151,13 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
         'Patron cannot place hold if holdallowed is set to "hold group" for library 2'
     );
 
-    # Cleanup default_branch_cirt_rules
-    $dbh->do('DELETE FROM circulation_rules');
-
     # Insert default item rule to "any" for itemtype 2
     Koha::CirculationRules->set_rules(
         {
-            branchcode => undef,
+            branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -880,11 +1174,10 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
     # Update default item rule to "hold group" for itemtype 2
     Koha::CirculationRules->set_rules(
         {
-            branchcode => undef,
+            branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 3,
+                holdallowed => 'from_local_hold_group',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -898,17 +1191,13 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
         'Patron cannot place hold if holdallowed is set to "hold group" for itemtype 2'
     );
 
-    # Cleanup default_branch_item_rules
-    $dbh->do('DELETE FROM circulation_rules');
-
     # Insert branch item rule to "any" for itemtype 2 and library 2
     Koha::CirculationRules->set_rules(
         {
             branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -927,9 +1216,8 @@ subtest 'CanItemBeReserved / branch_not_in_hold_group' => sub {
         {
             branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 3,
+                holdallowed => 'from_local_hold_group',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -951,30 +1239,24 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
     plan tests => 9;
 
     $schema->storage->txn_begin;
-
-    # Cleanup database
-    Koha::Holds->search->delete;
-    $dbh->do('DELETE FROM issues');
-    $dbh->do('DELETE FROM issuingrules');
-    $dbh->do(
-        q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
-        VALUES (?, ?, ?, ?)},
-        {},
-        '*', '*', '*', 25
+    Koha::CirculationRules->set_rule(
+        {
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
+            rule_name    => 'reservesallowed',
+            rule_value   => 25,
+        }
     );
-    $dbh->do('DELETE FROM circulation_rules');
-
-    Koha::Items->search->delete;
-    Koha::Biblios->search->delete;
 
     # Create item types
     my $itemtype1 = $builder->build_object( { class => 'Koha::ItemTypes' } );
     my $itemtype2 = $builder->build_object( { class => 'Koha::ItemTypes' } );
 
     # Create libraries
-    my $library1  = $builder->build_object( { class => 'Koha::Libraries' } );
-    my $library2  = $builder->build_object( { class => 'Koha::Libraries' } );
-    my $library3  = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $library1  = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } );
+    my $library2  = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } );
+    my $library3  = $builder->build_object( { class => 'Koha::Libraries', value => {pickup_location => 1} } );
 
     # Create library groups hierarchy
     my $rootgroup  = $builder->build_object( { class => 'Koha::Library::Groups', value => {ft_local_hold_group => 1} } );
@@ -987,25 +1269,26 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
 
     # Create 3 biblios with items
     my $biblio_1 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
-    my ( undef, undef, $itemnumber_1 ) = AddItem(
-        {   homebranch    => $library1->branchcode,
-            holdingbranch => $library1->branchcode
-        },
-        $biblio_1->biblionumber
+    my $item_1   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio_1->biblionumber,
+            library      => $library1->branchcode
+        }
     );
     my $biblio_2 = $builder->build_sample_biblio({ itemtype => $itemtype2->itemtype });
-    my ( undef, undef, $itemnumber_2 ) = AddItem(
-        {   homebranch    => $library2->branchcode,
-            holdingbranch => $library2->branchcode
-        },
-        $biblio_2->biblionumber
+    my $item_2   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio_2->biblionumber,
+            library      => $library2->branchcode
+        }
     );
+    my $itemnumber_2 = $item_2->itemnumber;
     my $biblio_3 = $builder->build_sample_biblio({ itemtype => $itemtype1->itemtype });
-    my ( undef, undef, $itemnumber_3 ) = AddItem(
-        {   homebranch    => $library1->branchcode,
-            holdingbranch => $library1->branchcode
-        },
-        $biblio_3->biblionumber
+    my $item_3   = $builder->build_sample_item(
+        {
+            biblionumber => $biblio_3->biblionumber,
+            library      => $library1->branchcode
+        }
     );
 
     # Test 1: Patron 3 can place hold
@@ -1020,9 +1303,8 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
         {
             branchcode => undef,
             itemtype   => undef,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'holdgroup',
                 returnbranch => 'any'
             }
@@ -1043,17 +1325,13 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
         'Patron cannot place hold because pickup location is not part of hold group'
     );
 
-    # Cleanup default_cirt_rules
-    $dbh->do('DELETE FROM circulation_rules');
-
     # Insert default circ rule to "any" for library 2
     Koha::CirculationRules->set_rules(
         {
             branchcode => $library2->branchcode,
             itemtype   => undef,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -1072,9 +1350,8 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
         {
             branchcode => $library2->branchcode,
             itemtype   => undef,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'holdgroup',
                 returnbranch => 'any'
             }
@@ -1088,17 +1365,13 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
         'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for library 2'
     );
 
-    # Cleanup default_branch_cirt_rules
-    $dbh->do('DELETE FROM circulation_rules');
-
     # Insert default item rule to "any" for itemtype 2
     Koha::CirculationRules->set_rules(
         {
-            branchcode => undef,
+            branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -1115,11 +1388,10 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
     # Update default item rule to "hold group" for itemtype 2
     Koha::CirculationRules->set_rules(
         {
-            branchcode => undef,
+            branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'holdgroup',
                 returnbranch => 'any'
             }
@@ -1133,17 +1405,13 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
         'Patron cannot place hold if hold_fulfillment_policy is set to "hold group" for itemtype 2'
     );
 
-    # Cleanup default_branch_item_rules
-    $dbh->do('DELETE FROM circulation_rules');
-
     # Insert branch item rule to "any" for itemtype 2 and library 2
     Koha::CirculationRules->set_rules(
         {
             branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'any',
                 returnbranch => 'any'
             }
@@ -1162,9 +1430,8 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
         {
             branchcode => $library2->branchcode,
             itemtype   => $itemtype2->itemtype,
-            categorycode => undef,
             rules => {
-                holdallowed => 2,
+                holdallowed => 'from_any_library',
                 hold_fulfillment_policy => 'holdgroup',
                 returnbranch => 'any'
             }
@@ -1180,3 +1447,166 @@ subtest 'CanItemBeReserved / pickup_not_in_hold_group' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'non priority holds' => sub {
+
+    plan tests => 6;
+
+    $schema->storage->txn_begin;
+
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode   => undef,
+            categorycode => undef,
+            itemtype     => undef,
+            rules        => {
+                renewalsallowed => 5,
+                reservesallowed => 5,
+            }
+        }
+    );
+
+    my $item = $builder->build_sample_item;
+
+    my $patron1 = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { branchcode => $item->homebranch }
+        }
+    );
+    my $patron2 = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { branchcode => $item->homebranch }
+        }
+    );
+
+    Koha::Checkout->new(
+        {
+            borrowernumber => $patron1->borrowernumber,
+            itemnumber     => $item->itemnumber,
+            branchcode     => $item->homebranch
+        }
+    )->store;
+
+    my $hid = AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $patron2->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => 1,
+            itemnumber     => $item->itemnumber,
+        }
+    );
+
+    my ( $ok, $err ) =
+      CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber );
+
+    ok( !$ok, 'Cannot renew' );
+    is( $err, 'on_reserve', 'Item is on hold' );
+
+    my $hold = Koha::Holds->find($hid);
+    $hold->non_priority(1)->store;
+
+    ( $ok, $err ) =
+      CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber );
+
+    ok( $ok, 'Can renew' );
+    is( $err, undef, 'Item is on non priority hold' );
+
+    my $patron3 = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { branchcode => $item->homebranch }
+        }
+    );
+
+    # Add second hold with non_priority = 0
+    AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $patron3->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => 2,
+            itemnumber     => $item->itemnumber,
+        }
+    );
+
+    ( $ok, $err ) =
+      CanBookBeRenewed( $patron1->borrowernumber, $item->itemnumber );
+
+    ok( !$ok, 'Cannot renew' );
+    is( $err, 'on_reserve', 'Item is on hold' );
+
+    $schema->storage->txn_rollback;
+
+};
+
+subtest 'CanItemBeReserved rule precedence tests' => sub {
+
+    plan tests => 3;
+
+    t::lib::Mocks::mock_preference('ReservesControlBranch', 'ItemHomeLibrary');
+    $schema->storage->txn_begin;
+    my $library  = $builder->build_object( { class => 'Koha::Libraries', value => {
+        pickup_location => 1,
+    }});
+    my $item = $builder->build_sample_item({
+        homebranch    => $library->branchcode,
+        holdingbranch => $library->branchcode
+    });
+    my $item2 = $builder->build_sample_item({
+        homebranch    => $library->branchcode,
+        holdingbranch => $library->branchcode,
+        itype         => $item->itype
+    });
+    my $patron   = $builder->build_object({ class => 'Koha::Patrons', value => {
+        branchcode => $library->branchcode
+    }});
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode   => undef,
+            categorycode => $patron->categorycode,
+            itemtype     => $item->itype,
+            rules        => {
+                reservesallowed  => 1,
+            }
+        }
+    );
+    is_deeply(
+        CanItemBeReserved( $patron->borrowernumber, $item->itemnumber, $library->branchcode ),
+        { status => 'OK' },
+        'Patron of specified category can place 1 hold on specified itemtype'
+    );
+    my $hold = $builder->build_object({ class => 'Koha::Holds', value => {
+        biblionumber   => $item2->biblionumber,
+        itemnumber     => $item2->itemnumber,
+        found          => undef,
+        priority       => 1,
+        branchcode     => $library->branchcode,
+        borrowernumber => $patron->borrowernumber,
+    }});
+    is_deeply(
+        CanItemBeReserved( $patron->borrowernumber, $item->itemnumber, $library->branchcode ),
+        { status => 'tooManyReserves', limit => 1 },
+        'Patron of specified category can place 1 hold on specified itemtype, cannot place a second'
+    );
+    Koha::CirculationRules->set_rules(
+        {
+            branchcode   => $library->branchcode,
+            categorycode => undef,
+            itemtype     => undef,
+            rules        => {
+                reservesallowed  => 2,
+            }
+        }
+    );
+    is_deeply(
+        CanItemBeReserved( $patron->borrowernumber, $item->itemnumber, $library->branchcode ),
+        { status => 'OK' },
+        'Patron of specified category can place 1 hold on specified itemtype if library rule for all types and categories set to 2'
+    );
+
+    $schema->storage->txn_rollback;
+
+};