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

Bug 31391: (QA follow-up) Add missing test coverage for hold-to-recall conversion 

Two code paths introduced by this bug had no direct test coverage:

- C4::Reserves::ModReserve's title-level branch of the new
  `rank => "recall"` conversion (only the item-level branch was
  tested).
- The `hold_convert` bypass in Koha::Item::can_be_recalled, which
  skips the "patron already has a hold on this item" check so an
  existing hold can be converted to a recall for the same patron.

No production code changes here, coverage only.

Test plan:
1) prove t/db_dependent/Holds.t t/db_dependent/Koha/Item.t - all pass
2) Confirm the new tests actually exercise the code they claim to:
   reverting the `unless ( $params->{hold_convert} )` guard in
   Koha/Item.pm locally makes the new Item.t subtest fail, as
   expected.

Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize 2026-09-15 11:15:04 +01:00 committed by Pedro Amorim
parent cd30d78c0e
commit 0b7d3b856a
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
2 changed files with 156 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
t/db_dependent/Holds.t
View File

@ -7,7 +7,7 @@ use t::lib::TestBuilder;
use C4::Context;
use Test::More tests => 75;
use Test::More tests => 76;
use Test::NoWarnings;
use Test::Exception;
@ -2246,3 +2246,101 @@ subtest 'ModReserve to convert a hold to a recall' => sub {
$schema->storage->txn_rollback;
};
subtest 'ModReserve to convert a title-level hold to a recall' => sub {
plan tests => 2;
$schema->storage->txn_begin;
my $category = $builder->build( { source => 'Category' } );
my $branch = $builder->build( { source => 'Branch' } )->{branchcode};
my $biblio = $builder->build_sample_biblio( { itemtype => 'DUMMY' } );
my $itype = $builder->build( { source => 'Itemtype' } );
my $item = $builder->build_sample_item(
{ library => $branch, biblionumber => $biblio->biblionumber, itype => $itype->{itemtype} } );
my $patron = Koha::Patron->new(
{
firstname => 'my firstname',
surname => 'whatever surname',
categorycode => $category->{categorycode},
branchcode => $branch,
email => 'name@email.com',
}
)->store;
my $borrowernumber = $patron->borrowernumber;
my $patron2 = Koha::Patron->new(
{
firstname => 'my firstname',
surname => 'whatever surname',
categorycode => $category->{categorycode},
branchcode => $branch,
email => 'name2@email.com',
}
)->store;
t::lib::Mocks::mock_preference( "UseRecalls", 'opac_and_staff' );
t::lib::Mocks::mock_userenv( { branchcode => $branch } );
C4::Circulation::AddIssue( $patron2, $item->barcode );
Koha::CirculationRules->set_rules(
{
branchcode => undef,
categorycode => undef,
itemtype => undef,
rules => {
recalls_allowed => 5,
recalls_per_record => 1,
on_shelf_recalls => 'any',
},
}
);
my $notice = Koha::Notice::Template->new(
{
name => 'Hold cancellation',
module => 'reserves',
code => 'HOLD_CANCELLATION',
title => 'Hold cancelled',
content => 'Your hold was cancelled.',
message_transport_type => 'email',
branchcode => q{},
}
)->store();
# Title-level hold: no itemnumber supplied
my $reserve_id = AddReserve(
{
branchcode => $branch,
borrowernumber => $borrowernumber,
biblionumber => $biblio->biblionumber,
priority => C4::Reserves::CalculatePriority( $biblio->biblionumber ),
}
);
my $hold = Koha::Holds->find($reserve_id);
ok( !$hold->item_level_hold, 'Hold is title-level, as a control on the rest of the test' );
my $before_recalls = Koha::Recalls->search->filter_by_current->count;
ModReserve(
{
reserve_id => $hold->id,
expirationdate => '1981-06-10',
priority => 99,
rank => "recall",
}
);
my $after_recalls = Koha::Recalls->search->filter_by_current->count;
$hold = Koha::Holds->find($reserve_id);
ok(
!defined($hold) && $after_recalls == $before_recalls + 1,
'Title-level hold was cancelled and converted to a recall'
);
$schema->storage->txn_rollback;
};

58
t/db_dependent/Koha/Item.t
View File

@ -21,7 +21,7 @@ use Modern::Perl;
use utf8;
use Test::NoWarnings;
use Test::More tests => 42;
use Test::More tests => 43;
use Test::Exception;
use Test::MockModule;
use Test::Warn;
@ -3211,6 +3211,62 @@ subtest 'Recalls tests' => sub {
$schema->storage->txn_rollback;
};
subtest 'can_be_recalled() hold_convert param tests' => sub {
plan tests => 2;
$schema->storage->txn_begin;
my $item1 = $builder->build_sample_item;
my $biblio = $item1->biblio;
my $branchcode = $item1->holdingbranch;
my $patron1 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $branchcode } } );
my $patron2 = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $branchcode } } );
t::lib::Mocks::mock_userenv( { patron => $patron1 } );
t::lib::Mocks::mock_preference( 'UseRecalls', 'opac' );
Koha::CirculationRules->set_rules(
{
branchcode => $branchcode,
categorycode => $patron1->categorycode,
itemtype => $item1->effective_itemtype,
rules => {
recalls_allowed => 1,
recalls_per_record => 1,
on_shelf_recalls => 'any',
},
}
);
C4::Circulation::AddIssue( $patron2, $item1->barcode );
my $reserve_id = C4::Reserves::AddReserve(
{
branchcode => $branchcode, borrowernumber => $patron1->borrowernumber,
biblionumber => $item1->biblionumber, itemnumber => $item1->itemnumber
}
);
is(
$item1->can_be_recalled( { patron => $patron1 } ), 0,
"Can't recall item if patron has already reserved it"
);
is(
$item1->can_be_recalled( { patron => $patron1, hold_convert => 1 } ), 1,
"Can recall item if patron has already reserved it, when converting that hold to a recall"
);
C4::Reserves::ModReserve(
{
rank => 'del', reserve_id => $reserve_id, branchcode => $branchcode, itemnumber => $item1->itemnumber,
borrowernumber => $patron1->borrowernumber, biblionumber => $item1->biblionumber
}
);
$schema->storage->txn_rollback;
};
subtest 'item_group() tests' => sub {
plan tests => 4;