From ad376ee78bd69a082788401bad4098679fd6cad7 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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