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
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Nick Clemens 2025-12-09 18:58:58 +00:00 committed by Lucas Gass
parent 881d0c143c
commit f21255ca54
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
5 changed files with 28 additions and 21 deletions

View File

@ -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',

View File

@ -930,7 +930,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'} ) {
@ -1029,7 +1029,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;
@ -1118,7 +1118,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();

View File

@ -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

View File

@ -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');

View File

@ -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,