Bugzilla – Attachment 163236 Details for
Bug 36331
Items that cannot be held are prevented renewal when there are holds on the record
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36331: Don't check reserves that an item cannot fill when checking if it can be renewed
Bug-36331-Dont-check-reserves-that-an-item-cannot-.patch (text/plain), 4.73 KB, created by
Nick Clemens (kidclamp)
on 2024-03-15 13:53:27 UTC
(
hide
)
Description:
Bug 36331: Don't check reserves that an item cannot fill when checking if it can be renewed
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2024-03-15 13:53:27 UTC
Size:
4.73 KB
patch
obsolete
>From 90b477f8008c96d519d7f865689f3eb3f67d1b49 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Fri, 15 Mar 2024 13:45:58 +0000 >Subject: [PATCH] Bug 36331: Don't check reserves that an item cannot fill when > checking if it can be renewed > >Before this patch we get all holds on a record and see if we can fill them with available items. >This means we check to fill holds that the item in questoion may not be able to fill, especially >in the case where no holds are allowed on the item type, this is wrong > >To test: >1 - Find or create a biblio with two items of different item types >2 - Make sure one item type allows holds, and the other has: > "Default holds policy by item type" > Set to "No holds allowed" >3 - Set system preference "AllowRenewalIfOtherItemsAvailable" to "Don't allow" >4 - Check out the unholdable item to a patron >5 - Set a hold for a different patron on the next available item >6 - Confirm the checked out item can be renewed (don't renew, just view the checkouts page) >7 - Checkout the other item to a third patron >8 - Confirm the first item can still be renewed >9 - Set system preference "AllowRenewalIfOtherItemsAvailable" to "Allow" >10 - Confirm the item cannot be renewed now >11 - Apply patch, restart all >12 - Confirm the item can be renewed >13 - Set the item type to a type that allows holds >14 - Confirm the item can no longer be renewed >15 - Restore the item type >16 - Set system preference "AllowRenewalIfOtherItemsAvailable" to "Don't allow" >17 - Confirm the item can be renewed >18 - Check in the item from the third patron >19 - Confirm the item can still be renewed >20 - prove -v t/db_dependent/Circulation.t - test still pass >--- > C4/Circulation.pm | 23 ++++++----------------- > t/db_dependent/Circulation.t | 7 ++----- > 2 files changed, 8 insertions(+), 22 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 13d1758f493..b56fac2fbdb 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -3069,18 +3069,10 @@ sub CanBookBeRenewed { > return ( 0, "on_reserve" ) > if ( $item->current_holds->search( { non_priority => 0 } )->count ); > >- my $fillable_holds = Koha::Holds->search( >- { >- biblionumber => $item->biblionumber, >- non_priority => 0, >- found => undef, >- reservedate => { '<=' => \'NOW()' }, >- suspend => 0 >- } >- ); >- if ( $fillable_holds->count ) { >+ >+ my ($status, $matched_reserve, $fillable_holds) = CheckReserves($item); >+ if ( $fillable_holds ) { > 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 >@@ -3090,14 +3082,12 @@ sub CanBookBeRenewed { > notforloan => 0, > -not => { itemnumber => $item->itemnumber } })->as_list; > >- return ( 0, "on_reserve" ) if @possible_holds && (scalar @other_items < scalar @possible_holds); >+ return ( 0, "on_reserve" ) if @{$fillable_holds} && (scalar @other_items < scalar @{$fillable_holds} ); > > my %matched_items; >- foreach my $possible_hold (@possible_holds) { >+ foreach my $possible_hold ( @{$fillable_holds} ) { > my $fillable = 0; >- my $patron_with_reserve = Koha::Patrons->find($possible_hold->borrowernumber); >- >- # FIXME: We are not checking whether the item we are renewing can fill the hold >+ my $patron_with_reserve = Koha::Patrons->find($possible_hold->{borrowernumber}); > > foreach my $other_item (@other_items) { > next if defined $matched_items{$other_item->itemnumber}; >@@ -3115,7 +3105,6 @@ sub CanBookBeRenewed { > } > } > else { >- my ($status, $matched_reserve, $possible_reserves) = CheckReserves($item); > return ( 0, "on_reserve" ) if $status; > } > } >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index 8dd246578f2..e13985f8ffd 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -4421,12 +4421,9 @@ subtest 'ItemsDeniedRenewal rules are checked' => sub { > } > ); > >- my $allow_book = $builder->build_object({ class => 'Koha::Items', value => { >+ my $allow_book = $builder->build_sample_item({ > homebranch => $idr_lib->branchcode, >- withdrawn => 0, >- itype => 'NOHIDE', >- location => 'NOPROC' >- } >+ holdingbranch => $idr_lib->branchcode, > }); > > my $idr_borrower = $builder->build_object({ class => 'Koha::Patrons', value=> { >-- >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 36331
:
163236
|
163254
|
163673
|
163706
|
163707