Attachment 96265 Details for Bug 24160 – Bug 24160: Fix OPACHoldsIfAvailableAtPickup to follow holds policy

[patch] Bug 24160: Fix OPACHoldsIfAvailableAtPickup to follow holds policy

Bug-24160-Fix-OPACHoldsIfAvailableAtPickup-to-foll.patch (text/plain), 8.02 KB, created by Jonathan Druart on 2019-12-13 16:21:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2019-12-13 16:21:28 UTC

Size: 8.02 KB

patch

obsolete

>From ad376ee78bd69a082788401bad4098679fd6cad7 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 11 Dec 2019 19:43:32 +0100
>Subject: [PATCH] Bug 24160: Fix OPACHoldsIfAvailableAtPickup to follow holds
> policy
>
>Bug 17453 added the ability to restrict the pick up location for items,
>if at least one item was available at this location.
>It assumed that an available item was an item that was not checked out,
>not lost and not damaged.
>But actually we should follow holds policy, defined in the circulation
>rules.
>An item that is not checked out but on which no hold can be placed on,
>it should be considered as not available.
>
>Test plan:
>0/ Setting up the problematic situation
>Considering a bibliographic record with 3 items I1, I2, and I3:
>  I1, I2 and I3 have the same location (say Centerville)
>  I1 and I2 have the item type 'Book' and are checked out
>  I3 is a "Computer Files"
>"Computer Files" has a "No holds allowed" hold policy (bottom of the
>circulation rules view)
>
>Turn on off (Don't allow) OPACHoldsIfAvailableAtPickup
>
>1/ At the OPAC place a hold on this bibliographic record (biblio or
>item-level hold), open the "Pick up location" dropdown list
>=> Without this patch you cannot select "Centerville"
>=> With this patch applied you can select it
>
>2/ Confirm the hold
>=> The hold as been correctly placed on the item and the pickup location
>is correct.
>
>3/ Cancel the hold
>4/ Check I1 in
>
>5/ Repeat 1/
>=> Without and with this patch you cannot select "Centerville"
>
>6/ Open the HTML inspector in the browser, select the dropdown list and
>remove the 'disabled="disabled"' attribute of the CPL option
>
>7/ Select (force) Centerville
>
>8/ Confirm the hold
>=> Nothing happened! You tried to cheat and place a hold to pickup at an
>invalid location.
>
>Note for QA: I have tried to move that code to a module but it's not
>trivial at all. The availability needs to be indexed in the search
>engine to make all of this much more easy.
>
>Sponsored-by: Educampus
>---
> opac/opac-reserve.pl | 76 +++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 60 insertions(+), 16 deletions(-)
>
>diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl
>index 4d7cf1334c..665d5e3769 100755
>--- a/opac/opac-reserve.pl
>+++ b/opac/opac-reserve.pl
>@@ -295,11 +295,32 @@ if ( $query->param('place_reserve') ) {
>         }
> 
>         unless ( $can_place_hold_if_available_at_pickup ) {
>+            # Search for libraries where the hold cannot be picked up at
>             my $items_in_this_library = Koha::Items->search({ biblionumber => $biblioNum, holdingbranch => $branch });
>-            my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count;
>-            my $nb_of_items_unavailable = $items_in_this_library->search({ -or => { lost => { '!=' => 0 }, damaged => { '!=' => 0 }, } });
>-            if ( $items_in_this_library->count > $nb_of_items_issued + $nb_of_items_unavailable ) {
>+            my @availables_for_hold;
>+            # The hold avaibility has not been calculated before, we must call CanItemBeReserved here.
>+            # Note that the complexity is quite bad here, but until we store the avaibility we will have to calculate it!
>+            while ( my $item = $items_in_this_library->next ) {
>+                push @availables_for_hold, $item
>+                    if CanItemBeReserved( $borrowernumber, $item->itemnumber, $branch )->{status} eq 'OK'
>+            }
>+
>+            if ( @availables_for_hold > 0 ) {
>+                my $items = Koha::Items->search(
>+                    {
>+                        'me.itemnumber' => {
>+                            -in =>
>+                              [ map { $_->itemnumber } @availables_for_hold ]
>+                        }
>+                    }
>+                );
>+                my $checked_out_items = $items->search(
>+                    { 'issue.itemnumber' => { not => undef }, },
>+                    { join => 'issue', },
>+                );
>+                # If there are not all checked out, the hold cannot be picked at this library
>                 $canreserve = 0
>+                    if $items->count > $checked_out_items->count;
>             }
>         }
> 
>@@ -461,6 +482,7 @@ foreach my $biblioNum (@biblionumbers) {
>         my $item = Koha::Items->find( $itemNum );
>         my $itemLoopIter = {};
> 
>+        $itemLoopIter->{item} = $item;
>         $itemLoopIter->{itemnumber} = $itemNum;
>         $itemLoopIter->{barcode} = $itemInfo->{barcode};
>         $itemLoopIter->{homeBranchName} = $itemInfo->{homebranch};
>@@ -555,14 +577,6 @@ foreach my $biblioNum (@biblionumbers) {
>                 $biblioLoopIter{force_hold} = 1 if $opac_hold_policy eq 'F';
>             }
>             $numCopiesAvailable++;
>-
>-            unless ( $can_place_hold_if_available_at_pickup ) {
>-                my $items_in_this_library = Koha::Items->search({ biblionumber => $itemInfo->{biblionumber}, holdingbranch => $itemInfo->{holdingbranch} });
>-                my $nb_of_items_issued = $items_in_this_library->search({ 'issue.itemnumber' => { not => undef }}, { join => 'issue' })->count;
>-                if ( $items_in_this_library->count > $nb_of_items_issued ) {
>-                    push @not_available_at, $itemInfo->{holdingbranch};
>-                }
>-            }
>         }
> 
>         $itemLoopIter->{imageurl} = getitemtypeimagelocation( 'opac', $itemtypes->{ $itemInfo->{itype} }{imageurl} );
>@@ -578,6 +592,7 @@ foreach my $biblioNum (@biblionumbers) {
> 
>         push @{$biblioLoopIter{itemLoop}}, $itemLoopIter;
>     }
>+
>     $template->param(
>         itemdata_enumchron => $itemdata_enumchron,
>         itemdata_ccode     => $itemdata_ccode,
>@@ -599,14 +614,43 @@ foreach my $biblioNum (@biblionumbers) {
>     }
> 
>     if ( $biblioLoopIter{holdable} ) {
>-        @not_available_at = uniq @not_available_at;
>-        $biblioLoopIter{not_available_at} = \@not_available_at ;
>-    }
>+        unless ($can_place_hold_if_available_at_pickup) {
>+            # Search for libraries where the hold cannot be picked up at
>+            my @branchcodes = uniq map { $_->{item}->holdingbranch } @{ $biblioLoopIter{itemLoop} };
>+            for my $branchcode (@branchcodes) {
>+                # The hold avaibility has been calculated before, retrieving the items
>+                my @availables_for_hold_in_this_library = map {
>+                    $_->{item}->holdingbranch eq $branchcode
>+                      && exists $_->{available} && $_->{available}
>+                      ? $_->{item}
>+                      : ()
>+                } @{ $biblioLoopIter{itemLoop} };
>+
>+                if ( @availables_for_hold_in_this_library > 0 ) {
>+                    # Searching for items available for hold but that are not checked out
>+                    my $items_in_this_library = Koha::Items->search(
>+                        {
>+                            'me.itemnumber' => {
>+                                -in => [
>+                                    map { $_->itemnumber } @availables_for_hold_in_this_library
>+                                ]
>+                            }
>+                        }
>+                    );
>+                    my $checked_out_items = $items_in_this_library->search(
>+                        { 'issue.itemnumber' => { not => undef } },
>+                        { join               => 'issue' } );
>+                    # If there are not all checked out, the library cannot be used to pickup the hold
>+                    push @not_available_at, $branchcode
>+                        if $items_in_this_library->count > $checked_out_items->count;
>+                }
>+            }
>+        }
> 
>-    unless ( $can_place_hold_if_available_at_pickup ) {
>         @not_available_at = uniq @not_available_at;
>         $biblioLoopIter{not_available_at} = \@not_available_at ;
>-        # The record is not holdable is not available at any of the libraries
>+
>+        # The record is not holdable if not available at any of the libraries
>         if ( Koha::Libraries->search->count == @not_available_at ) {
>             $biblioLoopIter{holdable} = 0;
>         }
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 24160: 96265 | 99640