Bugzilla – Attachment 22644 Details for
Bug 9788
Improvements for calling GetReservesFromItemnumber
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9788: Followup removing the alldates parameter
Bug-9788-Followup-removing-the-alldates-parameter.patch (text/plain), 6.26 KB, created by
Kyle M Hall (khall)
on 2013-10-31 13:31:35 UTC
(
hide
)
Description:
Bug 9788: Followup removing the alldates parameter
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2013-10-31 13:31:35 UTC
Size:
6.26 KB
patch
obsolete
>From 1fa0602c78f5f5adac318479f01e956368e558cb Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Thu, 26 Sep 2013 10:33:46 +0200 >Subject: [PATCH] Bug 9788: Followup removing the alldates parameter > >Before bug 9788 the alldates parameter of GetReservesFromItemnumber was >actually not used in the codebase. >The first patch of bug 9788 did change that and passed true by default. > >But a closer look revealed that we do not really need it. >The parameter is removed by this patch; the SQL statement is slightly >adjusted: if reservedate<=now or a waitingdate is filled for the >requested itemnumber, GetReservesFromItemnumber will return the reserve. >This includes so-called future waits: a future hold that has been confirmed >ahead of time with pref ConfirmFutureHolds > 0 days. > >Note that future item-level holds are not really interesting to return; this >just corresponds to original behavior. Future next-available holds are not >in view at all; they do not contain an item number. > >Test plan: >Actually, the test plan of the first patch is valid. But for completeness I >repeat it here: > >[1] Enable future holds and set ConfirmFutureHolds to 2 days. >[2] Place a future next-available hold for 2 days ahead. >[3] Check item status on catalogue detail. Available? That is fine. >[4] Confirm the future hold by checking it in. ('future wait') >[5] Look at item status again on catalogue detail. Must be Waiting now. >[6] Switch to OPAC and login as another opac user. Goto Place a hold. >[7] Check item status with item level hold info. Is it waiting? >[8] Try to place hold in staff, check item level status again. Waiting? >[9] Make a transfer for the item. Switch branch. Check hold status on > Transfers to receive. > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Reserves.pm | 14 ++++++-------- > catalogue/detail.pl | 3 +-- > circ/transferstoreceive.pl | 2 +- > opac/opac-reserve.pl | 2 +- > reserve/request.pl | 2 +- > 5 files changed, 10 insertions(+), 13 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 2cbd7f4..ede36b4 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -350,26 +350,24 @@ sub GetReservesFromBiblionumber { > > =head2 GetReservesFromItemnumber > >- ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $waitingdate ) = GetReservesFromItemnumber($itemnumber, $all_dates); >+ ( $reservedate, $borrowernumber, $branchcode, $reserve_id, $waitingdate ) = GetReservesFromItemnumber($itemnumber); > > Get the first reserve for a specific item number (based on priority). Returns the abovementioned values for that reserve. > >-all_dates is an optional parameter, telling Koha to include or exclude future holds >+The routine does not look at future reserves (read: item level holds), but DOES include future waits (a confirmed future hold). > > =cut > > sub GetReservesFromItemnumber { >- my ( $itemnumber, $all_dates ) = @_; >+ my ( $itemnumber ) = @_; > my $dbh = C4::Context->dbh; > my $query = " > SELECT reservedate,borrowernumber,branchcode,reserve_id,waitingdate > FROM reserves >- WHERE itemnumber=? >+ WHERE itemnumber=? AND ( reservedate <= CURRENT_DATE() OR >+ waitingdate IS NOT NULL ) >+ ORDER BY priority > "; >- unless ( $all_dates ) { >- $query .= " AND reservedate <= CURRENT_DATE()"; >- } >- $query .= ' ORDER BY priority'; > my $sth_res = $dbh->prepare($query); > $sth_res->execute($itemnumber); > my ( $reservedate, $borrowernumber,$branchcode, $reserve_id, $wait ) = $sth_res->fetchrow_array; >diff --git a/catalogue/detail.pl b/catalogue/detail.pl >index dc2071c..5d19620 100755 >--- a/catalogue/detail.pl >+++ b/catalogue/detail.pl >@@ -220,8 +220,7 @@ foreach my $item (@items) { > } > > # checking for holds >- my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($item->{itemnumber}, 1); #second parameter: all dates >- # all dates will include a future item level hold or a future wait >+ my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($item->{itemnumber}); > my $ItemBorrowerReserveInfo = GetMemberDetails( $reservedfor, 0); > > if (C4::Context->preference('HidePatronName')){ >diff --git a/circ/transferstoreceive.pl b/circ/transferstoreceive.pl >index 676a2c5..365f200 100755 >--- a/circ/transferstoreceive.pl >+++ b/circ/transferstoreceive.pl >@@ -99,7 +99,7 @@ foreach my $br ( keys %$branches ) { > $getransf{'subtitle'} = GetRecordValue('subtitle', $record, GetFrameworkCode($gettitle->{'biblionumber'})); > > # we check if we have a reserv for this transfer >- my @checkreserv = GetReservesFromItemnumber($num->{'itemnumber'},1 ); #alldate parameter for future holds and waits >+ my @checkreserv = GetReservesFromItemnumber($num->{'itemnumber'}); > if ( $checkreserv[0] ) { > my $getborrower = GetMemberDetails( $checkreserv[1] ); > $getransf{'borrowernum'} = $getborrower->{'borrowernumber'}; >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 87d3128..3b1ca2b 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -431,7 +431,7 @@ foreach my $biblioNum (@biblionumbers) { > } > > # checking reserve >- my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($itemNum,1); #with alldates parameter include future item level holds and waits >+ my ($reservedate,$reservedfor,$expectedAt,undef,$wait) = GetReservesFromItemnumber($itemNum); > my $ItemBorrowerReserveInfo = GetMemberDetails( $reservedfor, 0); > > # the item could be reserved for this borrower vi a host record, flag this >diff --git a/reserve/request.pl b/reserve/request.pl >index b684632..f29ac46 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -328,7 +328,7 @@ foreach my $biblionumber (@biblionumbers) { > } > > # checking reserve >- my ($reservedate,$reservedfor,$expectedAt,$reserve_id,$wait) = GetReservesFromItemnumber($itemnumber,1); #alldates parameter to include future holds/waits >+ my ($reservedate,$reservedfor,$expectedAt,$reserve_id,$wait) = GetReservesFromItemnumber($itemnumber); > my $ItemBorrowerReserveInfo = GetMember( borrowernumber => $reservedfor ); > > if ( defined $reservedate ) { >-- >1.7.2.5
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 9788
:
16026
|
16092
|
19192
|
19193
|
19194
|
19940
|
19941
|
19943
|
19944
|
21479
|
21481
|
21750
|
22309
|
22610
|
22615
|
22640
|
22641
|
22642
|
22643
|
22644
|
22645
|
22646
|
22685
|
22686
|
22687
|
22688
|
22689
|
22693
|
23931
|
23932
|
23933
|
23934
|
23935
|
23936