Bugzilla – Attachment 190383 Details for
Bug 41416
Poor performance when clicking 'Update hold(s)' on request.pl for records with many holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41416: Add skip_fix_priority option to ModReserve
Bug-41416-Add-skipfixpriority-option-to-ModReserve.patch (text/plain), 9.75 KB, created by
Nick Clemens (kidclamp)
on 2025-12-09 19:07:36 UTC
(
hide
)
Description:
Bug 41416: Add skip_fix_priority option to ModReserve
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2025-12-09 19:07:36 UTC
Size:
9.75 KB
patch
obsolete
>From b3048305d898ef3962699351644651b3ab492fa1 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Tue, 9 Dec 2025 18:58:58 +0000 >Subject: [PATCH] Bug 41416: Add skip_fix_priority option to ModReserve > >This patch adds a new parameter to ModReserve: skip_fix_priority > >The page for altering priority ensures each hold has a unique rank, so doesn't allow for setting two >holds to the same rank, so we don't need to reorder everything until the end (in case of some cancellations etc) > >This patch also makes FixPriority exported and updates the name as it is already called in a few places outside of >C4:Reserve > >To test: > 1 - Place many holds on a record, 50 - 150 > https://github.com/kidclamp/handy-koha-script/blob/main/randhold.pl > 2 - View the record in the staff interface: > http://localhost:8081/cgi-bin/koha/catalogue/detail.pl?biblionumber=312 > 3 - Click on the 'Holds' tab > 4 - Adjust the priority of several holds using the dropdowns > 5 - Open the network tab in the browser console (F12) to view the timing > 6 - Click 'Update hold(s)' and note the time > 7 - Apply patch, restart all > 8 - Repeat > 9 - Confirm timing is better and numbering updates correctly >10 - Sign off! > >Note: The script adds holds that may be marked as 'lowestPriority' these will always all group at the bottom of the record >--- > C4/Reserves.pm | 33 ++++++++++++++++++--------------- > Koha/Hold.pm | 6 +++--- > Koha/REST/V1/Holds.pm | 2 +- > reserve/modrequest.pl | 6 +++++- > reserve/request.pl | 2 +- > 5 files changed, 28 insertions(+), 21 deletions(-) > >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 624cf08444d..45adc250486 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -60,6 +60,8 @@ BEGIN { > GetMaxPatronHoldsForRecord > > MergeHolds >+ >+ FixPriority > ); > } > >@@ -290,7 +292,7 @@ sub AddReserve { > ChargeReserveFee( $borrowernumber, $reserve_fee, $title ); > } > >- _FixPriority( { biblionumber => $biblionumber } ); >+ FixPriority( { biblionumber => $biblionumber } ); > > # Send e-mail to librarian if syspref is active > if ( C4::Context->preference("emailLibrarianWhenHoldIsPlaced") ) { >@@ -1219,6 +1221,7 @@ sub ModReserve { > my $biblionumber = $params->{'biblionumber'}; > my $cancellation_reason = $params->{'cancellation_reason'}; > my $date = $params->{expirationdate}; >+ my $skip_fixup_priority = $params->{skip_fixup_priority}; > > return if defined $rank && $rank eq "n"; > >@@ -1277,7 +1280,7 @@ sub ModReserve { > } > } > >- _FixPriority( { reserve_id => $reserve_id, rank => $rank } ); >+ FixPriority( { reserve_id => $reserve_id, rank => $rank } ) unless $skip_fixup_priority; > > logaction( 'HOLDS', 'MODIFY', $hold->reserve_id, $hold, undef, $original ) > if C4::Context->preference('HoldsLog'); >@@ -1390,7 +1393,7 @@ sub ModReserveAffect { > > _koha_notify_hold_changed($hold) if $notify_library; > >- _FixPriority( { biblionumber => $biblionumber } ); >+ FixPriority( { biblionumber => $biblionumber } ); > my $item = Koha::Items->find($itemnumber); > if ( $item->location > && $item->location eq 'CART' >@@ -1477,7 +1480,7 @@ sub ModReserveMinusPriority { > $sth_upd->execute( $itemnumber, $reserve_id ); > > # second step update all others reserves >- _FixPriority( { reserve_id => $reserve_id, rank => '0' } ); >+ FixPriority( { reserve_id => $reserve_id, rank => '0' } ); > } > > =head2 IsAvailableForItemLevelRequest >@@ -1634,14 +1637,14 @@ sub AlterPriority { > > if ( $where eq 'up' ) { > return unless $prev_priority; >- _FixPriority( { reserve_id => $reserve_id, rank => $prev_priority } ); >+ FixPriority( { reserve_id => $reserve_id, rank => $prev_priority } ); > } elsif ( $where eq 'down' ) { > return unless $next_priority; >- _FixPriority( { reserve_id => $reserve_id, rank => $next_priority } ); >+ FixPriority( { reserve_id => $reserve_id, rank => $next_priority } ); > } elsif ( $where eq 'top' ) { >- _FixPriority( { reserve_id => $reserve_id, rank => $first_priority } ); >+ FixPriority( { reserve_id => $reserve_id, rank => $first_priority } ); > } elsif ( $where eq 'bottom' ) { >- _FixPriority( { reserve_id => $reserve_id, rank => $last_priority } ); >+ FixPriority( { reserve_id => $reserve_id, rank => $last_priority } ); > } > > Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [ $hold->biblionumber ] } ) >@@ -1666,7 +1669,7 @@ sub ToggleLowestPriority { > my $sth = $dbh->prepare("UPDATE reserves SET lowestPriority = NOT lowestPriority WHERE reserve_id = ?"); > $sth->execute($reserve_id); > >- _FixPriority( { reserve_id => $reserve_id, rank => '999999' } ); >+ FixPriority( { reserve_id => $reserve_id, rank => '999999' } ); > } > > =head2 SuspendAll >@@ -1709,9 +1712,9 @@ sub SuspendAll { > } > } > >-=head2 _FixPriority >+=head2 FixPriority > >- _FixPriority({ >+ FixPriority({ > reserve_id => $reserve_id, > [rank => $rank,] > [ignoreSetLowestRank => $ignoreSetLowestRank] >@@ -1719,7 +1722,7 @@ sub SuspendAll { > > or > >- _FixPriority({ biblionumber => $biblionumber}); >+ FixPriority({ biblionumber => $biblionumber}); > > This routine adjusts the priority of a hold request and holds > on the same bib. >@@ -1740,11 +1743,11 @@ In both cases, holds that have the lowestPriority flag on are have their > priority adjusted to ensure that they remain at the end of the line. > > Note that the ignoreSetLowestRank parameter is meant to be used only >-when _FixPriority calls itself. >+when FixPriority calls itself. > > =cut > >-sub _FixPriority { >+sub FixPriority { > my ($params) = @_; > my $reserve_id = $params->{reserve_id}; > my $rank = $params->{rank} // ''; >@@ -1837,7 +1840,7 @@ sub _FixPriority { > "SELECT reserve_id FROM reserves WHERE lowestPriority = 1 AND biblionumber = ? ORDER BY priority"); > $sth->execute($biblionumber); > while ( my $res = $sth->fetchrow_hashref() ) { >- _FixPriority( >+ FixPriority( > { > reserve_id => $res->{'reserve_id'}, > rank => '999999', >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 66a236a853c..88c8e8a4938 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -917,7 +917,7 @@ sub cancel { > > $self->SUPER::delete(); # Do not add a DELETE log > # now fix the priority on the others.... >- C4::Reserves::_FixPriority( { biblionumber => $self->biblionumber } ); >+ C4::Reserves::FixPriority( { biblionumber => $self->biblionumber } ); > > # and, if desired, charge a cancel fee > if ( $params->{'charge_cancel_fee'} ) { >@@ -1016,7 +1016,7 @@ sub fill { > $self->SUPER::delete(); # Do not add a DELETE log > > # now fix the priority on the others.... >- C4::Reserves::_FixPriority( { biblionumber => $self->biblionumber } ); >+ C4::Reserves::FixPriority( { biblionumber => $self->biblionumber } ); > > if ( C4::Context->preference('HoldFeeMode') eq 'any_time_is_collected' ) { > my $fee = $patron->category->reservefee // 0; >@@ -1105,7 +1105,7 @@ sub revert_found { > logaction( 'HOLDS', 'MODIFY', $self->id, $self, undef, $original ) > if C4::Context->preference('HoldsLog'); > >- C4::Reserves::_FixPriority( { biblionumber => $self->biblionumber } ); >+ C4::Reserves::FixPriority( { biblionumber => $self->biblionumber } ); > > $self->remove_as_hold_group_target(); > >diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm >index 4fdb604a707..c4e9c47c90a 100644 >--- a/Koha/REST/V1/Holds.pm >+++ b/Koha/REST/V1/Holds.pm >@@ -515,7 +515,7 @@ sub update_priority { > > return try { > my $priority = $c->req->json; >- C4::Reserves::_FixPriority( >+ C4::Reserves::FixPriority( > { > reserve_id => $hold->id, > rank => $priority >diff --git a/reserve/modrequest.pl b/reserve/modrequest.pl >index edd2eeaf4ea..c6dfd3c134b 100755 >--- a/reserve/modrequest.pl >+++ b/reserve/modrequest.pl >@@ -28,7 +28,7 @@ use List::MoreUtils qw( uniq ); > use Try::Tiny; > > use C4::Output; >-use C4::Reserves qw( ModReserve ); >+use C4::Reserves qw( ModReserve FixPriority ); > use C4::Auth qw( checkauth ); > use Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue; > >@@ -63,6 +63,7 @@ if ( $op eq 'cud-cancelall' || $op eq 'cud-modifyall' ) { > itemnumber => $itemnumber[$i], > defined $suspend_until ? ( suspend_until => $suspend_until ) : (), > cancellation_reason => $cancellation_reason, >+ skip_fixup_priority => 1, > }; > if ( C4::Context->preference('AllowHoldDateInFuture') ) { > $params->{reservedate} = $reservedates[$i] || undef; >@@ -95,6 +96,9 @@ if ( $op eq 'cud-cancelall' || $op eq 'cud-modifyall' ) { > my @biblio_ids = uniq @biblionumber; > Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => \@biblio_ids } ) > if C4::Context->preference('RealTimeHoldsQueue'); >+ foreach my $biblio_id (@biblio_ids) { >+ FixPriority( { biblionumber => $biblio_id } ); >+ } > } > > my $from = $query->param('from'); >diff --git a/reserve/request.pl b/reserve/request.pl >index a00a6de53e1..41e05088835 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -157,7 +157,7 @@ if ( $op eq 'cud-move' ) { > } > > #Fix the priority on the original record >- C4::Reserves::_FixPriority( { biblionumber => $original_biblionumber } ); >+ C4::Reserves::FixPriority( { biblionumber => $original_biblionumber } ); > > $template->param( > hold_move_successes => \@success_messages, >-- >2.39.5
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 41416
: 190383