From 40ff2363fca0456760bdb9fc48304c090f4129a9 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Mon, 15 Oct 2012 14:06:04 +0200 Subject: [PATCH] Bug 8918: Calculate hold priority in AddReserve Priority was calculated outside of this sub, in separate places. Priority was not calculated when using ILS-DI. This patch factorize code by putting the priority calculation code into AddReserve. Test plan: 1/ Place multiple holds in staff interface and check the priority is incremented for each hold. 2/ Do the same in OPAC. 3/ Place multiple holds using ILS-DI HoldTitle service: /cgi-bin/koha/ilsdi.pl?service=HoldTitle&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER and check the priority is incremented for each hold. 4/ Do the same using HoldItem service: /cgi-bin/koha/ilsdi.pl?service=HoldItem&patron_id=BORROWERNUMBER&bib_id=BIBLIONUMBER&item_id=ITEMNUMBER --- C4/ILSDI/Services.pm | 16 ++----- C4/Reserves.pm | 50 ++++++++++++++++++-- .../en/modules/admin/preferences/circulation.pref | 2 +- .../en/modules/admin/preferences/web_services.pref | 3 +- opac/opac-reserve.pl | 14 +----- reserve/placerequest.pl | 21 ++------ 6 files changed, 59 insertions(+), 47 deletions(-) diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm index 7941a70..af8dd7c 100644 --- a/C4/ILSDI/Services.pm +++ b/C4/ILSDI/Services.pm @@ -625,7 +625,7 @@ 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 ); + AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, undef, undef, $title, undef, undef ); # Hashref building my $out; @@ -687,9 +687,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(); @@ -698,18 +697,9 @@ 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 ); + AddReserve( $branch, $borrowernumber, $biblionumber, 'a', undef, undef, '', '', '', $title, $itemnumber ); # Hashref building my $out; diff --git a/C4/Reserves.pm b/C4/Reserves.pm index d1bca25..1c2f72d 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -134,7 +134,10 @@ BEGIN { &GetReservesControlBranch ); - @EXPORT_OK = qw( MergeHolds ); + @EXPORT_OK = qw( + MergeHolds + GetReserveNextRank + ); } =head2 AddReserve @@ -161,9 +164,29 @@ sub AddReserve { } else { undef $expdate; # make reserves.expirationdate default to null rather than '0000-00-00' } + + # Calculate priority if not given + if (not defined $priority) { + if ($checkitem and not C4::Context->preference('ReservesNeedReturns')) { + $priority = 0; + } else { + $priority = GetReserveNextRank($biblionumber); + } + } + + # Calculate found if not given + if ($priority == 0 and not defined $found) { + my $item = GetItem($checkitem); + if ($item->{holdingbranch} eq $branch) { + $found = 'W'; + } else { + $found = 'T'; + } + } + if ( C4::Context->preference( 'AllowHoldDateInFuture' ) ) { - # Make room in reserves for this before those of a later reserve date - $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority ); + # Make room in reserves for this before those of a later reserve date + $priority = _ShiftPriorityByDateAndPriority( $biblionumber, $resdate, $priority ); } my $waitingdate; @@ -2214,6 +2237,27 @@ sub GetReservesControlBranch { return $branchcode; } +=head2 GetReserveNextRank + + my $next_rank = GetReserveNextRank($biblionumber); + +Calculate the next available reseve rank for a biblionumber. + +=cut + +sub GetReserveNextRank { + my ($biblionumber) = @_; + + my ( $count, $reserves ) = GetReservesFromBiblionumber($biblionumber,1); + foreach my $res (@$reserves) { + $count-- if (defined $res->{found} and $res->{found} eq 'W'); + } + my $rank = $count + 1; + + return $rank; +} + + =head1 AUTHOR Koha Development Team diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index 9ae9c44..85b46de 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -435,7 +435,7 @@ Circulation: choices: yes: "Don't automatically" no: Automatically - - mark holds as found and waiting when a hold is placed specifically on them and they are already checked in. + - mark a hold as found and waiting when a hold is placed on a specific item and that item is already checked in. - - Patrons can only have - pref: maxreserves diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref index 7244675..7a4c910 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref @@ -35,9 +35,10 @@ Web services: no: Disable - ILS-DI services for OPAC users - + - Allow IP addresses - pref: ILS-DI:AuthorizedIPs class: Text - - allowed IPs to use the ILS-DI services + - to use the ILS-DI services (when enabled). Separate the IP addresses with commas and without spaces. Leave the field blank to allow any IP address. Reporting: - - Only return diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index 97d136a..201f8ef 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -223,7 +223,6 @@ if ( $query->param('place_reserve') ) { } my $biblioData = $biblioDataHash{$biblioNum}; - my $found; # Check for user supplied reserve date my $startdate; @@ -235,17 +234,8 @@ if ( $query->param('place_reserve') ) { my $expiration_date = $query->param("expiration_date_$biblioNum"); - # If a specific item was selected and the pickup branch is the same as the - # holdingbranch, force the value $rank and $found. - my $rank = $biblioData->{rank}; if ( $itemNum ne '' ) { $canreserve = 1 if CanItemBeReserved( $borrowernumber, $itemNum ); - $rank = '0' unless C4::Context->preference('ReservesNeedReturns'); - my $item = GetItem($itemNum); - if ( $item->{'holdingbranch'} eq $branch ) { - $found = 'W' - unless C4::Context->preference('ReservesNeedReturns'); - } } else { $canreserve = 1 if CanBookBeReserved( $borrowernumber, $biblioNum ); @@ -266,10 +256,10 @@ if ( $query->param('place_reserve') ) { AddReserve( $branch, $borrowernumber, $biblioNum, 'a', - [$biblioNum], $rank, + [$biblioNum], undef, $startdate, $expiration_date, $notes, $biblioData->{title}, - $itemNum, $found + $itemNum, undef ); ++$reserve_cnt; } diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 3fe459c..bd1ce13 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -68,19 +68,6 @@ foreach my $bibnum (@biblionumbers) { $bibinfos{$bibnum} = \%bibinfo; } -my $found; - -# if we have an item selectionned, and the pickup branch is the same as the holdingbranch -# of the document, we force the value $rank and $found . -if ($checkitem ne ''){ - $rank[0] = '0' unless C4::Context->preference('ReservesNeedReturns'); - my $item = $checkitem; - $item = GetItem($item); - if ( $item->{'holdingbranch'} eq $branch ){ - $found = 'W' unless C4::Context->preference('ReservesNeedReturns'); - } -} - if ($type eq 'str8' && $borrower){ foreach my $biblionumber (keys %bibinfos) { @@ -110,17 +97,17 @@ if ($type eq 'str8' && $borrower){ if ($multi_hold) { my $bibinfo = $bibinfos{$biblionumber}; AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',[$biblionumber], - $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem,$found); + $bibinfo->{rank},$startdate,$expirationdate,$notes,$bibinfo->{title},$checkitem, undef); } 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); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,undef,$startdate,$expirationdate,$notes,$title,$checkitem,undef); } 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); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'o',\@reqbib,undef,$startdate,$expirationdate,$notes,$title,$checkitem,undef); } else { - AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,$rank[0],$startdate,$expirationdate,$notes,$title,$checkitem, $found); + AddReserve($branch,$borrower->{'borrowernumber'},$biblionumber,'a',\@realbi,undef,$startdate,$expirationdate,$notes,$title,$checkitem,undef); } } } -- 1.7.10.4