Bugzilla – Attachment 41759 Details for
Bug 9809
Get rid of reserveconstraints
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9809 - Update AddReserve prototype to remove constraint parameter
Bug-9809---Update-AddReserve-prototype-to-remove-c.patch (text/plain), 14.27 KB, created by
Marcel de Rooy
on 2015-08-21 09:00:11 UTC
(
hide
)
Description:
Bug 9809 - Update AddReserve prototype to remove constraint parameter
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2015-08-21 09:00:11 UTC
Size:
14.27 KB
patch
obsolete
>From 0ceb6c44cbf018c07893bb2f887cee570d0e1051 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 30 Jun 2015 08:19:29 -0400 >Subject: [PATCH] Bug 9809 - Update AddReserve prototype to remove constraint > parameter >Content-Type: text/plain; charset=utf-8 > >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 <m.de.rooy@rijksmuseum.nl> >--- > 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.10.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=41759">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9809
:
37638
|
40733
|
40734
|
40735
|
40736
|
40737
|
40738
|
41754
|
41755
|
41756
|
41757
|
41758
|
41759
|
41760
|
41761
|
41767
|
41768
|
41769
|
41770
|
41771
|
41772
|
41773
|
41774
|
41809
|
41810
|
41811
|
41812
|
41813
|
41814
|
41815
|
41816
|
41817
|
41845
|
41848
|
41849
|
41850
|
41851
|
41852
|
41892
|
41893
|
41894
|
41895
|
41896
|
41897
|
41898
|
41899
|
41900
|
41901
|
41902
|
41903