Bugzilla – Attachment 195131 Details for
Bug 41801
FixPriority recursive calls for lowestPriority holds can be removed
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41801: (QA folllow-up) Ensure toggling priority sets correct rank
Bug-41801-QA-folllow-up-Ensure-toggling-priority-s.patch (text/plain), 4.47 KB, created by
Nick Clemens (kidclamp)
on 2026-03-11 15:45:07 UTC
(
hide
)
Description:
Bug 41801: (QA folllow-up) Ensure toggling priority sets correct rank
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2026-03-11 15:45:07 UTC
Size:
4.47 KB
patch
obsolete
>From edb80838ca1d6973a5cddb97f2a81ba5abc82f03 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >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
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 41801
:
192820
|
194985
|
194986
|
194987
|
194988
|
194989
|
194990
| 195131