Bug 14695 [QA Followup] - Make existing unit tests pass

* Removes tests no longer needed
* Updates rules to work with existing tests
* Fixes code issues revealed by unit tests

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jason M. Burds <JBurds@dubuque.lib.ia.us>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

C4/Reserves.pm

@@ -562,16 +562,16 @@ sub CanItemBeReserved {
     }
 
     my $circ_control_branch =
-      C4::Circulation::_GetCircControlBranch( $item, $borrower );
+      C4::Circulation::_GetCircControlBranch( $item->unblessed(), $borrower );
     my $branchitemrule =
-      C4::Circulation::GetBranchItemRule( $circ_control_branch, $item->{itype} );
+      C4::Circulation::GetBranchItemRule( $circ_control_branch, $item->itype );
 
     if ( $branchitemrule->{holdallowed} == 0 ) {
         return 'notReservable';
     }
 
     if (   $branchitemrule->{holdallowed} == 1
-        && $borrower->{branchcode} ne $item->{homebranch} )
+        && $borrower->{branchcode} ne $item->homebranch )
     {
         return 'cannotReserveFromOtherBranches';
     }

t/db_dependent/Holds.t

@@ -237,16 +237,16 @@ my ($foreign_item_bibnum, $foreign_item_bibitemnum, $foreign_itemnumber)
   = AddItem({ homebranch => $branch_2, holdingbranch => $branch_2 } , $foreign_bibnum);
 $dbh->do('DELETE FROM issuingrules');
 $dbh->do(
-    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
-      VALUES (?, ?, ?, ?)},
+    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
+      VALUES (?, ?, ?, ?, ?)},
     {},
-    '*', '*', '*', 25
+    '*', '*', '*', 25, 99
 );
 $dbh->do(
-    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
-      VALUES (?, ?, ?, ?)},
+    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
+      VALUES (?, ?, ?, ?, ?)},
     {},
-    '*', '*', 'CANNOT', 0
+    '*', '*', 'CANNOT', 0, 99
 );
 
 # make sure some basic sysprefs are set
@@ -337,7 +337,7 @@ is( $reserve3->{priority}, 1, "After ModReserve, the 3rd reserve becomes the fir
 
 ModItem({ damaged => 1 }, $item_bibnum, $itemnumber);
 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 1 );
-ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" );
+is( CanItemBeReserved( $borrowernumbers[0], $itemnumber), 'OK', "Patron can reserve damaged item with AllowHoldsOnDamagedItems enabled" );
 ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold can be trapped for damaged item with AllowHoldsOnDamagedItems enabled" );
 t::lib::Mocks::mock_preference( 'AllowHoldsOnDamagedItems', 0 );
 ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'damaged', "Patron cannot reserve damaged item with AllowHoldsOnDamagedItems disabled" );
@@ -353,8 +353,8 @@ AddReserve(
     '',
     1,
 );
-ok(
-    CanItemBeReserved( $borrowernumbers[0], $itemnumber) eq 'tooManyReserves',
+is(
+    CanItemBeReserved( $borrowernumbers[0], $itemnumber), 'tooManyReserves',
     "cannot request item if policy that matches on item-level item type forbids it"
 );
 ModItem({ itype => 'CAN' }, $item_bibnum, $itemnumber);
@@ -460,10 +460,10 @@ $dbh->do('DELETE FROM biblio');
     = AddItem( { homebranch => $branch_1, holdingbranch => $branch_1 }, $bibnum );
 
 $dbh->do(
-    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed)
-      VALUES (?, ?, ?, ?)},
+    q{INSERT INTO issuingrules (categorycode, branchcode, itemtype, reservesallowed, holds_per_record)
+      VALUES (?, ?, ?, ?, ?)},
     {},
-    '*', '*', 'ONLY1', 1
+    '*', '*', 'ONLY1', 1, 99
 );
 is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ),
     'OK', 'Patron can reserve item with hold limit of 1, no holds placed' );

t/db_dependent/Reserves.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 74;
+use Test::More tests => 72;
 use Test::MockModule;
 use Test::Warn;
 
@@ -450,14 +450,6 @@ $p = C4::Reserves::CalculatePriority($bibnum, $resdate);
 is($p, 3, 'CalculatePriority should now return priority 3');
 # End of tests for bug 8918
 
-# Test for bug 5144
-warning_is {
-    $reserve_id = AddReserve('CPL',  $requesters{'CPL3'}, $bibnum,
-           $bibitems,  $p, output_pref($resdate), $expdate, $notes,
-           $title,      $checkitem, $found)
-} "AddReserve: borrower $requesters{CPL3} already has a hold for biblionumber $bibnum";
-is( $reserve_id, undef, 'Attempt to add a second reserve on a given record for the same patron fails.' );
-
 # Tests for cancel reserves by users from OPAC.
 $dbh->do('DELETE FROM reserves', undef, ($bibnum));
 AddReserve('CPL',  $requesters{'CPL'}, $item_bibnum,