From b3048305d898ef3962699351644651b3ab492fa1 Mon Sep 17 00:00:00 2001 From: Nick Clemens 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