Bugzilla – Attachment 137856 Details for
Bug 31112
Able to renew checkout when the number of holds exceeds available number of items
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31112: Remove unnecessary if-clause
Bug-31112-Remove-unnecessary-if-clause.patch (text/plain), 7.18 KB, created by
Biblibre Sandboxes
on 2022-07-19 08:52:56 UTC
(
hide
)
Description:
Bug 31112: Remove unnecessary if-clause
Filename:
MIME Type:
Creator:
Biblibre Sandboxes
Created:
2022-07-19 08:52:56 UTC
Size:
7.18 KB
patch
obsolete
>From 5193e6b47d5447a989eed22c76f6b5c090b344ba Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Joonas=20Kylm=C3=A4l=C3=A4?= <joonas.kylmala@iki.fi> >Date: Mon, 18 Jul 2022 16:29:06 +0000 >Subject: [PATCH] Bug 31112: Remove unnecessary if-clause > >To test: > 1) Please check manually that the logic stays the same, use git's -w command line parameter to > ignore whitespace changes in the diff output. > 2) prove t/db_dependent/Circulation.t > >Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> >--- > C4/Circulation.pm | 114 +++++++++++++++++++++------------------------- > 1 file changed, 53 insertions(+), 61 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index a18864dea7..cd4b5e0836 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2965,68 +2965,60 @@ sub CanBookBeRenewed { > # 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') ) >+ && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') && !Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count() ) > { >- my $item_holds = Koha::Holds->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 >- # FIXME We could join reserves (or more tables) here to eliminate some checks later >- my $items = Koha::Items->search({ >- biblionumber => $resrec->{biblionumber}, >- onloan => undef, >- notforloan => 0, >- -not => { itemnumber => $itemnumber } >- }); >- my $item_count = $items->count(); >- >- # Get all other reserves that could have been filled by this item >- my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves; >- # Note: fetching the patrons in this manner means that a patron with 2 holds will >- # not block renewal if one reserve can be satisfied i.e. each patron is checked once >- my $patrons = Koha::Patrons->search({ >- borrowernumber => { -in => \@borrowernumbers } >- }); >- my $patron_count = $patrons->count(); >- >- return ( 0, "on_reserve" ) if ($patron_count > $item_count); >- # We cannot possibly fill all reserves if we don't have enough items >- >- # If we can fill each hold that has been found with the available items on the record >- # then the patron can renew. If we cannot, they cannot renew. >- # FIXME This code does not check whether the item we are renewing can fill >- # any of the existing reserves. >- my $reservable = 0; >- my %matched_items; >- my $seen = 0; >- PATRON: while ( my $patron = $patrons->next ) { >- # If there is a reserve that cannot be filled we are done >- return ( 0, "on_reserve" ) if ( $seen > $reservable ); >- my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron }); >- while ( my $other_item = $items->next ) { >- next if defined $matched_items{$other_item->itemnumber}; >- next if IsItemOnHoldAndFound( $other_item->itemnumber ); >- next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available); >- next unless CanItemBeReserved($patron,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK'; >- # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy' >- # CanItemBeReserved checks 'rules' and 'policies' which means >- # items will fill holds at checkin that are rejected here >- $reservable++; >- if ($reservable >= $patron_count) { >- $resfound = 0; >- last PATRON; >- } >- $matched_items{$other_item->itemnumber} = 1; >- last; >- } >- $items->reset; >- $seen++; >- } >- } >+ # Get all other items that could possibly fill reserves >+ # FIXME We could join reserves (or more tables) here to eliminate some checks later >+ my $items = Koha::Items->search({ >+ biblionumber => $resrec->{biblionumber}, >+ onloan => undef, >+ notforloan => 0, >+ -not => { itemnumber => $itemnumber } >+ }); >+ my $item_count = $items->count(); >+ >+ # Get all other reserves that could have been filled by this item >+ my @borrowernumbers = map { $_->{borrowernumber} } @$possible_reserves; >+ # Note: fetching the patrons in this manner means that a patron with 2 holds will >+ # not block renewal if one reserve can be satisfied i.e. each patron is checked once >+ my $patrons = Koha::Patrons->search({ >+ borrowernumber => { -in => \@borrowernumbers } >+ }); >+ my $patron_count = $patrons->count(); >+ >+ return ( 0, "on_reserve" ) if ($patron_count > $item_count); >+ # We cannot possibly fill all reserves if we don't have enough items >+ >+ # If we can fill each hold that has been found with the available items on the record >+ # then the patron can renew. If we cannot, they cannot renew. >+ # FIXME This code does not check whether the item we are renewing can fill >+ # any of the existing reserves. >+ my $reservable = 0; >+ my %matched_items; >+ my $seen = 0; >+ PATRON: while ( my $patron = $patrons->next ) { >+ # If there is a reserve that cannot be filled we are done >+ return ( 0, "on_reserve" ) if ( $seen > $reservable ); >+ my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron }); >+ while ( my $other_item = $items->next ) { >+ next if defined $matched_items{$other_item->itemnumber}; >+ next if IsItemOnHoldAndFound( $other_item->itemnumber ); >+ next unless IsAvailableForItemLevelRequest($other_item, $patron, undef, $items_any_available); >+ next unless CanItemBeReserved($patron,$other_item,undef,{ignore_hold_counts=>1})->{status} eq 'OK'; >+ # NOTE: At checkin we call 'CheckReserves' which checks hold 'policy' >+ # CanItemBeReserved checks 'rules' and 'policies' which means >+ # items will fill holds at checkin that are rejected here >+ $reservable++; >+ if ($reservable >= $patron_count) { >+ $resfound = 0; >+ last PATRON; >+ } >+ $matched_items{$other_item->itemnumber} = 1; >+ last; >+ } >+ $items->reset; >+ $seen++; >+ } > } > > return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 31112
:
137826
|
137827
|
137828
|
137856
|
137857
|
137858
|
139492
|
139493
|
139494
|
139495
|
139622
|
139623
|
139729
|
139730
|
139731
|
139732
|
139733
|
139734
|
139735