Bugzilla – Attachment 185543 Details for
Bug 40672
`desk_id` not cleared when `revert_found()` called
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40672: Reset desk_id when reverting waiting holds
Bug-40672-Reset-deskid-when-reverting-waiting-hold.patch (text/plain), 6.05 KB, created by
Tomás Cohen Arazi (tcohen)
on 2025-08-19 14:50:29 UTC
(
hide
)
Description:
Bug 40672: Reset desk_id when reverting waiting holds
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2025-08-19 14:50:29 UTC
Size:
6.05 KB
patch
obsolete
>From 15394b04ac19196795271b6742998326bb111e8c Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Tue, 19 Aug 2025 14:34:31 +0000 >Subject: [PATCH] Bug 40672: Reset desk_id when reverting waiting holds > >This patch adds desk_id reset functionality to the revert_found method, >but only for waiting holds where desk_id is actually relevant. > >Changes: >* Modified revert_found() to conditionally reset desk_id >* Only resets desk_id for waiting holds (found=W) >* Leaves desk_id unchanged for in transit (T) and in processing (P) holds >* Added comprehensive test coverage for all scenarios >* Updated POD documentation to explain desk_id behavior > >The logic is conservative and only clears desk_id when it makes sense: >waiting holds have their desk assignment cleared since they are no longer >waiting at a specific desk, while transit and processing holds remain >unchanged as they do not use desk_id. > >To test: >1. Apply patch >2. Run tests: > $ ktd --shell > k$ cd /kohadevbox/koha > k$ prove t/db_dependent/Koha/Hold.t >=> SUCCESS: All tests pass including new desk_id handling tests >3. Verify all hold test suites still pass: > k$ prove t/db_dependent/Koha/Hold*.t >=> SUCCESS: All hold-related tests pass >4. Sign off :-D >--- > Koha/Hold.pm | 5 +++ > t/db_dependent/Koha/Hold.t | 83 +++++++++++++++++++++++++++++++++++++- > 2 files changed, 87 insertions(+), 1 deletion(-) > >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 83715506490..994cc89ab0f 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -896,6 +896,10 @@ sub fill { > Reverts any 'found' hold back to a regular hold with a priority of 1. > This method can revert holds in 'waiting' (W), 'in transit' (T), or 'in processing' (P) status. > >+For waiting holds, the desk_id is also cleared since the hold is no longer waiting >+at a specific desk. For in transit and in processing holds, desk_id remains unchanged >+(typically NULL as these statuses don't set desk_id). >+ > =cut > > sub revert_found { >@@ -928,6 +932,7 @@ sub revert_found { > waitingdate => undef, > expirationdate => $self->patron_expiration_date, > itemnumber => $self->item_level_hold ? $self->itemnumber : undef, >+ ( $self->is_waiting ? ( desk_id => undef ) : () ), > } > )->store( { hold_reverted => 1 } ); > >diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t >index d0ff9a8817f..f7258f1f41b 100755 >--- a/t/db_dependent/Koha/Hold.t >+++ b/t/db_dependent/Koha/Hold.t >@@ -1264,7 +1264,7 @@ subtest 'strings_map() tests' => sub { > > subtest 'revert_found() tests' => sub { > >- plan tests => 5; >+ plan tests => 6; > > subtest 'item-level holds tests' => sub { > >@@ -1591,4 +1591,85 @@ subtest 'revert_found() tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+ subtest 'desk_id handling tests' => sub { >+ >+ plan tests => 12; >+ >+ $schema->storage->txn_begin; >+ >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $desk = $builder->build_object( >+ { >+ class => 'Koha::Desks', >+ value => { branchcode => $library->branchcode } >+ } >+ ); >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { branchcode => $library->branchcode } >+ } >+ ); >+ my $item = $builder->build_sample_item( { library => $library->branchcode } ); >+ >+ my $hold = $builder->build_object( >+ { >+ class => 'Koha::Holds', >+ value => { >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $item->biblionumber, >+ itemnumber => $item->itemnumber, >+ branchcode => $library->branchcode, >+ priority => 1, >+ found => undef, >+ } >+ } >+ ); >+ >+ # Test 1: Waiting hold - desk_id should be cleared >+ $hold->set_waiting( $desk->desk_id ); >+ $hold->discard_changes; >+ is( $hold->desk_id, $desk->desk_id, 'desk_id set for waiting hold' ); >+ ok( $hold->is_waiting, 'Hold is in waiting status' ); >+ >+ $hold->revert_found(); >+ $hold->discard_changes; >+ is( $hold->desk_id, undef, 'desk_id cleared when reverting waiting hold' ); >+ ok( !$hold->is_found, 'Hold is no longer in found status' ); >+ >+ # Test 2: In transit hold with desk_id - desk_id should be preserved >+ $hold->set_transfer(); >+ $hold->desk_id( $desk->desk_id )->store(); # Manually set desk_id >+ $hold->discard_changes; >+ is( $hold->desk_id, $desk->desk_id, 'desk_id manually set for transit hold' ); >+ ok( $hold->is_in_transit, 'Hold is in transit status' ); >+ >+ $hold->revert_found(); >+ $hold->discard_changes; >+ is( $hold->desk_id, $desk->desk_id, 'desk_id preserved when reverting transit hold' ); >+ >+ # Test 3: In processing hold with desk_id - desk_id should be preserved >+ $hold->set_processing(); >+ $hold->desk_id( $desk->desk_id )->store(); # Manually set desk_id >+ $hold->discard_changes; >+ is( $hold->desk_id, $desk->desk_id, 'desk_id manually set for processing hold' ); >+ ok( $hold->is_in_processing, 'Hold is in processing status' ); >+ >+ $hold->revert_found(); >+ $hold->discard_changes; >+ is( $hold->desk_id, $desk->desk_id, 'desk_id preserved when reverting processing hold' ); >+ >+ # Test 4: In transit hold without desk_id - desk_id should remain NULL >+ $hold->set_transfer(); >+ $hold->desk_id(undef)->store(); # Ensure desk_id is NULL >+ $hold->discard_changes; >+ is( $hold->desk_id, undef, 'desk_id is NULL for transit hold without desk_id' ); >+ >+ $hold->revert_found(); >+ $hold->discard_changes; >+ is( $hold->desk_id, undef, 'desk_id remains NULL after reverting transit hold without desk_id' ); >+ >+ $schema->storage->txn_rollback; >+ }; > }; >-- >2.51.0
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 40672
:
185543
|
185567