From a5fddf43f630322f1ee95b337ea70732a50e4ea7 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 30 Jun 2015 08:19:29 -0400 Subject: [PATCH] [SIGNED-OFF] Bug 9809 - Update AddReserve prototype to remove constraint parameter Test Plan: 1) Apply this patch set 2) prove t/db_dependent/Circulation.t 3) prove t/db_dependent/Holds.t 4) prove t/db_dependent/Holds/LocalHoldsPriority.t 5) prove t/db_dependent/Holds/RevertWaitingStatus.t 6) prove t/db_dependent/HoldsQueue.t 7) prove t/db_dependent/Reserves.t Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall --- C4/ILSDI/Services.pm | 4 +- C4/Reserves.pm | 74 ++++++++++++++-------------------------- C4/SIP/ILS/Transaction/Hold.pm | 60 ++++++++++++++++---------------- opac/opac-reserve.pl | 2 +- reserve/placerequest.pl | 14 +------ serials/routing-preview.pl | 3 +- 6 files changed, 62 insertions(+), 95 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 5d3f882..e69c070 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -627,7 +627,7 @@ sub HoldTitle { # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, # $title, $checkitem, $found my $priority= C4::Reserves::CalculatePriority( $biblionumber ); - AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, $priority, undef, undef, undef, $title, undef, undef ); + AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, undef, undef ); # Hashref building my $out; @@ -704,7 +704,7 @@ sub HoldItem { # $constraint, $bibitems, $priority, $resdate, $expdate, $notes, # $title, $checkitem, $found my $priority= C4::Reserves::CalculatePriority( $biblionumber ); - AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, $priority, undef, undef, undef, $title, $itemnumber, undef ); + AddReserve( $branch, $borrowernumber, $biblionumber, undef, $priority, undef, undef, undef, $title, $itemnumber, undef ); # Hashref building my $out; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 3013999..10579d9 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -146,21 +146,20 @@ BEGIN { =head2 AddReserve - AddReserve($branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found) + AddReserve($branch,$borrowernumber,$biblionumber,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found) =cut sub AddReserve { my ( $branch, $borrowernumber, $biblionumber, - $constraint, $bibitems, $priority, $resdate, $expdate, $notes, + $bibitems, $priority, $resdate, $expdate, $notes, $title, $checkitem, $found ) = @_; my $fee = - GetReserveFee($borrowernumber, $biblionumber, $constraint, + GetReserveFee($borrowernumber, $biblionumber, $bibitems ); my $dbh = C4::Context->dbh; - my $const = lc substr( $constraint, 0, 1 ); $resdate = format_date_in_iso( $resdate ) if ( $resdate ); $resdate = C4::Dates->today( 'iso' ) unless ( $resdate ); if ($expdate) { @@ -194,20 +193,18 @@ sub AddReserve { "Reserve Charge - $title", $fee ); } - #if ($const eq 'a'){ my $query = qq{ INSERT INTO reserves - (borrowernumber,biblionumber,reservedate,branchcode,constrainttype, + (borrowernumber,biblionumber,reservedate,branchcode, priority,reservenotes,itemnumber,found,waitingdate,expirationdate) VALUES (?,?,?,?,?, - ?,?,?,?,?,?) + ?,?,?,?,?) }; my $sth = $dbh->prepare($query); $sth->execute( - $borrowernumber, $biblionumber, $resdate, $branch, - $const, $priority, $notes, $checkitem, - $found, $waitingdate, $expdate + $borrowernumber, $biblionumber, $resdate, $branch, $priority, + $notes, $checkitem, $found, $waitingdate, $expdate ); my $reserve_id = $sth->{mysql_insertid}; @@ -240,9 +237,6 @@ sub AddReserve { } } - #} - ($const eq "o" || $const eq "e") or return; # FIXME: why not have a useful return value? - return $reserve_id; } @@ -302,7 +296,6 @@ sub GetReservesFromBiblionumber { biblionumber, borrowernumber, reservedate, - constrainttype, found, itemnumber, reservenotes, @@ -659,18 +652,17 @@ sub GetOtherReserves { =head2 GetReserveFee - $fee = GetReserveFee($borrowernumber,$biblionumber,$constraint,$biblionumber); + $fee = GetReserveFee($borrowernumber,$biblionumber,$biblionumber); Calculate the fee for a reserve =cut sub GetReserveFee { - my ($borrowernumber, $biblionumber, $constraint, $bibitems ) = @_; + my ($borrowernumber, $biblionumber, $bibitems ) = @_; #check for issues; my $dbh = C4::Context->dbh; - my $const = lc substr( $constraint, 0, 1 ); my $query = qq{ SELECT * FROM borrowers LEFT JOIN categories ON borrowers.categorycode = categories.categorycode @@ -693,30 +685,19 @@ sub GetReserveFee { ); $sth1->execute($biblionumber); while ( my $data1 = $sth1->fetchrow_hashref ) { - if ( $const eq "a" ) { - push @biblioitems, $data1; - } - else { - my $found = 0; - my $x = 0; - while ( $x < $cntitems ) { - if ( @$bibitems->{'biblioitemnumber'} == - $data->{'biblioitemnumber'} ) - { - $found = 1; - } - $x++; - } - if ( $const eq 'o' ) { - if ( $found == 1 ) { - push @biblioitems, $data1; - } - } - else { - if ( $found == 0 ) { - push @biblioitems, $data1; - } + my $found = 0; + my $x = 0; + while ( $x < $cntitems ) { + if ( @$bibitems->{'biblioitemnumber'} == + $data->{'biblioitemnumber'} ) + { + $found = 1; } + $x++; + } + + if ( $found == 0 ) { + push @biblioitems, $data1; } } my $cntitemsfound = @biblioitems; @@ -1792,7 +1773,7 @@ sub _FixPriority { # get whats left my $query = " - SELECT reserve_id, borrowernumber, reservedate, constrainttype + SELECT reserve_id, borrowernumber, reservedate FROM reserves WHERE biblionumber = ? AND ((found <> 'W' AND found <> 'T') OR found IS NULL) @@ -1854,13 +1835,10 @@ sub _FixPriority { @results = &_Findgroupreserve($biblioitemnumber, $biblionumber, $itemnumber, $lookahead, $ignore_borrowers); -Looks for an item-specific match first, then for a title-level match, returning the -first match found. If neither, then we look for a 3rd kind of match based on -reserve constraints. +Looks for a holds-queue based item-specific match first, then for a holds-queue title-level match, returning the +first match found. If neither, then we look for non-holds-queue based holds. Lookahead is the number of days to look in advance. -TODO: add more explanation about reserve constraints - C<&_Findgroupreserve> returns : C<@results> is an array of references-to-hash whose keys are mostly fields from the reserves table of the Koha database, plus @@ -2235,7 +2213,7 @@ sub MergeHolds { ); my $upd_sth = $dbh->prepare( "UPDATE reserves SET priority = ? WHERE biblionumber = ? AND borrowernumber = ? - AND reservedate = ? AND constrainttype = ? AND (itemnumber = ? or itemnumber is NULL) " + AND reservedate = ? AND (itemnumber = ? or itemnumber is NULL) " ); $sth->execute( $to_biblio, 'W', 'T' ); my $priority = 1; @@ -2243,7 +2221,7 @@ sub MergeHolds { $upd_sth->execute( $priority, $to_biblio, $reserve->{'borrowernumber'}, $reserve->{'reservedate'}, - $reserve->{'constrainttype'}, $reserve->{'itemnumber'} + $reserve->{'itemnumber'} ); $priority++; } diff --git a/C4/SIP/ILS/Transaction/Hold.pm b/C4/SIP/ILS/Transaction/Hold.pm index 0e00d3d..fa2e5ba 100644 --- a/C4/SIP/ILS/Transaction/Hold.pm +++ b/C4/SIP/ILS/Transaction/Hold.pm @@ -38,36 +38,36 @@ sub queue_position { } sub do_hold { - my $self = shift; - unless ($self->{patron}) { - $self->screen_msg('do_hold called with undefined patron'); - $self->ok(0); - return $self; - } - my $borrower = GetMember( 'cardnumber'=>$self->{patron}->id); - unless ($borrower) { - $self->screen_msg('No borrower matches cardnumber "' . $self->{patron}->id . '".'); - $self->ok(0); - return $self; - } - my $bib = GetBiblioFromItemNumber(undef, $self->{item}->id); - unless ($bib) { - $self->screen_msg('No biblio record matches barcode "' . $self->{item}->id . '".'); - $self->ok(0); - return $self; - } - my $branch = ($self->pickup_location || $self->{patron}->branchcode); - unless ($branch) { - $self->screen_msg('No branch specified (or found w/ patron).'); - $self->ok(0); - return $self; - } - my $bibno = $bib->{biblionumber}; - AddReserve($branch, $borrower->{borrowernumber}, - $bibno, 'a', GetBiblioItemByBiblioNumber($bibno)) ; - # unfortunately no meaningful return value - $self->ok(1); - return $self; + my $self = shift; + unless ( $self->{patron} ) { + $self->screen_msg('do_hold called with undefined patron'); + $self->ok(0); + return $self; + } + my $borrower = GetMember( 'cardnumber' => $self->{patron}->id ); + unless ($borrower) { + $self->screen_msg( 'No borrower matches cardnumber "' . $self->{patron}->id . '".' ); + $self->ok(0); + return $self; + } + my $bib = GetBiblioFromItemNumber( undef, $self->{item}->id ); + unless ($bib) { + $self->screen_msg( 'No biblio record matches barcode "' . $self->{item}->id . '".' ); + $self->ok(0); + return $self; + } + my $branch = ( $self->pickup_location || $self->{patron}->branchcode ); + unless ($branch) { + $self->screen_msg('No branch specified (or found w/ patron).'); + $self->ok(0); + return $self; + } + my $bibno = $bib->{biblionumber}; + AddReserve( $branch, $borrower->{borrowernumber}, $bibno, GetBiblioItemByBiblioNumber($bibno) ); + + # unfortunately no meaningful return value + $self->ok(1); + return $self; } sub drop_hold { diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 68abc0a..6e6e928 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -285,7 +285,7 @@ if ( $query->param('place_reserve') ) { if ($canreserve) { AddReserve( $branch, $borrowernumber, - $biblioNum, 'a', + $biblioNum, [$biblioNum], $rank, $startdate, $expiration_date, $notes, $biblioData->{title}, diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 1fc2436..dbfe34b 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -38,10 +38,6 @@ my $input = CGI->new(); checkauth($input, 0, { reserveforothers => 'place_holds' }, 'intranet'); my @bibitems=$input->param('biblioitem'); -# FIXME I think reqbib does not exist anymore, it's used in line 82, to AddReserve of contraint type 'o' -# I bet it's a 2.x feature, reserving a given biblioitem, that is useless in Koha 3.0 -# we can remove this line, the AddReserve of constrainttype 'o', -# and probably remove the reserveconstraint table as well, I never could fill anything in this table. my @reqbib=$input->param('reqbib'); my $biblionumber=$input->param('biblionumber'); my $borrowernumber=$input->param('borrowernumber'); @@ -109,18 +105,12 @@ if ($type eq 'str8' && $borrower){ if ($multi_hold) { my $bibinfo = $bibinfos{$biblionumber}; - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',[$biblionumber], + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,[$biblionumber], $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found); } else { if ($input->param('request') eq 'any'){ # place a request on 1st available - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); - } elsif ($reqbib[0] ne ''){ - # FIXME : elsif probably never reached, (see top of the script) - # place a request on a given item - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); - } else { - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem,$found); } } } diff --git a/serials/routing-preview.pl b/serials/routing-preview.pl index 1fd5814..13bca6c 100755 --- a/serials/routing-preview.pl +++ b/serials/routing-preview.pl @@ -79,7 +79,6 @@ if($ok){ $count--; } } - my $const = 'o'; my $notes; my $title = $subs->{'bibliotitle'}; for my $routing ( @routinglist ) { @@ -95,7 +94,7 @@ if($ok){ branchcode => $branch }); } else { - AddReserve($branch,$routing->{borrowernumber},$biblio,$const,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title); + AddReserve($branch,$routing->{borrowernumber},$biblio,\@bibitems,$routing->{ranking}, undef, undef, $notes,$title); } } } -- 1.7.2.5