From 6fb2c0418520ee964d57d9376327602c86b7f4ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= Date: Tue, 25 Sep 2018 13:27:07 +0000 Subject: [PATCH 1/2] Bug 21373: refactor code in C4/Circulation.pm This moves a code chunk from CanBookBeRenewed() to a new function IsItemAvailableForCheckout(). To test: 1. Run tests and make sure they pass --- C4/Circulation.pm | 151 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 85 insertions(+), 66 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index dce77ea..ad95ea5 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2529,6 +2529,88 @@ sub GetBiblioIssues { return \@issues; } + +=head2 IsItemAvailableForCheckout + + $available = ($itemnumber); + +Find out whether an item doesn't have any holds, item level or more +biblio level holds than there are available items. + +C<$itemnumber> is the number of the item to check the availability for. + +=cut +sub IsItemAvailableForCheckout { + my ($itemnumber) = @_; + my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber); + + # This item can fill one or more unfilled reserve, can those unfilled reserves + # all be filled by other available items? + if ( $resfound ) + { + my $schema = Koha::Database->new()->schema(); + + my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count(); + if ($item_holds) { + # There is an item level hold on this item, no other item can fill the hold + $resfound = 1; + } + else { + + # Get all other items that could possibly fill reserves + my @itemnumbers = $schema->resultset('Item')->search( + { + biblionumber => $resrec->{biblionumber}, + onloan => undef, + notforloan => 0, + -not => { itemnumber => $itemnumber } + }, + { columns => 'itemnumber' } + )->get_column('itemnumber')->all(); + + # Get all other reserves that could have been filled by this item + my @borrowernumbers; + while (1) { + my ( $reserve_found, $reserve, undef ) = + C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers ); + + if ($reserve_found) { + push( @borrowernumbers, $reserve->{borrowernumber} ); + } + else { + last; + } + } + + # If the count of the union of the lists of reservable items for each borrower + # is equal or greater than the number of borrowers, we know that all reserves + # can be filled with available items. We can get the union of the sets simply + # by pushing all the elements onto an array and removing the duplicates. + my @reservable; + my %borrowers; + ITEM: foreach my $i (@itemnumbers) { + my $item = GetItem($i); + next if IsItemOnHoldAndFound($i); + for my $b (@borrowernumbers) { + my $borr = $borrowers{$b} //= Koha::Patrons->find( $b )->unblessed; + next unless IsAvailableForItemLevelRequest($item, $borr); + next unless CanItemBeReserved($b,$i); + + push @reservable, $i; + if (@reservable >= @borrowernumbers) { + $resfound = 0; + last ITEM; + } + last; + } + } + } + } + + return !$resfound; +} + + =head2 GetUpcomingDueIssues my $upcoming_dues = GetUpcomingDueIssues( { days_in_advance => 4 } ); @@ -2600,72 +2682,9 @@ sub CanBookBeRenewed { my $patron = Koha::Patrons->find( $borrowernumber ) or return; - my ( $resfound, $resrec, undef ) = C4::Reserves::CheckReserves($itemnumber); - - # This item can fill one or more unfilled reserve, can those unfilled reserves - # all be filled by other available items? - if ( $resfound - && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) - { - my $schema = Koha::Database->new()->schema(); - - my $item_holds = $schema->resultset('Reserve')->search( { itemnumber => $itemnumber, found => undef } )->count(); - if ($item_holds) { - # There is an item level hold on this item, no other item can fill the hold - $resfound = 1; - } - else { - - # Get all other items that could possibly fill reserves - my @itemnumbers = $schema->resultset('Item')->search( - { - biblionumber => $resrec->{biblionumber}, - onloan => undef, - notforloan => 0, - -not => { itemnumber => $itemnumber } - }, - { columns => 'itemnumber' } - )->get_column('itemnumber')->all(); - - # Get all other reserves that could have been filled by this item - my @borrowernumbers; - while (1) { - my ( $reserve_found, $reserve, undef ) = - C4::Reserves::CheckReserves( $itemnumber, undef, undef, \@borrowernumbers ); - - if ($reserve_found) { - push( @borrowernumbers, $reserve->{borrowernumber} ); - } - else { - last; - } - } - - # If the count of the union of the lists of reservable items for each borrower - # is equal or greater than the number of borrowers, we know that all reserves - # can be filled with available items. We can get the union of the sets simply - # by pushing all the elements onto an array and removing the duplicates. - my @reservable; - my %borrowers; - ITEM: foreach my $i (@itemnumbers) { - my $item = GetItem($i); - next if IsItemOnHoldAndFound($i); - for my $b (@borrowernumbers) { - my $borr = $borrowers{$b} //= Koha::Patrons->find( $b )->unblessed; - next unless IsAvailableForItemLevelRequest($item, $borr); - next unless CanItemBeReserved($b,$i); - - push @reservable, $i; - if (@reservable >= @borrowernumbers) { - $resfound = 0; - last ITEM; - } - last; - } - } - } - } - return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found + my ( $resfound, undef, undef ) = C4::Reserves::CheckReserves($itemnumber); + return ( 0, "on_reserve" ) if !C4::Context->preference('AllowRenewalIfOtherItemsAvailable') && $resfound; + return ( 0, "on_reserve" ) if !IsItemAvailableForCheckout($itemnumber); return ( 1, undef ) if $override_limit; -- 2.7.4