Bugzilla – Attachment 139735 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: (QA follow-up) Reduce database queries
Bug-31112-QA-follow-up-Reduce-database-queries.patch (text/plain), 6.33 KB, created by
Kyle M Hall (khall)
on 2022-08-24 17:14:15 UTC
(
hide
)
Description:
Bug 31112: (QA follow-up) Reduce database queries
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2022-08-24 17:14:15 UTC
Size:
6.33 KB
patch
obsolete
>From 9892638ce5d1404738e01f8aacbc1af6a300374d Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Wed, 24 Aug 2022 17:10:18 +0000 >Subject: [PATCH] Bug 31112: (QA follow-up) Reduce database queries > >Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> >--- > C4/Circulation.pm | 93 +++++++++++++++++++----------------- > t/db_dependent/Circulation.t | 1 + > 2 files changed, 50 insertions(+), 44 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 1667ec7332..3b113ff434 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -2945,52 +2945,57 @@ sub CanBookBeRenewed { > } > } > >- my $biblio_has_holds = Koha::Holds->search({ biblionumber => $item->biblionumber, non_priority => 0 } )->count > 0; >- if ( $biblio_has_holds && C4::Context->preference('AllowRenewalIfOtherItemsAvailable') && !Koha::Holds->search( { itemnumber => $itemnumber, found => undef } )->count() >- ) >- { >- my $biblio = Koha::Biblios->find($item->biblionumber); >- my @possible_holds = $biblio->current_holds->unfilled->search( >- {non_priority => 0}, >- { prefetch => 'patron' } >- )->as_list; >- >- # Get all other items that could possibly fill reserves >- # FIXME We could join reserves (or more tables) here to eliminate some checks later >- my @other_items = Koha::Items->search({ >- biblionumber => $biblio->biblionumber, >- onloan => undef, >- notforloan => 0, >- -not => { itemnumber => $itemnumber } })->as_list; >- >- return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds); >- >- my %matched_items; >- foreach my $possible_hold (@possible_holds) { >- my $fillable = 0; >- my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber); >- my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron_with_reserve }); >- >- # FIXME: We are not checking whether the item we are renewing can fill the hold >- >- foreach my $other_item (@other_items) { >- next if defined $matched_items{$other_item->itemnumber}; >- next if IsItemOnHoldAndFound( $other_item->itemnumber ); >- next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef, $items_any_available); >- next unless CanItemBeReserved($patron_with_reserve,$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 >- $fillable = 1; >- $matched_items{$other_item->itemnumber} = 1; >- last; >+ my $fillable_holds = Koha::Holds->search( >+ { >+ biblionumber => $item->biblionumber, >+ non_priority => 0, >+ found => undef, >+ reservedate => { '<=' => \'NOW()' }, >+ suspend => 0, >+ }, >+ { prefetch => 'patron' } >+ ); >+ if ( $fillable_holds->count ) { >+ if ( C4::Context->preference('AllowRenewalIfOtherItemsAvailable') ) { >+ my @possible_holds = $fillable_holds->as_list; >+ >+ # Get all other items that could possibly fill reserves >+ # FIXME We could join reserves (or more tables) here to eliminate some checks later >+ my @other_items = Koha::Items->search({ >+ biblionumber => $item->biblionumber, >+ onloan => undef, >+ notforloan => 0, >+ -not => { itemnumber => $itemnumber } })->as_list; >+ >+ return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds); >+ >+ my %matched_items; >+ foreach my $possible_hold (@possible_holds) { >+ my $fillable = 0; >+ my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber); >+ my $items_any_available = ItemsAnyAvailableAndNotRestricted( { biblionumber => $item->biblionumber, patron => $patron_with_reserve }); >+ >+ # FIXME: We are not checking whether the item we are renewing can fill the hold >+ >+ foreach my $other_item (@other_items) { >+ next if defined $matched_items{$other_item->itemnumber}; >+ next if IsItemOnHoldAndFound( $other_item->itemnumber ); >+ next unless IsAvailableForItemLevelRequest($other_item, $patron_with_reserve, undef, $items_any_available); >+ next unless CanItemBeReserved($patron_with_reserve,$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 >+ $fillable = 1; >+ $matched_items{$other_item->itemnumber} = 1; >+ last; >+ } >+ return ( 0, "on_reserve" ) unless $fillable; > } >- return ( 0, "on_reserve" ) unless $fillable; >- } > >- } elsif ($biblio_has_holds) { >- my ($status, $matched_reserve, $possible_reserves) = CheckReserves($itemnumber); >- return ( 0, "on_reserve" ) if $status; >+ } else { >+ my ($status, $matched_reserve, $possible_reserves) = CheckReserves($itemnumber); >+ return ( 0, "on_reserve" ) if $status; >+ } > } > > return ( 0, $auto_renew, { soonest_renew_date => $soonest } ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok"; >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 6c53bbf0b1..37b6f2d753 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -597,6 +597,7 @@ subtest "CanBookBeRenewed tests" => sub { > itemnumber => $item_1->itemnumber, > branchcode => $branch, > priority => 3, >+ reservedate => '1999-01-01', > } > ); > ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber); >-- >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