Bug 14711: Change prototype for AddReserve - pass a hashref
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Thu, 16 Aug 2018 17:41:37 +0000 (14:41 -0300)
committerMartin Renvoize <martin.renvoize@ptfs-europe.com>
Tue, 11 Feb 2020 14:32:47 +0000 (14:32 +0000)
The number of parameters of AddReserve makes it hard to read and
maintain.
This patch replace it with a hashref, which will make the calls more
readable.

Moreover the bibitems has been removed as it was not used by the
subroutine.

Test plan:
- Make sure the tests pass
- Read the diff and search for typos
- Place a hold on few items

Note for QA: reservation_date and expiration_date do not match the DB column's names,
should we?

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

28 files changed:
C4/ILSDI/Services.pm
C4/Reserves.pm
C4/SIP/ILS/Transaction/Hold.pm
Koha/Club/Hold.pm
Koha/REST/V1/Holds.pm
opac/opac-reserve.pl
reserve/placerequest.pl
serials/routing-preview.pl
t/db_dependent/Circulation.t
t/db_dependent/Circulation/issue.t
t/db_dependent/Holds.t
t/db_dependent/Holds/HoldFulfillmentPolicy.t
t/db_dependent/Holds/HoldItemtypeLimit.t
t/db_dependent/Holds/LocalHoldsPriority.t
t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Items/GetItemsForInventory.t
t/db_dependent/Koha/Acquisition/Order.t
t/db_dependent/Koha/Biblios.t
t/db_dependent/Koha/Holds.t
t/db_dependent/Koha/Patrons.t
t/db_dependent/Letters/TemplateToolkit.t
t/db_dependent/Reserves.t
t/db_dependent/Reserves/GetReserveFee.t
t/db_dependent/Reserves/MultiplePerRecord.t
t/db_dependent/SIP/Transaction.t
t/db_dependent/UsageStats.t
t/db_dependent/api/v1/holds.t

index 64caa4a..1ed424e 100644 (file)
@@ -733,7 +733,15 @@ sub HoldTitle {
     #    $constraint, $bibitems,  $priority, $resdate, $expdate, $notes,
     #    $title,      $checkitem, $found
     my $priority= C4::Reserves::CalculatePriority( $biblionumber );
-    AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, undef, undef );
+    AddReserve(
+        {
+            branch         => $branch,
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblionumber,
+            priority       => $priority,
+            title          => $title,
+        }
+    );
 
     # Hashref building
     my $out;
@@ -808,11 +816,17 @@ sub HoldItem {
     return { code => $canitembereserved } unless $canitembereserved eq 'OK';
 
     # Add the reserve
-    #    $branch,    $borrowernumber, $biblionumber,
-    #    $constraint, $bibitems,  $priority, $resdate, $expdate, $notes,
-    #    $title,      $checkitem, $found
-    my $priority= C4::Reserves::CalculatePriority( $biblionumber );
-    AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, $itemnumber, undef );
+    my $priority = C4::Reserves::CalculatePriority($biblionumber);
+    AddReserve(
+        {
+            branch         => $branch,
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblionumber,
+            priority       => $priority,
+            title          => $title,
+            itemnumber     => $itemnumber,
+        }
+    );
 
     # Hashref building
     my $out;
index 8e6dca8..2b87fc2 100644 (file)
@@ -141,7 +141,21 @@ BEGIN {
 
 =head2 AddReserve
 
-    AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found)
+    AddReserve(
+        {
+            branch           => $branchcode,
+            borrowernumber   => $borrowernumber,
+            biblionumber     => $biblionumber,
+            priority         => $priority,
+            reservation_date => $reservation_date,
+            expiration_date  => $expiration_date,
+            notes            => $notes,
+            title            => $title,
+            itemnumber       => $itemnumber,
+            found            => $found,
+            itemtype         => $itemtype,
+        }
+    );
 
 Adds reserve and generates HOLDPLACED message.
 
@@ -157,11 +171,18 @@ The following tables are available witin the HOLDPLACED message:
 =cut
 
 sub AddReserve {
-    my (
-        $branch,   $borrowernumber, $biblionumber, $bibitems,
-        $priority, $resdate,        $expdate,      $notes,
-        $title,    $checkitem,      $found,        $itemtype
-    ) = @_;
+    my ($params)       = @_;
+    my $branch         = $params->{branchcode};
+    my $borrowernumber = $params->{borrowernumber};
+    my $biblionumber   = $params->{biblionumber};
+    my $priority       = $params->{priority};
+    my $resdate        = $params->{reservation_date};
+    my $expdate        = $params->{expiration_date};
+    my $notes          = $params->{notes};
+    my $title          = $params->{title};
+    my $checkitem      = $params->{itemnumber};
+    my $found          = $params->{found};
+    my $itemtype       = $params->{itemtype};
 
     $resdate = output_pref( { str => dt_from_string( $resdate ), dateonly => 1, dateformat => 'iso' })
         or output_pref({ dt => dt_from_string, dateonly => 1, dateformat => 'iso' });
index daefee5..ecf8416 100644 (file)
@@ -66,7 +66,13 @@ sub do_hold {
         return $self;
     }
 
-    AddReserve( $branch, $patron->borrowernumber, $item->biblionumber );
+    AddReserve(
+        {
+            branch         => $branch,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblionumber
+        }
+    );
 
     # unfortunately no meaningful return value
     $self->ok(1);
index b8b78fb..c278e9f 100644 (file)
@@ -91,18 +91,18 @@ sub add {
         my $priority = C4::Reserves::CalculatePriority($params->{biblio_id});
 
         my $hold_id = C4::Reserves::AddReserve(
-            $params->{pickup_library_id},
-            $patron_id,
-            $params->{biblio_id},
-            undef,    # $bibitems param is unused
-            $priority,
-            undef,    # hold date, we don't allow it currently
-            $params->{expiration_date},
-            $params->{notes},
-            $biblio->title,
-            $params->{item_id},
-            undef,    # TODO: Why not?
-            $params->{item_type}
+            {
+                branchcode      => $params->{pickup_library_id},
+                borrowernumber  => $patron_id,
+                biblionumber    => $params->{biblio_id},
+                priority        => $priority,
+                expiration_date => $params->{expiration_date},
+                notes           => $params->{notes},
+                title           => $biblio->title,
+                itemnumber      => $params->{item_id},
+                found           => undef,                       # TODO: Why not?
+                itemtype        => $params->{item_type},
+            }
         );
         if ($hold_id) {
             Koha::Club::Hold::PatronHold->new({
index 7d181b9..0fd1b3f 100644 (file)
@@ -161,18 +161,19 @@ sub add {
         }
 
         my $hold_id = C4::Reserves::AddReserve(
-            $pickup_library_id,
-            $patron_id,
-            $biblio_id,
-            undef,    # $bibitems param is unused
-            $priority,
-            $hold_date,
-            $expiration_date,
-            $notes,
-            $biblio->title,
-            $item_id,
-            undef,    # TODO: Why not?
-            $item_type
+            {
+                branchcode       => $pickup_library_id,
+                borrowernumber   => $patron_id,
+                biblionumber     => $biblio_id,
+                priority         => $priority,
+                reservation_date => $hold_date,
+                expiration_date  => $expiration_date,
+                notes            => $notes,
+                title            => $biblio->title,
+                itemnumber       => $item_id,
+                found            => undef,                # TODO: Why not?
+                itemtype         => $item_type,
+            }
         );
 
         unless ($hold_id) {
index b781e32..96834cb 100755 (executable)
@@ -309,10 +309,19 @@ if ( $query->param('place_reserve') ) {
         # Here we actually do the reserveration. Stage 3.
         if ($canreserve) {
             my $reserve_id = AddReserve(
-                $branch,          $borrowernumber, $biblioNum,
-                [$biblioNum],     $rank,           $startdate,
-                $expiration_date, $notes,          $biblioData->{title},
-                $itemNum,         $found,          $itemtype,
+                {
+                    branchcode       => $branch,
+                    borrowernumber   => $borrowernumber,
+                    biblionumber     => $biblioNum,
+                    priority         => $rank,
+                    reservation_date => $startdate,
+                    expiration_date  => $expiration_date,
+                    notes            => $notes,
+                    title            => $biblioData->{title},
+                    itemnumber       => $itemNum,
+                    found            => $found,
+                    itemtype         => $itemtype,
+                }
             );
             $failed_holds++ unless $reserve_id;
             ++$reserve_cnt;
index 89c591d..264ba96 100755 (executable)
@@ -100,24 +100,62 @@ if ( $type eq 'str8' && $borrower ) {
             my $can_item_be_reserved = CanItemBeReserved($borrower->{'borrowernumber'}, $item->itemnumber, $branch)->{status};
 
             if ( $can_item_be_reserved eq 'OK' || ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) ) {
-                AddReserve( $branch, $borrower->{'borrowernumber'},
-                    $biblionumber, \@realbi, $rank[0], $startdate, $expirationdate, $notes, $title,
-                    $checkitem, $found, $itemtype );
+                AddReserve(
+                    {
+                        branchcode       => $branch,
+                        borrowernumber   => $borrower->{'borrowernumber'},
+                        biblionumber     => $biblionumber,
+                        priority         => $rank[0],
+                        reservation_date => $startdate,
+                        expiration_date  => $expirationdate,
+                        notes            => $notes,
+                        title            => $title,
+                        itemnumber       => $checkitem,
+                        found            => $found,
+                        itemtype         => $itemtype,
+                    }
+                );
+
             }
 
         } elsif ($multi_hold) {
             my $bibinfo = $bibinfos{$biblionumber};
             if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) {
-                AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,[$biblionumber],
-                           $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found);
+                AddReserve(
+                    {
+                        branchcode       => $branch,
+                        borrowernumber   => $borrower->{'borrowernumber'},
+                        biblionumber     => $biblionumber,
+                        priority         => $bibinfo->{rank},
+                        reservation_date => $startdate,
+                        expiration_date  => $expirationdate,
+                        notes            => $notes,
+                        title            => $bibinfo->{title},
+                        itemnumber       => $checkitem,
+                        found            => $found,
+                        itemtype         => $itemtype,
+                    }
+                );
             }
         } else {
             # place a request on 1st available
             for ( my $i = 0 ; $i < $holds_to_place_count ; $i++ ) {
                 if ( $can_override || CanBookBeReserved($borrower->{'borrowernumber'}, $biblionumber)->{status} eq 'OK' ) {
-                    AddReserve( $branch, $borrower->{'borrowernumber'},
-                        $biblionumber, \@realbi, $rank[0], $startdate, $expirationdate, $notes, $title,
-                        $checkitem, $found, $itemtype );
+                    AddReserve(
+                        {
+                            branchcode       => $branch,
+                            borrowernumber   => $borrower->{'borrowernumber'},
+                            biblionumber     => $biblionumber,
+                            priority         => $rank[0],
+                            reservation_date => $startdate,
+                            expiration_date  => $expirationdate,
+                            notes            => $notes,
+                            title            => $title,
+                            itemnumber       => $checkitem,
+                            found            => $found,
+                            itemtype         => $itemtype,
+                        }
+                    );
                 }
             }
         }
index 7ddbfe5..2bf7eb3 100755 (executable)
@@ -94,7 +94,16 @@ if($ok){
                     branchcode     => $branch
                 });
             } else {
-                AddReserve($branch,$routing->{borrowernumber},$biblionumber,undef,$routing->{ranking}, undef, undef, $notes,$title);
+                AddReserve(
+                    {
+                        branchcode     => $branch,
+                        borrowernumber => $routing->{borrowernumber},
+                        biblionumber   => $biblionumber,
+                        priority       => $routing->{ranking},
+                        notes          => $notes,
+                        title          => $title,
+                    }
+                );
         }
     }
        }
index 4b17bf5..e87abff 100755 (executable)
@@ -385,9 +385,17 @@ subtest "CanBookBeRenewed tests" => sub {
 
     # Biblio-level hold, renewal test
     AddReserve(
-        $branch, $reserving_borrowernumber, $biblio->biblionumber,
-        $bibitems,  $priority, $resdate, $expdate, $notes,
-        'a title', $checkitem, $found
+        {
+            branchcode       => $branch,
+            borrowernumber   => $reserving_borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            priority         => $priority,
+            reservation_date => $resdate,
+            expiration_date  => $expdate,
+            notes            => $notes,
+            itemnumber       => $checkitem,
+            found            => $found,
+        }
     );
 
     # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds
@@ -458,9 +466,17 @@ subtest "CanBookBeRenewed tests" => sub {
 
     # Item-level hold, renewal test
     AddReserve(
-        $branch, $reserving_borrowernumber, $biblio->biblionumber,
-        $bibitems,  $priority, $resdate, $expdate, $notes,
-        'a title', $item_1->itemnumber, $found
+        {
+            branchcode       => $branch,
+            borrowernumber   => $reserving_borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            priority         => $priority,
+            reservation_date => $resdate,
+            expiration_date  => $expdate,
+            notes            => $notes,
+            itemnumber       => $item_1->itemnumber,
+            found            => $found,
+        }
     );
 
     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1);
@@ -1378,9 +1394,12 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
     is( $renewokay, 1, 'Bug 14337 - Verify the borrower can renew with no hold on the record' );
 
     AddReserve(
-        $library2->{branchcode}, $borrowernumber2, $biblio->biblionumber,
-        '',  1, undef, undef, '',
-        undef, undef, undef
+        {
+            branchcode     => $library2->{branchcode},
+            borrowernumber => $borrowernumber2,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1,
+        }
     );
 
     Koha::CirculationRules->set_rules(
@@ -1799,9 +1818,17 @@ subtest 'MultipleReserves' => sub {
     );
     my $reserving_borrowernumber1 = Koha::Patron->new(\%reserving_borrower_data1)->store->borrowernumber;
     AddReserve(
-        $branch, $reserving_borrowernumber1, $biblio->biblionumber,
-        $bibitems,  $priority, $resdate, $expdate, $notes,
-        'a title', $checkitem, $found
+        {
+            branchcode       => $branch,
+            borrowernumber   => $reserving_borrowernumber1,
+            biblionumber     => $biblio->biblionumber,
+            priority         => $priority,
+            reservation_date => $resdate,
+            expiration_date  => $expdate,
+            notes            => $notes,
+            itemnumber       => $checkitem,
+            found            => $found,
+        }
     );
 
     my %reserving_borrower_data2 = (
@@ -1812,9 +1839,17 @@ subtest 'MultipleReserves' => sub {
     );
     my $reserving_borrowernumber2 = Koha::Patron->new(\%reserving_borrower_data2)->store->borrowernumber;
     AddReserve(
-        $branch, $reserving_borrowernumber2, $biblio->biblionumber,
-        $bibitems,  $priority, $resdate, $expdate, $notes,
-        'a title', $checkitem, $found
+        {
+            branchcode       => $branch,
+            borrowernumber   => $reserving_borrowernumber2,
+            biblionumber     => $biblio->biblionumber,
+            priority         => $priority,
+            reservation_date => $resdate,
+            expiration_date  => $expdate,
+            notes            => $notes,
+            itemnumber       => $checkitem,
+            found            => $found,
+        }
     );
 
     {
@@ -2861,8 +2896,13 @@ subtest 'Set waiting flag' => sub {
 
     set_userenv( $library_2 );
     my $reserve_id = AddReserve(
-        $library_2->{branchcode}, $patron_2->{borrowernumber}, $item->{biblionumber},
-        '', 1, undef, undef, '', undef, $item->{itemnumber},
+        {
+            branchcode     => $library_2->{branchcode},
+            borrowernumber => $patron_2->{borrowernumber},
+            biblionumber   => $item->{biblionumber},
+            priority       => 1,
+            itemnumber     => $item->{itemnumber},
+        }
     );
 
     set_userenv( $library_1 );
@@ -2899,7 +2939,13 @@ subtest 'Cancel transfers on lost items' => sub {
 
     set_userenv( $library_2 );
     my $reserve_id = AddReserve(
-        $library_2->{branchcode}, $patron_2->{borrowernumber}, $item->biblionumber, '', 1, undef, undef, '', undef, $item->itemnumber,
+        {
+            branchcode     => $library_2->{branchcode},
+            borrowernumber => $patron_2->{borrowernumber},
+            biblionumber   => $item->biblionumber,
+            priority       => 1,
+            itemnumber     => $item->itemnumber,
+        }
     );
 
     #Return book and add transfer
index 3280071..c2fcbaa 100644 (file)
@@ -433,8 +433,16 @@ ok( $item2->permanent_location eq '' , q{UpdateItemLocationOnCheckin does not up
 
 
 # Bug 14640 - Cancel the hold on checking out if asked
-my $reserve_id = AddReserve($branchcode_1, $borrower_id1, $biblionumber,
-    undef,  1, undef, undef, "a note", "a title", undef, '');
+my $reserve_id = AddReserve(
+    {
+        branchcode     => $branchcode_1,
+        borrowernumber => $borrower_id1,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+        notes          => "a note",
+        title          => "a title",
+    }
+);
 ok( $reserve_id, 'The reserve should have been inserted' );
 AddIssue( $borrower_2, $barcode_1, dt_from_string, 'cancel' );
 my $hold = Koha::Holds->find( $reserve_id );
index d5a2f8b..a84eba8 100755 (executable)
@@ -76,17 +76,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,
+        }
     );
 }
 
@@ -191,19 +187,14 @@ 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;
@@ -297,11 +288,35 @@ 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);
+    my $reserveid1 = AddReserve(
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumbers[0],
+            biblionumber   => $biblio->biblionumber,
+            priority       => 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);
+    my $reserveid2 = AddReserve(
+        {
+            branchcode     => $branch_1,
+            borrowernumber => $borrowernumbers[1],
+            biblionumber   => $biblio->biblionumber,
+            priority       => 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);
+    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" );
@@ -335,11 +350,12 @@ ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for d
 $biblio = $builder->build_sample_biblio({ itemtype => 'CANNOT' });
 ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $branch_1, holdingbranch => $branch_1, itype => 'CANNOT' } , $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',
@@ -442,7 +458,14 @@ 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' );
@@ -474,9 +497,17 @@ subtest 'Test max_holds per library/patron category' => sub {
             }
         }
     );
-    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();
@@ -635,7 +666,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 ),
@@ -662,7 +700,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 },
@@ -718,14 +763,36 @@ subtest 'CanItemBeReserved / holds_per_day tests' => sub {
         { 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 },
index 189c3c6..a835be3 100755 (executable)
@@ -85,19 +85,40 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-my $reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+my $reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 my ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Hold where pickup branch matches home branch targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is($status, q{}, "Hold where pickup ne home, pickup eq home not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
@@ -116,19 +137,40 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, q{}, "Hold where pickup eq home, pickup ne holding not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, q{}, "Hold where pickup ne home, pickup ne holding not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
@@ -147,19 +189,40 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Hold where pickup eq home, pickup ne holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Hold where pickup ne home, pickup eq holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Hold where pickup ne home, pickup ne holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
@@ -176,7 +239,14 @@ my $limit = Koha::Item::Transfer::Limit->new(
         itemtype   => $item->effective_itemtype,
     }
 )->store();
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1
+    }
+);
 ($status) = CheckReserves($itemnumber);
 is( $status, '',  "No hold where branch transfer is not allowed" );
 Koha::Holds->find($reserve_id)->cancel;
index 751047d..13958e6 100644 (file)
@@ -99,19 +99,43 @@ Koha::CirculationRules->set_rules(
 );
 
 # Itemtypes match
-my $reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
+my $reserve_id = AddReserve(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+        itemtype       => $right_itemtype,
+    }
+);
 my ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Hold where itemtype matches item's itemtype targed" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Itemtypes don't match
-$reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+        itemtype       => $wrong_itemtype,
+    }
+);
+
 ( $status ) = CheckReserves($itemnumber);
 is($status, q{}, "Hold where itemtype does not match item's itemtype not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # No itemtype set
-$reserve_id = AddReserve( $branchcode, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 ( $status ) = CheckReserves($itemnumber);
 is( $status, 'Reserved', "Item targeted with no hold itemtype set" );
 Koha::Holds->find( $reserve_id )->cancel;
index 0ecb9d1..28849d2 100755 (executable)
@@ -69,17 +69,12 @@ foreach ( 1 .. $borrowers_count ) {
 my $i = 1;
 foreach my $borrowernumber (@borrowernumbers) {
     AddReserve(
-        $branchcodes[$i],
-        $borrowernumber,
-        $biblio->biblionumber,
-        my $bibitems   = q{},
-        my $priority = $i,
-        my $resdate,
-        my $expdate,
-        my $notes = q{},
-        'a title',
-        my $checkitem,
-        my $found,
+        {
+            branchcode     => $branchcodes[$i],
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => $i,
+        }
     );
 
     $i++;
index 30228df..41a22e9 100755 (executable)
@@ -73,17 +73,11 @@ foreach my $i ( 1 .. $borrowers_count ) {
 # Create five item level holds
 foreach my $borrowernumber (@borrowernumbers) {
     AddReserve(
-        $branchcode,
-        $borrowernumber,
-        $biblio->biblionumber,
-        my $bibitems   = q{},
-        my $priority,
-        my $resdate,
-        my $expdate,
-        my $notes = q{},
-        'a title',
-        my $checkitem,
-        my $found,
+        {
+            branchcode     => $branchcode,
+            borrowernumber => $borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+        }
     );
 }
 
index f70e61f..ef5d893 100755 (executable)
@@ -114,7 +114,14 @@ $dbh->do("DELETE FROM reserves");
 my $bibitems = undef;
 my $priority = 1;
 # Make a reserve
-AddReserve ( $borrower_branchcode, $borrowernumber, $biblionumber, $bibitems,  $priority );
+AddReserve(
+    {
+        branchcode     => $borrower_branchcode,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => $priority,
+    }
+);
 #                           $resdate, $expdate, $notes, $title, $checkitem, $found
 $dbh->do("UPDATE reserves SET reservedate = DATE_SUB( reservedate, INTERVAL 1 DAY )");
 
@@ -533,7 +540,14 @@ $dbh->do( "UPDATE systempreferences SET value = ? WHERE variable = 'StaticHoldsQ
     undef, join( ',', $library_B, $library_A, $library_C ) );
 $dbh->do( "UPDATE systempreferences SET value = 0 WHERE variable = 'RandomizeHoldsQueueWeight'" );
 
-my $reserve_id = AddReserve ( $library_C, $borrowernumber, $biblionumber, '', 1 );
+my $reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
 is( @$holds_queue, 1, "Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from le");
@@ -577,7 +591,15 @@ $dbh->do("
     VALUES ($biblionumber, $biblioitemnumber, '$library_A', '$library_A', 0, 0, 0, 0, NULL, '$itemtype')
 ");
 
-$reserve_id = AddReserve ( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
 is( @$holds_queue, 0, "Bug 15062 - Holds queue with Transport Cost Matrix will transfer item even if transfers disabled");
@@ -628,21 +650,46 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Hold where pickup branch matches home branch targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 0, "Hold where pickup ne home, pickup eq home not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" );
@@ -662,21 +709,45 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 0, "Hold where pickup eq home, pickup ne holding not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 0, "Hold where pickup ne home, pickup ne holding not targeted" );
@@ -696,21 +767,45 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Hold where pickup eq home, pickup ne holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_B, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_B,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Hold where pickup ne home, pickup eq holding targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_C, $borrowernumber, $biblionumber, '', 1 );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_C,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Hold where pickup ne home, pickup ne holding targeted" );
@@ -763,21 +858,47 @@ Koha::CirculationRules->set_rules(
 );
 
 # Home branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $wrong_itemtype );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+        itemtype       => $wrong_itemtype,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 0, "Item with incorrect itemtype not targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Holding branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, $right_itemtype );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+        itemtype       => $right_itemtype,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Item with matching itemtype is targeted" );
 Koha::Holds->find( $reserve_id )->cancel;
 
 # Neither branch matches pickup branch
-$reserve_id = AddReserve( $library_A, $borrowernumber, $biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
+$reserve_id = AddReserve(
+    {
+        branchcode     => $library_A,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
+
 C4::HoldsQueue::CreateQueue();
 $holds_queue = $dbh->selectall_arrayref( "SELECT * FROM tmp_holdsqueue", { Slice => {} } );
 is( @$holds_queue, 1, "Item targeted when hold itemtype is not set" );
@@ -819,12 +940,22 @@ subtest "Test Local Holds Priority - Bib level" => sub {
         }
     );
 
-    my $reserve_id =
-      AddReserve( $branch2->branchcode, $other_patron->borrowernumber,
-        $biblio->biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
-    my $reserve_id2 =
-      AddReserve( $item->homebranch, $local_patron->borrowernumber,
-        $biblio->biblionumber, '', 2, undef, undef, undef, undef, undef, undef, undef );
+    my $reserve_id = AddReserve(
+        {
+            branchcode     => $branch2->branchcode,
+            borrowernumber => $other_patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1,
+        }
+    );
+    my $reserve_id2 = AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $local_patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 2,
+        }
+    );
 
     C4::HoldsQueue::CreateQueue();
 
@@ -871,12 +1002,24 @@ subtest "Test Local Holds Priority - Item level" => sub {
         }
     );
 
-    my $reserve_id =
-      AddReserve( $branch2->branchcode, $other_patron->borrowernumber,
-        $biblio->biblionumber, '', 1, undef, undef, undef, undef, $item->id, undef, undef );
-    my $reserve_id2 =
-      AddReserve( $item->homebranch, $local_patron->borrowernumber,
-        $biblio->biblionumber, '', 2, undef, undef, undef, undef, $item->id, undef, undef );
+    my $reserve_id = AddReserve(
+        {
+            branchcode     => $branch2->branchcode,
+            borrowernumber => $other_patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1,
+            itemnumber     => $item->id,
+        }
+    );
+    my $reserve_id2 = AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $local_patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 2,
+            itemnumber     => $item->id,
+        }
+    );
 
     C4::HoldsQueue::CreateQueue();
 
@@ -924,12 +1067,23 @@ subtest "Test Local Holds Priority - Item level hold over Record level hold (Bug
         }
     );
 
-    my $reserve_id =
-      AddReserve( $branch2->branchcode, $other_patron->borrowernumber,
-        $biblio->biblionumber, '', 1, undef, undef, undef, undef, undef, undef, undef );
-    my $reserve_id2 =
-      AddReserve( $item->homebranch, $local_patron->borrowernumber,
-        $biblio->biblionumber, '', 2, undef, undef, undef, undef, $item->id, undef, undef );
+    my $reserve_id = AddReserve(
+        {
+            branchcode     => $branch2->branchcode,
+            borrowernumber => $other_patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1,
+        }
+    );
+    my $reserve_id2 = AddReserve(
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $local_patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 2,
+            itemnumber     => $item->id,
+        }
+    );
 
     C4::HoldsQueue::CreateQueue();
 
@@ -987,9 +1141,14 @@ subtest "Test Local Holds Priority - Ensure no duplicate requests in holds queue
         }
     );
 
-    $reserve_id =
-      AddReserve( $item1->homebranch, $patron->borrowernumber, $biblio->id, '',
-        1, undef, undef, undef, undef, undef, undef, undef );
+    $reserve_id = AddReserve(
+        {
+            branchcode     => $item1->homebranch,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio->id,
+            priority       => 1
+        }
+    );
 
     C4::HoldsQueue::CreateQueue();
 
index c54d8ae..2099351 100755 (executable)
@@ -100,15 +100,34 @@ subtest 'Skip items with waiting holds' => sub {
     is( $first_items_count + 2, $second_items_count, 'Two items added, count makes sense' );
 
     # Add 2 waiting holds
-    C4::Reserves::AddReserve( $library->branchcode, $patron_1->borrowernumber,
-        $item_1->biblionumber, '', 1, undef, undef, '', "title for fee",
-        $item_1->itemnumber, 'W' );
-    C4::Reserves::AddReserve( $library->branchcode, $patron_1->borrowernumber,
-        $item_2->biblionumber, '', 1, undef, undef, '', "title for fee",
-        $item_2->itemnumber, undef );
-    C4::Reserves::AddReserve( $library->branchcode, $patron_2->borrowernumber,
-        $item_2->biblionumber, '', 2, undef, undef, '', "title for fee",
-        $item_2->itemnumber, undef );
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron_1->borrowernumber,
+            biblionumber   => $item_1->biblionumber,
+            priority       => 1,
+            itemnumber     => $item_1->itemnumber,
+            found          => 'W'
+        }
+    );
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron_1->borrowernumber,
+            biblionumber   => $item_2->biblionumber,
+            priority       => 1,
+            itemnumber     => $item_2->itemnumber,
+        }
+    );
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron_2->borrowernumber,
+            biblionumber   => $item_2->biblionumber,
+            priority       => 2,
+            itemnumber     => $item_2->itemnumber,
+        }
+    );
 
     my ( $new_items, $new_items_count ) = GetItemsForInventory( { ignore_waiting_holds => 1 } );
     is( $new_items_count, $first_items_count + 1, 'Item on hold skipped, count makes sense' );
index 5d2a1bd..f62f3eb 100644 (file)
@@ -296,26 +296,32 @@ subtest 'current_item_level_holds() tests' => sub {
     my $item_3 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } );
 
     C4::Reserves::AddReserve(
-        $patron->branchcode,   $patron->borrowernumber,
-        $biblio->biblionumber, undef,
-        undef, dt_from_string->add( days => -2 ),
-        undef, undef,
-        undef, $item_1->itemnumber
+        {
+            branchcode       => $patron->branchcode,
+            borrowernumber   => $patron->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            reservation_date => dt_from_string->add( days => -2 ),
+            itemnumber       => $item_1->itemnumber,
+        }
     );
     C4::Reserves::AddReserve(
-        $patron->branchcode,   $patron->borrowernumber,
-        $biblio->biblionumber, undef,
-        undef, dt_from_string->add( days => -2 ),
-        undef, undef,
-        undef, $item_2->itemnumber
+        {
+            branchcode       => $patron->branchcode,
+            borrowernumber   => $patron->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            reservation_date => dt_from_string->add( days => -2 ),
+            itemnumber       => $item_2->itemnumber,
+        }
     );
     # Add a hold in the future
     C4::Reserves::AddReserve(
-        $patron->branchcode,   $patron->borrowernumber,
-        $biblio->biblionumber, undef,
-        undef, dt_from_string->add( days => 2 ),
-        undef, undef,
-        undef, $item_3->itemnumber
+        {
+            branchcode       => $patron->branchcode,
+            borrowernumber   => $patron->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            reservation_date => dt_from_string->add( days => 2 ),
+            itemnumber       => $item_3->itemnumber,
+        }
     );
 
     # Add an order with no biblionumber
index 2d5acbb..45ceb1a 100644 (file)
@@ -64,7 +64,13 @@ subtest 'store' => sub {
 
 subtest 'holds + current_holds' => sub {
     plan tests => 5;
-    C4::Reserves::AddReserve( $patron->branchcode, $patron->borrowernumber, $biblio->biblionumber );
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $patron->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+        }
+    );
     my $holds = $biblio->holds;
     is( ref($holds), 'Koha::Holds', '->holds should return a Koha::Holds object' );
     is( $holds->count, 1, '->holds should only return 1 hold' );
@@ -72,7 +78,14 @@ subtest 'holds + current_holds' => sub {
     $holds->delete;
 
     # Add a hold in the future
-    C4::Reserves::AddReserve( $patron->branchcode, $patron->borrowernumber, $biblio->biblionumber, undef, undef, dt_from_string->add( days => 2 ) );
+    C4::Reserves::AddReserve(
+        {
+            branchcode       => $patron->branchcode,
+            borrowernumber   => $patron->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            reservation_date => dt_from_string->add( days => 2 ),
+        }
+    );
     $holds = $biblio->holds;
     is( $holds->count, 1, '->holds should return future holds' );
     $holds = $biblio->current_holds;
index 783204a..c8b9679 100644 (file)
@@ -60,11 +60,14 @@ subtest 'cancel' => sub {
             }
         );
         my $reserve_id = C4::Reserves::AddReserve(
-            $library->branchcode, $patron->borrowernumber,
-            $item->biblionumber,  '',
-            $priority,            undef,
-            undef,                '',
-            "title for fee",      $item->itemnumber,
+            {
+                branchcode     => $library->branchcode,
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $item->biblionumber,
+                priority       => $priority,
+                title          => "title for fee",
+                itemnumber     => $item->itemnumber,
+            }
         );
         my $hold = Koha::Holds->find($reserve_id);
         push @patrons, $patron;
@@ -107,30 +110,31 @@ subtest 'cancel' => sub {
         my $patron = $builder->build_object({ class => 'Koha::Patrons', value => { categorycode => $patron_category->categorycode } });
         is( $patron->account->balance, 0, 'A new patron does not have any charges' );
 
-        my @hold_info = (
-            $library->branchcode, $patron->borrowernumber,
-            $item->biblionumber,  '',
-            1,                    undef,
-            undef,                '',
-            "title for fee",      $item->itemnumber,
-        );
+        my $hold_info = {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => 1,
+            title          => "title for fee",
+            itemnumber     => $item->itemnumber,
+        };
 
         # First, test cancelling a reserve when there's no charge configured.
         t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 0);
-        my $reserve_id = C4::Reserves::AddReserve( @hold_info );
+        my $reserve_id = C4::Reserves::AddReserve( $hold_info );
         Koha::Holds->find( $reserve_id )->cancel( { charge_cancel_fee => 1 } );
         is( $patron->account->balance, 0, 'ExpireReservesMaxPickUpDelayCharge=0 - The patron should not have been charged' );
 
         # Then, test cancelling a reserve when there's no charge desired.
         t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 42);
-        $reserve_id = C4::Reserves::AddReserve( @hold_info );
+        $reserve_id = C4::Reserves::AddReserve( $hold_info );
         Koha::Holds->find( $reserve_id )->cancel(); # charge_cancel_fee => 0
         is( $patron->account->balance, 0, 'ExpireReservesMaxPickUpDelayCharge=42, but charge_cancel_fee => 0, The patron should not have been charged' );
 
 
         # Finally, test cancelling a reserve when there's a charge desired and configured.
         t::lib::Mocks::mock_preference('ExpireReservesMaxPickUpDelayCharge', 42);
-        $reserve_id = C4::Reserves::AddReserve( @hold_info );
+        $reserve_id = C4::Reserves::AddReserve( $hold_info );
         Koha::Holds->find( $reserve_id )->cancel( { charge_cancel_fee => 1 } );
         is( int($patron->account->balance), 42, 'ExpireReservesMaxPickUpDelayCharge=42 and charge_cancel_fee => 1, The patron should have been charged!' );
     };
@@ -139,12 +143,15 @@ subtest 'cancel' => sub {
         plan tests => 1;
         my $patron = $builder->build_object({ class => 'Koha::Patrons' });
         my $reserve_id = C4::Reserves::AddReserve(
-            $library->branchcode, $patron->borrowernumber,
-            $item->biblionumber,  '',
-            1,                    undef,
-            undef,                '',
-            "title for fee",      $item->itemnumber,
-            'W',
+            {
+                branchcode     => $library->branchcode,
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $item->biblionumber,
+                priority       => 1,
+                title          => "title for fee",
+                itemnumber     => $item->itemnumber,
+                found          => 'W',
+            }
         );
         Koha::Holds->find( $reserve_id )->cancel;
         my $hold_old = Koha::Old::Holds->find( $reserve_id );
@@ -154,22 +161,23 @@ subtest 'cancel' => sub {
     subtest 'HoldsLog' => sub {
         plan tests => 2;
         my $patron = $builder->build_object({ class => 'Koha::Patrons' });
-        my @hold_info = (
-            $library->branchcode, $patron->borrowernumber,
-            $item->biblionumber,  '',
-            1,                    undef,
-            undef,                '',
-            "title for fee",      $item->itemnumber,
-        );
+        my $hold_info = {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => 1,
+            title          => "title for fee",
+            itemnumber     => $item->itemnumber,
+        };
 
         t::lib::Mocks::mock_preference('HoldsLog', 0);
-        my $reserve_id = C4::Reserves::AddReserve(@hold_info);
+        my $reserve_id = C4::Reserves::AddReserve($hold_info);
         Koha::Holds->find( $reserve_id )->cancel;
         my $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'HOLDS', action => 'CANCEL', object => $reserve_id } )->count;
         is( $number_of_logs, 0, 'Without HoldsLog, Koha::Hold->cancel should not have logged' );
 
         t::lib::Mocks::mock_preference('HoldsLog', 1);
-        $reserve_id = C4::Reserves::AddReserve(@hold_info);
+        $reserve_id = C4::Reserves::AddReserve($hold_info);
         Koha::Holds->find( $reserve_id )->cancel;
         $number_of_logs = $schema->resultset('ActionLog')->search( { module => 'HOLDS', action => 'CANCEL', object => $reserve_id } )->count;
         is( $number_of_logs, 1, 'With HoldsLog, Koha::Hold->cancel should have logged' );
@@ -189,16 +197,17 @@ subtest 'cancel' => sub {
                 value => { categorycode => $patron_category->categorycode }
             }
         );
-        my @hold_info = (
-            $library->branchcode, $patron->borrowernumber,
-            $item->biblionumber,  '',
-            1,                    undef,
-            undef,                '',
-            "title for fee",      $item->itemnumber,
-        );
+        my $hold_info = {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => 1,
+            title          => "title for fee",
+            itemnumber     => $item->itemnumber,
+        };
 
         t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelayCharge',42 );
-        my $reserve_id = C4::Reserves::AddReserve(@hold_info);
+        my $reserve_id = C4::Reserves::AddReserve($hold_info);
         my $hold       = Koha::Holds->find($reserve_id);
 
         # Add a row with the same id to make the cancel fails
index f94bd2a..53ee33e 100644 (file)
@@ -919,11 +919,22 @@ subtest 'holds and old_holds' => sub {
         'Koha::Patron->holds should return a Koha::Holds objects' );
     is( $holds->count, 0, 'There should not be holds placed by this patron yet' );
 
-    C4::Reserves::AddReserve( $library->{branchcode},
-        $patron->borrowernumber, $biblionumber_1 );
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $library->{branchcode},
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblionumber_1
+        }
+    );
     # In the future
-    C4::Reserves::AddReserve( $library->{branchcode},
-        $patron->borrowernumber, $biblionumber_2, undef, undef, dt_from_string->add( days => 2 ) );
+    C4::Reserves::AddReserve(
+        {
+            branchcode      => $library->{branchcode},
+            borrowernumber  => $patron->borrowernumber,
+            biblionumber    => $biblionumber_2,
+            expiration_date => dt_from_string->add( days => 2 )
+        }
+    );
 
     $holds = $patron->holds;
     is( $holds->count, 2, 'There should be 2 holds placed by this patron' );
index fff90b1..80415d6 100644 (file)
@@ -524,8 +524,25 @@ You have [% count %] items due
 
         my $code = 'HOLD_SLIP';
 
-        C4::Reserves::AddReserve( $library->{branchcode}, $patron->{borrowernumber}, $biblio1->{biblionumber}, undef, undef, undef, undef, "a note", undef, $item1->{itemnumber}, 'W' );
-        C4::Reserves::AddReserve( $library->{branchcode}, $patron->{borrowernumber}, $biblio2->{biblionumber}, undef, undef, undef, undef, "another note", undef, $item2->{itemnumber} );
+        C4::Reserves::AddReserve(
+            {
+                branchcode     => $library->{branchcode},
+                borrowernumber => $patron->{borrowernumber},
+                biblionumber   => $biblio1->{biblionumber},
+                notes          => "a note",
+                itemnumber     => $item1->{itemnumber},
+                found          => 'W'
+            }
+        );
+        C4::Reserves::AddReserve(
+            {
+                branchcode     => $library->{branchcode},
+                borrowernumber => $patron->{borrowernumber},
+                biblionumber   => $biblio2->{biblionumber},
+                notes          => "another note",
+                itemnumber     => $item2->{itemnumber},
+            }
+        );
 
         my $template = <<EOF;
 <h5>Date: <<today>></h5>
index d6eb517..ef47e26 100755 (executable)
@@ -116,19 +116,16 @@ my $borrower = $patron->unblessed;
 my $biblionumber   = $bibnum;
 my $barcode        = $testbarcode;
 
-my $bibitems       = '';
-my $priority       = '1';
-my $resdate        = undef;
-my $expdate        = undef;
-my $notes          = '';
-my $checkitem      = undef;
-my $found          = undef;
-
 my $branchcode = Koha::Libraries->search->next->branchcode;
 
-AddReserve($branchcode,    $borrowernumber, $biblionumber,
-        $bibitems,  $priority, $resdate, $expdate, $notes,
-        'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $biblionumber,
+        priority       => 1,
+    }
+);
 
 my ($status, $reserve, $all_reserves) = CheckReserves($itemnumber, $barcode);
 
@@ -251,15 +248,30 @@ my ($itemnum_cpl, $itemnum_fpl);
 # Ensure that priorities are numbered correcly when a hold is moved to waiting
 # (bug 11947)
 $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2));
-AddReserve($branch_3,  $requesters{$branch_3}, $bibnum2,
-           $bibitems,  1, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
-AddReserve($branch_2,  $requesters{$branch_2}, $bibnum2,
-           $bibitems,  2, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum2,
-           $bibitems,  3, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branch_3,
+        borrowernumber => $requesters{$branch_3},
+        biblionumber   => $bibnum2,
+        priority       => 1,
+    }
+);
+AddReserve(
+    {
+        branchcode     => $branch_2,
+        borrowernumber => $requesters{$branch_2},
+        biblionumber   => $bibnum2,
+        priority       => 2,
+    }
+);
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{$branch_1},
+        biblionumber   => $bibnum2,
+        priority       => 3,
+    }
+);
 ModReserveAffect($itemnum_cpl, $requesters{$branch_3}, 0);
 
 # Now it should have different priorities.
@@ -275,15 +287,31 @@ is( $reserves[0]->borrowernumber(), $requesters{$branch_3}, 'GetWaiting got the
 
 
 $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2));
-AddReserve($branch_3,  $requesters{$branch_3}, $bibnum2,
-           $bibitems,  1, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
-AddReserve($branch_2,  $requesters{$branch_2}, $bibnum2,
-           $bibitems,  2, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum2,
-           $bibitems,  3, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branch_3,
+        borrowernumber => $requesters{$branch_3},
+        biblionumber   => $bibnum2,
+        priority       => 1,
+    }
+);
+AddReserve(
+    {
+        branchcode     => $branch_2,
+        borrowernumber => $requesters{$branch_2},
+        biblionumber   => $bibnum2,
+        priority       => 2,
+    }
+);
+
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{$branch_1},
+        biblionumber   => $bibnum2,
+        priority       => 3,
+    }
+);
 
 # Ensure that the item's home library controls hold policy lookup
 t::lib::Mocks::mock_preference( 'ReservesControlBranch', 'ItemHomeLibrary' );
@@ -317,12 +345,15 @@ my $reserve_id = $holds->next->reserve_id;
 # Tests for bug 9761 (ConfirmFutureHolds): new CheckReserves lookahead parameter, and corresponding change in AddReturn
 # Note that CheckReserve uses its lookahead parameter and does not check ConfirmFutureHolds pref (it should be passed if needed like AddReturn does)
 # Test 9761a: Add a reserve without date, CheckReserve should return it
-$resdate= undef; #defaults to today in AddReserve
-$expdate= undef; #no expdate
 $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum,
-           $bibitems,  1, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{$branch_1},
+        biblionumber   => $bibnum,
+        priority       => 1,
+    }
+);
 ($status)=CheckReserves($itemnumber,undef,undef);
 is( $status, 'Reserved', 'CheckReserves returns reserve without lookahead');
 ($status)=CheckReserves($itemnumber,undef,7);
@@ -331,13 +362,18 @@ is( $status, 'Reserved', 'CheckReserves also returns reserve with lookahead');
 # Test 9761b: Add a reserve with future date, CheckReserve should not return it
 $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
 t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1);
-$resdate= dt_from_string();
+my $resdate= dt_from_string();
 $resdate->add_duration(DateTime::Duration->new(days => 4));
 $resdate=output_pref($resdate);
-$expdate= undef; #no expdate
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum,
-           $bibitems,  1, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode       => $branch_1,
+        borrowernumber   => $requesters{$branch_1},
+        biblionumber     => $bibnum,
+        priority         => 1,
+        reservation_date => $resdate,
+    }
+);
 ($status)=CheckReserves($itemnumber,undef,undef);
 is( $status, '', 'CheckReserves returns no future reserve without lookahead');
 
@@ -392,9 +428,16 @@ t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1);
 $resdate= dt_from_string();
 $resdate->add_duration(DateTime::Duration->new(days => 2));
 $resdate=output_pref($resdate);
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum,
-           $bibitems,  1, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode       => $branch_1,
+        borrowernumber   => $requesters{$branch_1},
+        biblionumber     => $bibnum,
+        priority         => 1,
+        reservation_date => $resdate,
+    }
+);
+
 my $item = Koha::Items->find( $itemnumber );
 $holds = $item->current_holds;
 my $dtf = Koha::Database->new->schema->storage->datetime_parser;
@@ -402,9 +445,16 @@ my $future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( d
 is( $future_holds->count, 0, 'current_holds does not return a future next available hold');
 # 9788b: current_holds does not return future item level hold
 $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum,
-           $bibitems,  1, $resdate, $expdate, $notes,
-           'a title',      $itemnumber, $found); #item level hold
+AddReserve(
+    {
+        branchcode       => $branch_1,
+        borrowernumber   => $requesters{$branch_1},
+        biblionumber     => $bibnum,
+        priority         => 1,
+        reservation_date => $resdate,
+        itemnumber       => $itemnumber,
+    }
+); #item level hold
 $future_holds = $holds->search({ reservedate => { '>' => $dtf->format_date( dt_from_string ) } } );
 is( $future_holds->count, 0, 'current_holds does not return a future item level hold' );
 # 9788c: current_holds returns future wait (confirmed future hold)
@@ -417,10 +467,14 @@ $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
 # Tests for CalculatePriority (bug 8918)
 my $p = C4::Reserves::CalculatePriority($bibnum2);
 is($p, 4, 'CalculatePriority should now return priority 4');
-$resdate=undef;
-AddReserve($branch_1,  $requesters{'CPL2'}, $bibnum2,
-           $bibitems,  $p, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{'CPL2'},
+        biblionumber   => $bibnum2,
+        priority       => $p,
+    }
+);
 $p = C4::Reserves::CalculatePriority($bibnum2);
 is($p, 5, 'CalculatePriority should now return priority 5');
 #some tests on bibnum
@@ -428,27 +482,44 @@ $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum));
 $p = C4::Reserves::CalculatePriority($bibnum);
 is($p, 1, 'CalculatePriority should now return priority 1');
 #add a new reserve and confirm it to waiting
-AddReserve($branch_1,  $requesters{$branch_1}, $bibnum,
-           $bibitems,  $p, $resdate, $expdate, $notes,
-           'a title',      $itemnumber, $found);
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{$branch_1},
+        biblionumber   => $bibnum,
+        priority       => $p,
+        itemnumber     => $itemnumber,
+    }
+);
 $p = C4::Reserves::CalculatePriority($bibnum);
 is($p, 2, 'CalculatePriority should now return priority 2');
 ModReserveAffect( $itemnumber,  $requesters{$branch_1} , 0);
 $p = C4::Reserves::CalculatePriority($bibnum);
 is($p, 1, 'CalculatePriority should now return priority 1');
 #add another biblio hold, no resdate
-AddReserve($branch_1,  $requesters{'CPL2'}, $bibnum,
-           $bibitems,  $p, $resdate, $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{'CPL2'},
+        biblionumber   => $bibnum,
+        priority       => $p,
+    }
+);
 $p = C4::Reserves::CalculatePriority($bibnum);
 is($p, 2, 'CalculatePriority should now return priority 2');
 #add another future hold
 t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1);
 $resdate= dt_from_string();
 $resdate->add_duration(DateTime::Duration->new(days => 1));
-AddReserve($branch_1,  $requesters{'CPL3'}, $bibnum,
-           $bibitems,  $p, output_pref($resdate), $expdate, $notes,
-           'a title',      $checkitem, $found);
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{'CPL2'},
+        biblionumber   => $bibnum,
+        priority       => $p,
+        reservation_date => output_pref($resdate),
+    }
+);
 $p = C4::Reserves::CalculatePriority($bibnum);
 is($p, 2, 'CalculatePriority should now still return priority 2');
 #calc priority with future resdate
@@ -458,9 +529,14 @@ is($p, 3, 'CalculatePriority should now return priority 3');
 
 # Tests for cancel reserves by users from OPAC.
 $dbh->do('DELETE FROM reserves', undef, ($bibnum));
-AddReserve($branch_1,  $requesters{$branch_1}, $item_bibnum,
-           $bibitems,  1, undef, $expdate, $notes,
-           'a title',      $checkitem, '');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{$branch_1},
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+    }
+);
 my (undef, $canres, undef) = CheckReserves($itemnumber);
 
 is( CanReserveBeCanceledFromOpac(), undef,
@@ -488,9 +564,14 @@ $cancancel = CanReserveBeCanceledFromOpac($canres->{reserve_id}, $requesters{$br
 is($cancancel, 0, 'Reserve in transfer status cant be canceled');
 
 $dbh->do('DELETE FROM reserves', undef, ($bibnum));
-AddReserve($branch_1,  $requesters{$branch_1}, $item_bibnum,
-           $bibitems,  1, undef, $expdate, $notes,
-           'a title',      $checkitem, '');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $requesters{$branch_1},
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+    }
+);
 (undef, $canres, undef) = CheckReserves($itemnumber);
 
 ModReserveAffect($itemnumber, $requesters{$branch_1}, 0);
@@ -568,14 +649,27 @@ Koha::CirculationRules->set_rules(
 $dbh->do('DELETE FROM reserves', undef, ($bibnum));
 t::lib::Mocks::mock_preference('ConfirmFutureHolds', 0);
 t::lib::Mocks::mock_preference('AllowHoldDateInFuture', 1);
-AddReserve($branch_1,  $borrowernumber, $item_bibnum,
-    $bibitems,  1, undef, $expdate, $notes, 'a title', $checkitem, '');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+    }
+);
 MoveReserve( $itemnumber, $borrowernumber );
 ($status)=CheckReserves( $itemnumber );
 is( $status, '', 'MoveReserve filled hold');
 #   hold from A waiting, today, no fut holds: MoveReserve should fill it
-AddReserve($branch_1,  $borrowernumber, $item_bibnum,
-   $bibitems,  1, undef, $expdate, $notes, 'a title', $checkitem, 'W');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+        found          => 'W',
+    }
+);
 MoveReserve( $itemnumber, $borrowernumber );
 ($status)=CheckReserves( $itemnumber );
 is( $status, '', 'MoveReserve filled waiting hold');
@@ -583,22 +677,43 @@ is( $status, '', 'MoveReserve filled waiting hold');
 $resdate= dt_from_string();
 $resdate->add_duration(DateTime::Duration->new(days => 1));
 $resdate=output_pref($resdate);
-AddReserve($branch_1,  $borrowernumber, $item_bibnum,
-    $bibitems,  1, $resdate, $expdate, $notes, 'a title', $checkitem, '');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+        reservation_date => $resdate,
+    }
+);
 MoveReserve( $itemnumber, $borrowernumber );
 ($status)=CheckReserves( $itemnumber, undef, 1 );
 is( $status, 'Reserved', 'MoveReserve did not fill future hold');
 $dbh->do('DELETE FROM reserves', undef, ($bibnum));
 #   hold from A pos 1, tomorrow, fut holds=2: MoveReserve should fill it
 t::lib::Mocks::mock_preference('ConfirmFutureHolds', 2);
-AddReserve($branch_1,  $borrowernumber, $item_bibnum,
-    $bibitems,  1, $resdate, $expdate, $notes, 'a title', $checkitem, '');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+        reservation_date => $resdate,
+    }
+);
 MoveReserve( $itemnumber, $borrowernumber );
 ($status)=CheckReserves( $itemnumber, undef, 2 );
 is( $status, '', 'MoveReserve filled future hold now');
 #   hold from A waiting, tomorrow, fut holds=2: MoveReserve should fill it
-AddReserve($branch_1,  $borrowernumber, $item_bibnum,
-    $bibitems,  1, $resdate, $expdate, $notes, 'a title', $checkitem, 'W');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+        reservation_date => $resdate,
+    }
+);
 MoveReserve( $itemnumber, $borrowernumber );
 ($status)=CheckReserves( $itemnumber, undef, 2 );
 is( $status, '', 'MoveReserve filled future waiting hold now');
@@ -606,8 +721,15 @@ is( $status, '', 'MoveReserve filled future waiting hold now');
 $resdate= dt_from_string();
 $resdate->add_duration(DateTime::Duration->new(days => 3));
 $resdate=output_pref($resdate);
-AddReserve($branch_1,  $borrowernumber, $item_bibnum,
-    $bibitems,  1, $resdate, $expdate, $notes, 'a title', $checkitem, '');
+AddReserve(
+    {
+        branchcode     => $branch_1,
+        borrowernumber => $borrowernumber,
+        biblionumber   => $item_bibnum,
+        priority       => 1,
+        reservation_date => $resdate,
+    }
+);
 MoveReserve( $itemnumber, $borrowernumber );
 ($status)=CheckReserves( $itemnumber, undef, 3 );
 is( $status, 'Reserved', 'MoveReserve did not fill future hold of 3 days');
@@ -668,8 +790,12 @@ subtest '_koha_notify_reserve() tests' => sub {
         })->{borrowernumber};
 
     C4::Reserves::AddReserve(
-        $item->homebranch, $hold_borrower,
-        $item->biblionumber );
+        {
+            branchcode     => $item->homebranch,
+            borrowernumber => $hold_borrower,
+            biblionumber   => $item->biblionumber,
+        }
+    );
 
     ModReserveAffect($item->itemnumber, $hold_borrower, 0);
     my $sms_message_address = $schema->resultset('MessageQueue')->search({
@@ -817,10 +943,15 @@ subtest 'reserves.item_level_hold' => sub {
 
     subtest 'item level hold' => sub {
         plan tests => 2;
-        my $reserve_id =
-          AddReserve( $item->homebranch, $patron->borrowernumber,
-            $item->biblionumber, undef, 1, undef, undef, '', '',
-            $item->itemnumber );
+        my $reserve_id = AddReserve(
+            {
+                branchcode     => $item->homebranch,
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $item->biblionumber,
+                priority       => 1,
+                itemnumber     => $item->itemnumber,
+            }
+        );
 
         my $hold = Koha::Holds->find($reserve_id);
         is( $hold->item_level_hold, 1, 'item_level_hold should be set when AddReserve is called with a specific item' );
@@ -841,8 +972,14 @@ subtest 'reserves.item_level_hold' => sub {
 
     subtest 'biblio level hold' => sub {
         plan tests => 3;
-        my $reserve_id = AddReserve( $item->homebranch, $patron->borrowernumber,
-            $item->biblionumber, undef, 1 );
+        my $reserve_id = AddReserve(
+            {
+                branchcode     => $item->homebranch,
+                borrowernumber => $patron->borrowernumber,
+                biblionumber   => $item->biblionumber,
+                priority       => 1,
+            }
+        );
 
         my $hold = Koha::Holds->find($reserve_id);
         is( $hold->item_level_hold, 0, 'item_level_hold should not be set when AddReserve is called without a specific item' );
@@ -877,8 +1014,24 @@ subtest 'MoveReserve additional test' => sub {
     my $patron_2 = $builder->build_object({ class => "Koha::Patrons" });
 
     # Place a hold on the title for both patrons
-    my $reserve_1 = AddReserve( $item_1->homebranch, $patron_1->borrowernumber, $biblio->biblionumber, undef, 1 );
-    my $reserve_2 = AddReserve( $item_2->homebranch, $patron_2->borrowernumber, $biblio->biblionumber, undef, 1 );
+    my $reserve_1 = AddReserve(
+        {
+            branchcode     => $item_1->homebranch,
+            borrowernumber => $patron_1->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1,
+            itemnumber     => $item_1->itemnumber,
+        }
+    );
+    my $reserve_2 = AddReserve(
+        {
+            branchcode     => $item_2->homebranch,
+            borrowernumber => $patron_2->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+            priority       => 1,
+            itemnumber     => $item_1->itemnumber,
+        }
+    );
     is($patron_1->holds->next()->reserve_id, $reserve_1, "The 1st patron has a hold");
     is($patron_2->holds->next()->reserve_id, $reserve_2, "The 2nd patron has a hold");
 
@@ -906,12 +1059,16 @@ sub place_item_hold {
     my ($patron,$item,$library,$priority) = @_;
 
     my $hold_id = C4::Reserves::AddReserve(
-        $library->branchcode, $patron->borrowernumber,
-        $item->biblionumber,  '',
-        $priority,            undef,
-        undef,                '',
-        "title for fee",      $item->itemnumber,
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblionumber,
+            priority       => $priority,
+            title          => "title for fee",
+            itemnumber     => $item->itemnumber,
+        }
     );
+
     my $hold = Koha::Holds->find($hold_id);
     return $hold;
 }
index 80d428b..4645c49 100755 (executable)
@@ -205,17 +205,13 @@ sub acctlines { #calculate number of accountlines for a patron
 
 sub addreserve {
     return AddReserve(
-        $library->{branchcode},
-        $_[0],
-        $biblio->{biblionumber},
-        undef,
-        '1',
-        undef,
-        undef,
-        '',
-        $biblio->{title},
-        undef,
-        ''
+        {
+            branchcode     => $library->{branchcode},
+            borrowernumber => $_[0],
+            biblionumber   => $biblio->{biblionumber},
+            priority       => '1',
+            title          => $biblio->{title},
+        }
     );
 }
 
index bd4529f..6fb431d 100755 (executable)
@@ -285,12 +285,26 @@ Koha::CirculationRules->set_rules(
 
 my $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber});
 is( $can->{status}, 'OK', 'Hold can be placed with 0 holds' );
-my $hold_id = AddReserve( $library->{branchcode}, $patron->{borrowernumber}, $biblio->{biblionumber}, '', 1 );
+my $hold_id = AddReserve(
+    {
+        branchcode     => $library->{branchcode},
+        borrowernumber => $patron->{borrowernumber},
+        biblionumber   => $biblio->{biblionumber},
+        priority       => 1
+    }
+);
 ok( $hold_id, 'First hold was placed' );
 
 $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber});
 is( $can->{status}, 'OK', 'Hold can be placed with 1 hold' );
-$hold_id = AddReserve( $library->{branchcode}, $patron->{borrowernumber}, $biblio->{biblionumber}, '', 1 );
+$hold_id = AddReserve(
+    {
+        branchcode     => $library->{branchcode},
+        borrowernumber => $patron->{borrowernumber},
+        biblionumber   => $biblio->{biblionumber},
+        priority       => 1
+    }
+);
 ok( $hold_id, 'Second hold was placed' );
 
 $can = CanBookBeReserved($patron->{borrowernumber}, $biblio->{biblionumber});
index 3e8edbf..bc7af8b 100755 (executable)
@@ -81,8 +81,21 @@ subtest fill_holds_at_checkout => sub {
         }
     );
 
-    my $reserve1 = AddReserve($branch->{branchcode},$borrower->{borrowernumber},$biblio->{biblionumber});
-    my $reserve2 = AddReserve($branch->{branchcode},$borrower->{borrowernumber},$biblio->{biblionumber});
+    my $reserve1 = AddReserve(
+        {
+            branchcode     => $branch->{branchcode},
+            borrowernumber => $borrower->{borrowernumber},
+            biblionumber   => $biblio->{biblionumber}
+        }
+    );
+    my $reserve2 = AddReserve(
+        {
+            branchcode     => $branch->{branchcode},
+            borrowernumber => $borrower->{borrowernumber},
+            biblionumber   => $biblio->{biblionumber}
+        }
+    );
+
     my $bib = Koha::Biblios->find( $biblio->{biblionumber} );
     is( $bib->holds->count(), 2, "Bib has 2 holds");
 
@@ -174,10 +187,14 @@ subtest cancel_hold => sub {
         }
     );
 
-    my $reserve1 =
-      AddReserve( $library->branchcode, $patron->borrowernumber,
-        $item->biblio->biblionumber,
-        undef, undef, undef, undef, undef, undef, $item->itemnumber );
+    my $reserve1 = AddReserve(
+        {
+            branchcode     => $library->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $item->biblio->biblionumber,
+            itemnumber     => $item->itemnumber,
+        }
+    );
     is( $item->biblio->holds->count(), 1, "Hold was placed on bib");
     is( $item->holds->count(),1,"Hold was placed on specific item");
 
index 47a9cd3..7c9e8be 100644 (file)
@@ -301,7 +301,15 @@ sub construct_objects_needed {
     my $issue_id1 = $dbh->last_insert_id( undef, undef, 'old_issues', undef );
 
     # ---------- Add 1 old_reserves
-    AddReserve( $branchcode, $borrowernumber1, $biblionumber1, '', 1, undef, undef, '', 'Title', undef, undef );
+    AddReserve(
+        {
+            branchcode     => $branchcode,
+            borrowernumber => $borrowernumber1,
+            biblionumber   => $biblionumber1,
+            priority       => 1,
+            title          => 'Title',
+        }
+    );
     my $biblio = Koha::Biblios->find( $biblionumber1 );
     my $holds = $biblio->holds;
     $holds->next->cancel if $holds->count;
index 95c70e0..6033db7 100644 (file)
@@ -127,12 +127,26 @@ Koha::CirculationRules->set_rules(
     }
 );
 
-my $reserve_id = C4::Reserves::AddReserve($branchcode, $patron_1->borrowernumber,
-    $biblio_1->biblionumber, undef, 1, undef, undef, undef, '', $item_1->itemnumber);
+my $reserve_id = C4::Reserves::AddReserve(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $patron_1->borrowernumber,
+        biblionumber   => $biblio_1->biblionumber,
+        priority       => 1,
+        itemnumber     => $item_1->itemnumber,
+    }
+);
 
 # Add another reserve to be able to change first reserve's rank
-my $reserve_id2 = C4::Reserves::AddReserve($branchcode, $patron_2->borrowernumber,
-    $biblio_1->biblionumber, undef, 2, undef, undef, undef, '', $item_1->itemnumber);
+my $reserve_id2 = C4::Reserves::AddReserve(
+    {
+        branchcode     => $branchcode,
+        borrowernumber => $patron_2->borrowernumber,
+        biblionumber   => $biblio_1->biblionumber,
+        priority       => 2,
+        itemnumber     => $item_1->itemnumber,
+    }
+);
 
 my $suspended_until = DateTime->now->add(days => 10)->truncate( to => 'day' );
 my $expiration_date = DateTime->now->add(days => 10)->truncate( to => 'day' );
@@ -469,23 +483,32 @@ subtest 'PUT /holds/{hold_id}/priority tests' => sub {
 
     my $hold_1 = Koha::Holds->find(
         AddReserve(
-            $library->branchcode,  $patron_1->borrowernumber,
-            $biblio->biblionumber, undef,
-            1
+            {
+                branchcode     => $library->branchcode,
+                borrowernumber => $patron_1->borrowernumber,
+                biblionumber   => $biblio->biblionumber,
+                priority       => 1,
+            }
         )
     );
     my $hold_2 = Koha::Holds->find(
         AddReserve(
-            $library->branchcode,  $patron_2->borrowernumber,
-            $biblio->biblionumber, undef,
-            2
+            {
+                branchcode     => $library->branchcode,
+                borrowernumber => $patron_2->borrowernumber,
+                biblionumber   => $biblio->biblionumber,
+                priority       => 2,
+            }
         )
     );
     my $hold_3 = Koha::Holds->find(
         AddReserve(
-            $library->branchcode,  $patron_3->borrowernumber,
-            $biblio->biblionumber, undef,
-            3
+            {
+                branchcode     => $library->branchcode,
+                borrowernumber => $patron_3->borrowernumber,
+                biblionumber   => $biblio->biblionumber,
+                priority       => 3,
+            }
         )
     );