Attachment 22731 Details for Bug 8918 – Bug 8918: Calculate hold priority in AddReserve

[patch] Bug 8918: Calculate hold priority in AddReserve

Bug-8918-Calculate-hold-priority-in-AddReserve.patch (text/plain), 10.93 KB, created by Julian Maurice on 2013-11-05 15:27:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Julian Maurice

Created: 2013-11-05 15:27:13 UTC

Size: 10.93 KB

patch

obsolete

>From 40ff2363fca0456760bdb9fc48304c090f4129a9 Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>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 <http://koha-community.org/>
>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

Actions: View | Diff | Splinter Review

Attachments on bug 8918: 12810 | 13472 | 13565 | 15435 | 17376 | 17881 | 17882 | 17888 | 17889 | 17890 | 21328 | 21329 | 21330 | 22731 | 22732 | 24863 | 24864 | 24880 | 24881 | 24882 | 24883 | 24884 | 24885 | 24886 | 24887 | 24888 | 24920 | 25080 | 25081 | 25082 | 25083 | 25084 | 25118 | 25119 | 25139 | 25140 | 25141 | 25142 | 25143 | 25144