From 0686bcbf934441f6ac3f0ebcf2ef2aaa4b208b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= Date: Wed, 2 Sep 2015 09:36:11 +0000 Subject: [PATCH] Bug 14711: We do no longer need bibitems in AddReserve Removes the unnecessary bibitems parameter from all the calls to AddReserve function and also from the function itself. Test plan: 1. Try some (if not all) of the pages' functionalities where the code was changed. 2. Make sure they work like before and there is no errors in the koha-error log. 3. If you didn't try all the pages make sure the code changes in the other pages are exactly like they were in the pages you tested. 4. Run all the test and look if they pass --- C4/ILSDI/Services.pm | 8 +++--- C4/Reserves.pm | 4 +-- C4/SIP/ILS/Transaction/Hold.pm | 2 +- opac/opac-reserve.pl | 3 +-- reserve/placerequest.pl | 20 +++------------ serials/routing-preview.pl | 3 +-- t/db_dependent/Circulation.t | 11 ++++---- t/db_dependent/Holds.t | 8 +----- t/db_dependent/Holds/LocalHoldsPriority.t | 1 - t/db_dependent/Holds/RevertWaitingStatus.t | 1 - t/db_dependent/Reserves.t | 40 ++++++++++++++---------------- t/db_dependent/Reserves/GetReserveFee.t | 1 - 12 files changed, 36 insertions(+), 66 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index e69c070..3e32c91 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -624,10 +624,10 @@ sub HoldTitle { # Add the reserve # $branch, $borrowernumber, $biblionumber, - # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, + # $constraint, $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, $borrowernumber, $biblionumber, $priority, undef, undef, undef, $title, undef, undef ); # Hashref building my $out; @@ -701,10 +701,10 @@ sub HoldItem { # Add the reserve # $branch, $borrowernumber, $biblionumber, - # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, + # $constraint, $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 ); + AddReserve( $branch, $borrowernumber, $biblionumber, $priority, undef, undef, undef, $title, $itemnumber, undef ); # Hashref building my $out; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 6120bbc..2b45ea7 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -145,14 +145,14 @@ BEGIN { =head2 AddReserve - AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found) + AddReserve($branch,$borrowernumber,$biblionumber,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found) =cut sub AddReserve { my ( $branch, $borrowernumber, $biblionumber, - $bibitems, $priority, $resdate, $expdate, $notes, + $priority, $resdate, $expdate, $notes, $title, $checkitem, $found ) = @_; my $dbh = C4::Context->dbh; diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm index fa2e5ba..08c018d 100644 --- a/C4/SIP/ILS/Transaction/Hold.pm +++ b/C4/SIP/ILS/Transaction/Hold.pm @@ -63,7 +63,7 @@ sub do_hold { return $self; } my $bibno = $bib->{biblionumber}; - AddReserve( $branch, $borrower->{borrowernumber}, $bibno, GetBiblioItemByBiblioNumber($bibno) ); + AddReserve( $branch, $borrower->{borrowernumber}, $bibno ); # unfortunately no meaningful return value $self->ok(1); diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 4dff9a2..684f287 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -285,8 +285,7 @@ if ( $query->param('place_reserve') ) { if ($canreserve) { AddReserve( $branch, $borrowernumber, - $biblioNum, - [$biblioNum], $rank, + $biblioNum, $rank, $startdate, $expiration_date, $notes, $biblioData->{title}, $itemNum, $found diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 6cbaa7b..d894b46 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -37,7 +37,6 @@ my $input = CGI->new(); checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); -my @bibitems=$input->param('biblioitem'); my @reqbib=$input->param('reqbib'); my $biblionumber=$input->param('biblionumber'); my $borrowernumber=$input->param('borrowernumber'); @@ -80,19 +79,6 @@ if (defined $checkitem && $checkitem ne ''){ if ($type eq 'str8' && $borrower){ foreach my $biblionumber (keys %bibinfos) { - my $count=@bibitems; - @bibitems=sort @bibitems; - my $i2=1; - my @realbi; - $realbi[0]=$bibitems[0]; - for (my $i=1;$i<$count;$i++) { - my $i3=$i2-1; - if ($realbi[$i3] ne $bibitems[$i]) { - $realbi[$i2]=$bibitems[$i]; - $i2++; - } - } - my $const; if (defined $checkitem && $checkitem ne ''){ my $item = GetItem($checkitem); @@ -105,11 +91,11 @@ if ($type eq 'str8' && $borrower){ if ($multi_hold) { my $bibinfo = $bibinfos{$biblionumber}; - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,[$biblionumber], - $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,$bibinfo->{rank}, + $startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found); } else { # place a request on 1st available - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); } } diff --git a/serials/routing-preview.pl b/serials/routing-preview.pl index 13bca6c..74b3424 100755 --- a/serials/routing-preview.pl +++ b/serials/routing-preview.pl @@ -64,7 +64,6 @@ my ($template, $loggedinuser, $cookie); if($ok){ # get biblio information.... my $biblio = $subs->{'biblionumber'}; - my ($count2,@bibitems) = GetBiblioItemByBiblioNumber($biblio); my @itemresults = GetItemsInfo( $subs->{biblionumber} ); my $branch = $itemresults[0]->{'holdingbranch'}; my $branchname = GetBranchName($branch); @@ -94,7 +93,7 @@ if($ok){ branchcode => $branch }); } else { - AddReserve($branch,$routing->{borrowernumber},$biblio,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title); + AddReserve($branch,$routing->{borrowernumber},$biblio,$routing->{ranking}, undef, undef, $notes,$title); } } } diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 591c7e4..8da7261 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -262,7 +262,6 @@ C4::Context->dbh->do("DELETE FROM accountlines"); my $renewing_borrower = GetMember( borrowernumber => $renewing_borrowernumber ); - my $bibitems = ''; my $priority = '1'; my $resdate = undef; my $expdate = undef; @@ -288,8 +287,8 @@ C4::Context->dbh->do("DELETE FROM accountlines"); # Biblio-level hold, renewal test AddReserve( $branch, $reserving_borrowernumber, $biblionumber, - $bibitems, $priority, $resdate, $expdate, $notes, - $title, $checkitem, $found + $priority, $resdate, $expdate, $notes, $title, + $checkitem, $found ); # Testing of feature to allow the renewal of reserved items if other items on the record can fill all needed holds @@ -353,8 +352,8 @@ C4::Context->dbh->do("DELETE FROM accountlines"); # Item-level hold, renewal test AddReserve( $branch, $reserving_borrowernumber, $biblionumber, - $bibitems, $priority, $resdate, $expdate, $notes, - $title, $itemnumber, $found + $priority, $resdate, $expdate, $notes, $title, + $itemnumber, $found ); ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $itemnumber, 1); @@ -654,7 +653,7 @@ C4::Context->dbh->do("DELETE FROM accountlines"); AddReserve( 'MPL', $borrowernumber2, $biblionumber, - '', 1, undef, undef, '', + 1, undef, undef, '', undef, undef, undef ); diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 82d6f62..4b6c76d 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -69,7 +69,6 @@ foreach my $borrowernumber ( @borrowernumbers ) { $branch, $borrowernumber, $biblionumber, - my $bibitems = q{}, my $priority, my $resdate, my $expdate, @@ -148,7 +147,6 @@ AddReserve( $branch, $borrowernumber, $biblionumber, - my $bibitems = q{}, my $priority, my $resdate, my $expdate, @@ -253,7 +251,6 @@ AddReserve( $branch, $borrowernumbers[0], $bibnum, - '', 1, ); @@ -269,7 +266,6 @@ AddReserve( $branch, $borrowernumbers[1], $bibnum, - '', 2, ); my $reserveid2 = C4::Reserves::GetReserveId( @@ -289,7 +285,6 @@ AddReserve( $branch, $borrowernumbers[0], $bibnum, - '', 2, ); my $reserveid3 = C4::Reserves::GetReserveId( @@ -321,7 +316,6 @@ AddReserve( $branch, $borrowernumbers[0], $bibnum, - '', 1, ); ok( @@ -438,7 +432,7 @@ $dbh->do( is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ), 'OK', 'Patron can reserve item with hold limit of 1, no holds placed' ); -my $res_id = AddReserve( $branch, $borrowernumbers[0], $bibnum, '', 1, ); +my $res_id = AddReserve( $branch, $borrowernumbers[0], $bibnum, 1, ); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber ), 'tooManyReserves', 'Patron cannot reserve item with hold limit of 1, 1 bib level hold placed' ); diff --git a/t/db_dependent/Holds/LocalHoldsPriority.t b/t/db_dependent/Holds/LocalHoldsPriority.t index 8bbcdb4..d17e8cb 100755 --- a/t/db_dependent/Holds/LocalHoldsPriority.t +++ b/t/db_dependent/Holds/LocalHoldsPriority.t @@ -58,7 +58,6 @@ foreach my $borrowernumber (@borrowernumbers) { $branchcodes[$i], $borrowernumber, $biblionumber, - my $bibitems = q{}, my $priority = $i, my $resdate, my $expdate, diff --git a/t/db_dependent/Holds/RevertWaitingStatus.t b/t/db_dependent/Holds/RevertWaitingStatus.t index a7648fd..eff3f97 100755 --- a/t/db_dependent/Holds/RevertWaitingStatus.t +++ b/t/db_dependent/Holds/RevertWaitingStatus.t @@ -67,7 +67,6 @@ foreach my $borrowernumber (@borrowernumbers) { $branch, $borrowernumber, $biblionumber, - my $bibitems = q{}, my $priority, my $resdate, my $expdate, diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 536d2c3..de96f22 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -100,7 +100,6 @@ my $borrower = GetMember( borrowernumber => $borrowernumber ); my $biblionumber = $bibnum; my $barcode = $testbarcode; -my $bibitems = ''; my $priority = '1'; my $resdate = undef; my $expdate = undef; @@ -112,8 +111,8 @@ my @branches = GetBranchesLoop(); my $branch = $branches[0][0]{value}; AddReserve($branch, $borrowernumber, $biblionumber, - $bibitems, $priority, $resdate, $expdate, $notes, - $title, $checkitem, $found); + $priority, $resdate, $expdate, $notes, $title, + $checkitem, $found); my ($status, $reserve, $all_reserves) = CheckReserves($itemnumber, $barcode); @@ -223,13 +222,13 @@ my ($itemnum_cpl, $itemnum_fpl); # (bug 11947) $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); AddReserve('RPL', $requesters{'RPL'}, $bibnum2, - $bibitems, 1, $resdate, $expdate, $notes, + 1, $resdate, $expdate, $notes, $title, $checkitem, $found); AddReserve('FPL', $requesters{'FPL'}, $bibnum2, - $bibitems, 2, $resdate, $expdate, $notes, + 2, $resdate, $expdate, $notes, $title, $checkitem, $found); AddReserve('CPL', $requesters{'CPL'}, $bibnum2, - $bibitems, 3, $resdate, $expdate, $notes, + 3, $resdate, $expdate, $notes, $title, $checkitem, $found); ModReserveAffect($itemnum_cpl, $requesters{'RPL'}, 0); @@ -248,13 +247,13 @@ is( $reserves[0]->borrowernumber(), $requesters{'RPL'}, 'GetWaiting got the rese $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum2)); AddReserve('RPL', $requesters{'RPL'}, $bibnum2, - $bibitems, 1, $resdate, $expdate, $notes, + 1, $resdate, $expdate, $notes, $title, $checkitem, $found); AddReserve('FPL', $requesters{'FPL'}, $bibnum2, - $bibitems, 2, $resdate, $expdate, $notes, + 2, $resdate, $expdate, $notes, $title, $checkitem, $found); AddReserve('CPL', $requesters{'CPL'}, $bibnum2, - $bibitems, 3, $resdate, $expdate, $notes, + 3, $resdate, $expdate, $notes, $title, $checkitem, $found); # Ensure that the item's home library controls hold policy lookup @@ -304,7 +303,7 @@ $resdate= undef; #defaults to today in AddReserve $expdate= undef; #no expdate $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, 1, $resdate, $expdate, $notes, + 1, $resdate, $expdate, $notes, $title, $checkitem, $found); ($status)=CheckReserves($itemnumber,undef,undef); is( $status, 'Reserved', 'CheckReserves returns reserve without lookahead'); @@ -319,7 +318,7 @@ $resdate->add_duration(DateTime::Duration->new(days => 4)); $resdate=output_pref($resdate); $expdate= undef; #no expdate AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, 1, $resdate, $expdate, $notes, + 1, $resdate, $expdate, $notes, $title, $checkitem, $found); ($status)=CheckReserves($itemnumber,undef,undef); is( $status, '', 'CheckReserves returns no future reserve without lookahead'); @@ -376,14 +375,14 @@ $resdate= dt_from_string(); $resdate->add_duration(DateTime::Duration->new(days => 2)); $resdate=output_pref($resdate); AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, 1, $resdate, $expdate, $notes, + 1, $resdate, $expdate, $notes, $title, $checkitem, $found); my @results= GetReservesFromItemnumber($itemnumber); is(defined $results[3]?1:0, 0, 'GetReservesFromItemnumber does not return a future next available hold'); # 9788b: GetReservesFromItemnumber does not return future item level hold $dbh->do("DELETE FROM reserves WHERE biblionumber=?",undef,($bibnum)); AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, 1, $resdate, $expdate, $notes, + 1, $resdate, $expdate, $notes, $title, $itemnumber, $found); #item level hold @results= GetReservesFromItemnumber($itemnumber); is(defined $results[3]?1:0, 0, 'GetReservesFromItemnumber does not return a future item level hold'); @@ -398,7 +397,7 @@ my $p = C4::Reserves::CalculatePriority($bibnum2); is($p, 4, 'CalculatePriority should now return priority 4'); $resdate=undef; AddReserve('CPL', $requesters{'CPL'}, $bibnum2, - $bibitems, $p, $resdate, $expdate, $notes, + $p, $resdate, $expdate, $notes, $title, $checkitem, $found); $p = C4::Reserves::CalculatePriority($bibnum2); is($p, 5, 'CalculatePriority should now return priority 5'); @@ -408,7 +407,7 @@ $p = C4::Reserves::CalculatePriority($bibnum); is($p, 1, 'CalculatePriority should now return priority 1'); #add a new reserve and confirm it to waiting AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, $p, $resdate, $expdate, $notes, + $p, $resdate, $expdate, $notes, $title, $itemnumber, $found); $p = C4::Reserves::CalculatePriority($bibnum); is($p, 2, 'CalculatePriority should now return priority 2'); @@ -417,7 +416,7 @@ $p = C4::Reserves::CalculatePriority($bibnum); is($p, 1, 'CalculatePriority should now return priority 1'); #add another biblio hold, no resdate AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, $p, $resdate, $expdate, $notes, + $p, $resdate, $expdate, $notes, $title, $checkitem, $found); $p = C4::Reserves::CalculatePriority($bibnum); is($p, 2, 'CalculatePriority should now return priority 2'); @@ -426,7 +425,7 @@ C4::Context->set_preference('AllowHoldDateInFuture', 1); $resdate= dt_from_string(); $resdate->add_duration(DateTime::Duration->new(days => 1)); AddReserve('CPL', $requesters{'CPL'}, $bibnum, - $bibitems, $p, output_pref($resdate), $expdate, $notes, + $p, output_pref($resdate), $expdate, $notes, $title, $checkitem, $found); $p = C4::Reserves::CalculatePriority($bibnum); is($p, 2, 'CalculatePriority should now still return priority 2'); @@ -438,7 +437,7 @@ 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('CPL', $requesters{'CPL'}, $item_bibnum, - $bibitems, 1, undef, $expdate, $notes, + 1, undef, $expdate, $notes, $title, $checkitem, ''); my (undef, $canres, undef) = CheckReserves($itemnumber); @@ -468,7 +467,7 @@ is($cancancel, 0, 'Reserve in transfer status cant be canceled'); $dbh->do('DELETE FROM reserves', undef, ($bibnum)); AddReserve('CPL', $requesters{'CPL'}, $item_bibnum, - $bibitems, 1, undef, $expdate, $notes, + 1, undef, $expdate, $notes, $title, $checkitem, ''); (undef, $canres, undef) = CheckReserves($itemnumber); @@ -545,7 +544,6 @@ my $bz14464_reserve = AddReserve( 'CPL', $borrowernumber, $bibnum, - undef, '1', undef, undef, @@ -569,7 +567,6 @@ $bz14464_reserve = AddReserve( 'CPL', $borrowernumber, $bibnum, - undef, '1', undef, undef, @@ -591,7 +588,6 @@ $bz14464_reserve = AddReserve( 'CPL', $borrowernumber, $bibnum, - undef, '1', undef, undef, diff --git a/t/db_dependent/Reserves/GetReserveFee.t b/t/db_dependent/Reserves/GetReserveFee.t index ddc25ec..f4a4011 100755 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ b/t/db_dependent/Reserves/GetReserveFee.t @@ -115,7 +115,6 @@ sub addreserve { 'CPL', $_[0], $biblio->{biblionumber}, - undef, '1', undef, undef, -- 1.9.1