From 3e344d7e0718e5a8ac810ca5235f68c1e6d3f370 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Mon, 15 Oct 2012 14:06:04 +0200 Subject: [PATCH] Bug 8918: Fix reserve priority in ILS-DI The priority of new hold requests was not calculated when using ILS-DI. A new routine is added, C4::Reserves::CalculatePriority(), to calculate the priority prior to placing a request. A separate bug report, 11640, covers the changes in reserves to use this new routine more generally. This patch does therefore only affect ILS-DI. Note: ILS-DI already allows you to generate multiple holds on a biblio or item for the same patron. This patch does not change that behavior. Test plan: [1] Place multiple holds using ILS-DI HoldTitle service: /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER&request_location=test Check the priority. [2] Do the same using HoldItem service: /cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER&item_id=ITEMNUMBER Check the priority again. [3] Use a biblio with multiple items. Place item level holds on both. Check in one of these items in another branch. Confirm transfer. Check in the other item in the original branch. Confirm hold. Now you have a waiting and a transit hold. Test HoldTitle and HoldItem service again a few times. [4] Enable AllowHoldDateInFuture and add a future hold. Now test HoldTitle and HoldItem again and check if these holds are inserted before the future hold (lower priority). January 29, 2014: Rebased this patch and amended it to make a distinction between fixing the ILS-DI bug and using the new routine. Updated commit message and test plan (marcelr). Signed-off-by: Marcel de Rooy Signed-off-by: Kyle M Hall Signed-off-by: Galen Charlton --- C4/ILSDI/Services.pm | 26 +++++++++++--------------- C4/Reserves.pm | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index f5e6747cd7..bd201d3d1a 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -623,8 +623,11 @@ sub HoldTitle { } # Add the reserve - # $branch, $borrowernumber, $biblionumber, $constraint, $bibitems, $priority, $notes, $title, $checkitem, $found - AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, 0, undef, $title, undef, undef ); + # $branch, $borrowernumber, $biblionumber, + # $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 ); # Hashref building my $out; @@ -686,9 +689,8 @@ sub HoldItem { my $canbookbereserved = C4::Reserves::CanBookBeReserved( $borrowernumber, $biblionumber ); return { code => 'NotHoldable' } unless $canbookbereserved and $canitembereserved; - my $branch; - # Pickup branch management + my $branch; if ( $cgi->param('pickup_location') ) { $branch = $cgi->param('pickup_location'); my $branches = GetBranches(); @@ -697,18 +699,12 @@ sub HoldItem { $branch = $$borrower{branchcode}; } - my $rank; - my $found; - - # Get rank and found - $rank = '0' unless C4::Context->preference('ReservesNeedReturns'); - if ( $item->{'holdingbranch'} eq $branch ) { - $found = 'W' unless C4::Context->preference('ReservesNeedReturns'); - } - # Add the reserve - # $branch,$borrowernumber,$biblionumber,$constraint,$bibitems,$priority,$resdate,$expdate,$notes,$title,$checkitem,$found - AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, $rank, '', '', '', $title, $itemnumber, $found ); + # $branch, $borrowernumber, $biblionumber, + # $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 ); # Hashref building my $out; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 8fe0fd0a00..03a5c30ba4 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2249,6 +2249,42 @@ sub GetReservesControlBranch { return $branchcode; } +=head2 CalculatePriority + + my $p = CalculatePriority($biblionumber, $resdate); + +Calculate priority for a new reserve on biblionumber. +The reserve date parameter is optional. Plays a role if the preference +AllowHoldDateInFuture is set. +After calculation of this priority, it is recommended to call +_ShiftPriorityByDateAndPriority. Note that this is currently done in +AddReserves. + +=cut + +sub CalculatePriority { + my ( $biblionumber, $resdate ) = @_; + + my $sql = qq{ + SELECT COUNT(*) FROM reserves + WHERE biblionumber=? AND priority>0 AND + (found IS NULL or found='') + }; + #skip found==W or found==T (waiting or transit holds) + if( $resdate ) { + $sql.= ' AND ( reservedate<=? )'; + } + else { + $sql.= ' AND ( reservedate < NOW() )'; + } + my $dbh = C4::Context->dbh(); + my @row= $dbh->selectrow_array( $sql, undef, $resdate? + ($biblionumber, $resdate): ($biblionumber) ); + + return @row? $row[0]+1: 1; + #if @row does not contain anything, something went wrong.. +} + =head1 AUTHOR Koha Development Team -- 2.39.5