gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Tomas Cohen Arazi 2025-08-19 14:34:31 +00:00 committed by Lucas Gass
parent 25b9895c8d
commit 2da5c04c2d
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 87 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
Koha/Hold.pm
View File

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

83
t/db_dependent/Koha/Hold.t
View File

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