Bugzilla – Attachment 42198 Details for
Bug 14711
C4::Reserves::AddReserves should take a hashref in parameters
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14711: We do no longer need bibitems in AddReserve
Bug-14711-We-do-no-longer-need-bibitems-in-AddRese.patch (text/plain), 19.25 KB, created by
Joonas Kylmälä
on 2015-09-02 11:56:45 UTC
(
hide
)
Description:
Bug 14711: We do no longer need bibitems in AddReserve
Filename:
MIME Type:
Creator:
Joonas Kylmälä
Created:
2015-09-02 11:56:45 UTC
Size:
19.25 KB
patch
obsolete
>From 0686bcbf934441f6ac3f0ebcf2ef2aaa4b208b34 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <j.kylmala@gmail.com> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14711
:
42198
|
77940
|
98708
|
98710