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

Bug 40671: Refactor Koha::Hold->revert_waiting into revert_found 

This patch refactors the revert_waiting method to handle all found statuses
instead of just waiting holds, making it more versatile and consistent.

Changes:
* Renamed method from revert_waiting() to revert_found()
* Expanded functionality to handle all found statuses (W, T, P)
* Updated exception logic to check is_found instead of is_waiting
* Changed exception message from hold_not_waiting to hold_not_found
* Added comprehensive tests for all found statuses
* Updated POD documentation to reflect new behavior

The method now reverts any hold with a found status back to a regular
hold with priority 1, regardless of whether it was waiting, in transit,
or in processing.

To test:
1. Apply the patches
2. Run tests:
   $ ktd --shell
  k$ cd /kohadevbox/koha
  k$ prove t/db_dependent/Koha/Hold.t
=> SUCCESS: All tests pass including new subtests for T and P statuses
3. Verify all hold test suites pass:
  k$ prove t/db_dependent/Koha/Hold*.t
=> SUCCESS: All hold-related tests pass
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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 13:58:38 +00:00 committed by Lucas Gass
parent 4066d2a4fc
commit bd9fa395e5
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
2 changed files with 121 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
Koha/Hold.pm
View File

@ -889,19 +889,20 @@ sub fill {
return $self;
}
=head3 revert_waiting
=head3 revert_found
$hold->revert_waiting();
$hold->revert_found();
Reverts a 'waiting' hold back to a regular hold with a priority of 1.
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.
=cut
sub revert_waiting {
sub revert_found {
my ( $self, $params ) = @_;
Koha::Exceptions::InvalidStatus->throw( invalid_status => 'hold_not_waiting' )
unless $self->is_waiting;
Koha::Exceptions::InvalidStatus->throw( invalid_status => 'hold_not_found' )
unless $self->is_found;
$self->_result->result_source->schema->txn_do(
sub {
@ -919,7 +920,7 @@ sub revert_waiting {
{ no_triggers => 1 }
);
## Fix up the currently waiting reserve
## Fix up the currently found reserve
$self->set(
{
priority => 1,

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

@ -1262,9 +1262,9 @@ subtest 'strings_map() tests' => sub {
$schema->txn_rollback;
};
subtest 'revert_waiting() tests' => sub {
subtest 'revert_found() tests' => sub {
plan tests => 3;
plan tests => 5;
subtest 'item-level holds tests' => sub {
@ -1318,8 +1318,8 @@ subtest 'revert_waiting() tests' => sub {
is( $hold->priority, 0, "'priority' set to 0" );
ok( $hold->is_waiting, 'Hold set to waiting' );
# Revert the waiting status
$hold->revert_waiting();
# Revert the found status
$hold->revert_found();
is( $hold->waitingdate, undef, "'waitingdate' reset" );
ok( !$hold->is_waiting, 'Hold no longer set to waiting' );
@ -1342,20 +1342,20 @@ subtest 'revert_waiting() tests' => sub {
$hold->set_waiting();
# Revert the waiting status, RealTimeHoldsQueue => shouldn't add a test
$hold->revert_waiting();
# Revert the found status, RealTimeHoldsQueue => shouldn't add a test
$hold->revert_found();
my $log_count_after =
Koha::ActionLogs->search( { module => 'HOLDS', action => 'MODIFY', object => $hold->reserve_id } )->count;
is( $log_count, $log_count_after, "No logging is added for ->revert_waiting() when HoldsLog is disabled" );
is( $log_count, $log_count_after, "No logging is added for ->revert_found() when HoldsLog is disabled" );
# Set as 'processing' to test the exception behavior
$hold->found('P');
throws_ok { $hold->revert_waiting() }
# Set as regular hold (not found) to test the exception behavior
$hold->found(undef);
throws_ok { $hold->revert_found() }
'Koha::Exceptions::InvalidStatus',
"Hold is not in 'waiting' status, exception thrown";
"Hold is not in 'found' status, exception thrown";
is( $@->invalid_status, 'hold_not_waiting', "'invalid_status' set the right value" );
is( $@->invalid_status, 'hold_not_found', "'invalid_status' set the right value" );
$schema->storage->txn_rollback;
};
@ -1399,8 +1399,8 @@ subtest 'revert_waiting() tests' => sub {
is( $hold->priority, 0, "'priority' set to 0" );
ok( $hold->is_waiting, 'Hold set to waiting' );
# Revert the waiting status
$hold->revert_waiting();
# Revert the found status
$hold->revert_found();
is( $hold->waitingdate, undef, "'waitingdate' reset" );
ok( !$hold->is_waiting, 'Hold no longer set to waiting' );
@ -1478,7 +1478,7 @@ subtest 'revert_waiting() tests' => sub {
$hold->set_waiting()->discard_changes();
is( $hold->priority, 0, "'priority' set to 0" );
$hold->revert_waiting()->discard_changes();
$hold->revert_found()->discard_changes();
is( $hold->priority, 1, "'priority' set to 1" );
my $holds = $item->biblio->holds;
@ -1493,4 +1493,102 @@ subtest 'revert_waiting() tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'in transit holds tests' => sub {
plan tests => 8;
$schema->storage->txn_begin;
my $item = $builder->build_sample_item;
my $patron = $builder->build_object(
{
class => 'Koha::Patrons',
value => { branchcode => $item->homebranch }
}
);
# Create item-level hold
my $hold = Koha::Holds->find(
AddReserve(
{
branchcode => $item->homebranch,
borrowernumber => $patron->borrowernumber,
biblionumber => $item->biblionumber,
priority => 1,
itemnumber => $item->itemnumber,
}
)
);
# Mark it in transit
$hold->set_transfer();
is( $hold->priority, 0, "'priority' set to 0" );
ok( $hold->is_in_transit, 'Hold set to in transit' );
is( $hold->found, 'T', "'found' set to 'T'" );
# Revert the found status
$hold->revert_found();
ok( !$hold->is_in_transit, 'Hold no longer set to in transit' );
ok( !$hold->is_found, 'Hold no longer has found status' );
is( $hold->found, undef, "'found' reset to undef" );
is( $hold->priority, 1, "'priority' set to 1" );
is(
$hold->itemnumber, $item->itemnumber,
'Itemnumber should not be removed when the found status is reverted'
);
$schema->storage->txn_rollback;
};
subtest 'in processing holds tests' => sub {
plan tests => 8;
$schema->storage->txn_begin;
my $item = $builder->build_sample_item;
my $patron = $builder->build_object(
{
class => 'Koha::Patrons',
value => { branchcode => $item->homebranch }
}
);
# Create item-level hold
my $hold = Koha::Holds->find(
AddReserve(
{
branchcode => $item->homebranch,
borrowernumber => $patron->borrowernumber,
biblionumber => $item->biblionumber,
priority => 1,
itemnumber => $item->itemnumber,
}
)
);
# Mark it in processing
$hold->set_processing();
is( $hold->priority, 0, "'priority' set to 0" );
ok( $hold->is_in_processing, 'Hold set to in processing' );
is( $hold->found, 'P', "'found' set to 'P'" );
# Revert the found status
$hold->revert_found();
ok( !$hold->is_in_processing, 'Hold no longer set to in processing' );
ok( !$hold->is_found, 'Hold no longer has found status' );
is( $hold->found, undef, "'found' reset to undef" );
is( $hold->priority, 1, "'priority' set to 1" );
is(
$hold->itemnumber, $item->itemnumber,
'Itemnumber should not be removed when the found status is reverted'
);
$schema->storage->txn_rollback;
};
};