From edb80838ca1d6973a5cddb97f2a81ba5abc82f03 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 11 Mar 2026 15:44:35 +0000 Subject: [PATCH] Bug 41801: (QA folllow-up) Ensure toggling priority sets correct rank --- C4/Reserves.pm | 16 +++++++++++----- Koha/REST/V1/Holds.pm | 2 +- t/db_dependent/Reserves.t | 14 +++++++++++--- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index bd1dbd57b80..adf53279336 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1576,21 +1576,27 @@ sub AlterPriority { =head2 ToggleLowestPriority - ToggleLowestPriority( $borrowernumber, $biblionumber ); + ToggleLowestPriority( $reserve_id, $is_lowestPriority ); -This function sets the lowestPriority field to true if is false, and false if it is true. +This function sets the lowestPriority field to true if is false, and false if it is true +and then reorders the holds on the record. + +A hold toggled to lowest priority will go the bottom. +A hold toggled to NOT lowest priority will go to the bottom of other non-lowest priority holds. =cut sub ToggleLowestPriority { - my ($reserve_id) = @_; + my ( $reserve_id, $is_lowestPriority ) = @_; my $dbh = C4::Context->dbh; + my $rank = $is_lowestPriority ? undef : '999999'; + 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 => $rank } ); } =head2 SuspendAll @@ -1744,7 +1750,7 @@ sub FixPriority { } # if index exists in array then move it to new position - if ( $key > -1 && $rank ne 'del' && $rank > 0 ) { + if ( $key > -1 && $rank ne 'del' && $rank && $rank > 0 ) { my $new_rank = $rank - 1; # $new_rank is what you want the new index to be in the array my $moving_item = splice( @priority, $key, 1 ); $new_rank = scalar @priority if $new_rank > scalar @priority; diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm index 90fdc38f7c8..cb715ffdfa2 100644 --- a/Koha/REST/V1/Holds.pm +++ b/Koha/REST/V1/Holds.pm @@ -665,7 +665,7 @@ sub lowest_priority { unless $hold; return try { - C4::Reserves::ToggleLowestPriority($hold_id); + C4::Reserves::ToggleLowestPriority( $hold_id, $hold->lowestPriority ); $hold->discard_changes; # refresh return $c->render( status => 200, openapi => $hold_id ); } catch { diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t index 9841241d965..988717b26c6 100755 --- a/t/db_dependent/Reserves.t +++ b/t/db_dependent/Reserves.t @@ -2148,7 +2148,7 @@ subtest 'Bug 40866: AddReserve override JSON logging' => sub { subtest 'FixPriority() lowestPriority tests' => sub { - plan tests => 14; + plan tests => 16; $schema->storage->txn_begin; @@ -2173,8 +2173,8 @@ subtest 'FixPriority() lowestPriority tests' => sub { } # Mark holds 4 and 5 as lowestPriority; initial priorities should be 1..5 - ToggleLowestPriority( $reserve_ids[3] ); # hold 4 → lowestPriority, pushed to bottom - ToggleLowestPriority( $reserve_ids[4] ); # hold 5 → lowestPriority, pushed to bottom + ToggleLowestPriority( $reserve_ids[3], 0 ); # hold 4 → lowestPriority, pushed to bottom + ToggleLowestPriority( $reserve_ids[4], 0 ); # hold 5 → lowestPriority, pushed to bottom my @holds = map { Koha::Holds->find($_) } @reserve_ids; @@ -2209,6 +2209,14 @@ subtest 'FixPriority() lowestPriority tests' => sub { isnt( $holds[3]->priority, $p4_before, 'lowestPriority hold moved within lowest-priority group' ); ok( $holds[3]->priority > $holds[2]->priority, 'Moved lowestPriority hold still below non-lowest holds' ); + ToggleLowestPriority( $reserve_ids[4], 1 ); # Untoggle lowest priority for hold 4 + $holds[4]->discard_changes; + is( $holds[4]->priority, 4, 'Hold is moved to the lowest of non-priority holds when toggled off' ); + + ToggleLowestPriority( $reserve_ids[1], 0 ); # Untoggle lowest priority for hold 4 + $holds[1]->discard_changes; + is( $holds[1]->priority, 5, 'Hold is moved to the lowest of low priority holds when toggled on' ); + # When ALL holds are lowestPriority, movement is unconstrained my $biblio2 = $builder->build_sample_biblio; my @all_low_ids; -- 2.39.5