Bug 40636: Regression tests
This patch adds regression tests that demonstrate the bug in
CancelExpiredReserves where the holiday logic checks if today is a
holiday instead of checking if the hold's expiration date was a holiday.
The bug causes:
1. Holds that expired on non-holidays to remain active if the cleanup
script runs on a holiday
2. Holds that expired on holidays to be cancelled if the cleanup script
runs on a non-holiday
Test scenarios added:
- Hold expired on Monday (non-holiday), script runs on Tuesday (holiday)
- Hold expired on Wednesday (holiday), script runs on Thursday (non-holiday)
- Hold expired on Friday (non-holiday), script runs on Sunday (holiday)
- Hold expired on Saturday (holiday), script runs on Sunday (holiday)
These tests currently fail, demonstrating the bug exists. When the bug
is fixed, all tests should pass.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 828bf1917d)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This commit is contained in:
parent
790e92ffb9
commit
5e5aba4e57
|
|
@ -1,9 +1,10 @@
|
|||
#!/usr/bin/perl
|
||||
|
||||
use Modern::Perl;
|
||||
use Test::More tests => 5;
|
||||
use Test::More tests => 6;
|
||||
use Test::NoWarnings;
|
||||
use Test::MockModule;
|
||||
use Time::Fake;
|
||||
|
||||
use t::lib::Mocks;
|
||||
use t::lib::TestBuilder;
|
||||
|
|
@ -15,14 +16,14 @@ use Koha::DateUtils qw( dt_from_string );
|
|||
use Koha::Holds;
|
||||
use Koha::Old::Holds;
|
||||
|
||||
my $schema = Koha::Database->new->schema;
|
||||
my $builder = t::lib::TestBuilder->new();
|
||||
my $schema = Koha::Database->new->schema;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
subtest 'CancelExpiredReserves tests incl. holidays' => sub {
|
||||
plan tests => 4;
|
||||
|
||||
my $builder = t::lib::TestBuilder->new();
|
||||
|
||||
t::lib::Mocks::mock_preference( 'ExpireReservesOnHolidays', 0 );
|
||||
|
||||
# Waiting holds could be cancelled only if ExpireReservesMaxPickUpDelay is set to "allow", see bug 19260
|
||||
|
|
@ -119,7 +120,6 @@ subtest 'Test handling of waiting reserves by CancelExpiredReserves' => sub {
|
|||
|
||||
Koha::Holds->delete;
|
||||
|
||||
my $builder = t::lib::TestBuilder->new();
|
||||
my $category = $builder->build( { source => 'Category' } );
|
||||
my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
|
||||
my $item = $builder->build_sample_item;
|
||||
|
|
@ -184,8 +184,6 @@ subtest 'Test handling of waiting reserves by CancelExpiredReserves' => sub {
|
|||
subtest 'Test handling of in transit reserves by CancelExpiredReserves' => sub {
|
||||
plan tests => 2;
|
||||
|
||||
my $builder = t::lib::TestBuilder->new();
|
||||
|
||||
t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 );
|
||||
my $expdate = dt_from_string->add( days => -2 );
|
||||
my $reserve = $builder->build(
|
||||
|
|
@ -225,8 +223,6 @@ subtest 'Test handling of in transit reserves by CancelExpiredReserves' => sub {
|
|||
subtest 'Test handling of cancellation reason if passed' => sub {
|
||||
plan tests => 2;
|
||||
|
||||
my $builder = t::lib::TestBuilder->new();
|
||||
|
||||
my $expdate = dt_from_string->add( days => -2 );
|
||||
my $reserve = $builder->build(
|
||||
{
|
||||
|
|
@ -254,4 +250,154 @@ subtest 'Test handling of cancellation reason if passed' => sub {
|
|||
is( $old_reserve->cancellation_reason, 'EXPIRED', "Hold cancellation_reason was set correctly" );
|
||||
};
|
||||
|
||||
subtest 'Holiday logic edge cases' => sub {
|
||||
|
||||
plan tests => 2;
|
||||
|
||||
subtest 'Hold expired on business day should be cancelled retrospectively when script runs on holiday' => sub {
|
||||
plan tests => 1;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
t::lib::Mocks::mock_preference( 'ExpireReservesOnHolidays', 0 );
|
||||
t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 );
|
||||
|
||||
# Clear any existing holidays from previous tests
|
||||
Koha::Caches->get_instance()->flush_all();
|
||||
|
||||
# Create a library for our tests
|
||||
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
|
||||
|
||||
# The bug scenario: Hold expired on Monday (business day), script runs on Wednesday (holiday)
|
||||
# The hold SHOULD be cancelled because it expired on a business day and Tuesday was a business day
|
||||
# that was missed, so we need retrospective cancellation even though today is a holiday
|
||||
|
||||
my $monday = dt_from_string('2024-01-15'); # Monday (business day, hold expires)
|
||||
my $wednesday = dt_from_string('2024-01-17'); # Wednesday (holiday, script runs now)
|
||||
|
||||
# Create a hold that expired on Monday (business day)
|
||||
my $hold_expired_monday = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Holds',
|
||||
value => {
|
||||
branchcode => $library->branchcode,
|
||||
patron_expiration_date => $monday->ymd,
|
||||
expirationdate => $monday->ymd,
|
||||
cancellationdate => undef,
|
||||
priority => 0,
|
||||
found => 'W',
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
# Make Wednesday a holiday (but Monday and Tuesday are business days)
|
||||
my $wednesday_holiday = $builder->build(
|
||||
{
|
||||
source => 'SpecialHoliday',
|
||||
value => {
|
||||
branchcode => $library->branchcode,
|
||||
day => $wednesday->day,
|
||||
month => $wednesday->month,
|
||||
year => $wednesday->year,
|
||||
title => 'Wednesday Holiday',
|
||||
isexception => 0
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
# Clear cache so holiday is recognized
|
||||
Koha::Caches->get_instance()->flush_all();
|
||||
|
||||
# Set fake time to Wednesday (holiday)
|
||||
Time::Fake->offset( $wednesday->epoch );
|
||||
|
||||
# The hold SHOULD be cancelled because:
|
||||
# 1. It expired on Monday (business day)
|
||||
# 2. Tuesday was a business day that was missed
|
||||
# 3. Even though today (Wednesday) is a holiday, we need retrospective cancellation
|
||||
CancelExpiredReserves();
|
||||
|
||||
my $hold_check = Koha::Holds->find( $hold_expired_monday->id );
|
||||
is(
|
||||
$hold_check, undef,
|
||||
'Hold that expired on business day should be cancelled retrospectively even when script runs on holiday'
|
||||
);
|
||||
|
||||
# Reset time
|
||||
Time::Fake->reset;
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
subtest 'Hold expired on holiday should not be cancelled when script runs on same holiday' => sub {
|
||||
plan tests => 1;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
t::lib::Mocks::mock_preference( 'ExpireReservesOnHolidays', 0 );
|
||||
t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 );
|
||||
|
||||
# Clear any existing holidays from previous tests
|
||||
Koha::Caches->get_instance()->flush_all();
|
||||
|
||||
# Create a library for our tests
|
||||
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
|
||||
|
||||
# Control scenario: Hold expired on Monday (holiday), script runs on Monday (same holiday)
|
||||
# The hold should NOT be cancelled because it expired today and today is a holiday
|
||||
|
||||
my $monday = dt_from_string('2024-01-15'); # Monday (holiday, hold expires and script runs)
|
||||
|
||||
# Create a hold that expired on Monday (holiday)
|
||||
my $hold_expired_monday = $builder->build_object(
|
||||
{
|
||||
class => 'Koha::Holds',
|
||||
value => {
|
||||
branchcode => $library->branchcode,
|
||||
patron_expiration_date => $monday->ymd,
|
||||
expirationdate => $monday->ymd,
|
||||
cancellationdate => undef,
|
||||
priority => 0,
|
||||
found => 'W',
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
# Make Monday a holiday
|
||||
my $monday_holiday = $builder->build(
|
||||
{
|
||||
source => 'SpecialHoliday',
|
||||
value => {
|
||||
branchcode => $library->branchcode,
|
||||
day => $monday->day,
|
||||
month => $monday->month,
|
||||
year => $monday->year,
|
||||
title => 'Monday Holiday',
|
||||
isexception => 0
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
# Clear cache so holiday is recognized
|
||||
Koha::Caches->get_instance()->flush_all();
|
||||
|
||||
# Set fake time to Monday (holiday)
|
||||
Time::Fake->offset( $monday->epoch );
|
||||
|
||||
# The hold should NOT be cancelled because it expired today and today is a holiday
|
||||
CancelExpiredReserves();
|
||||
|
||||
my $hold_check = Koha::Holds->find( $hold_expired_monday->id );
|
||||
ok(
|
||||
$hold_check,
|
||||
'Hold that expired on holiday should not be cancelled when script runs on same holiday'
|
||||
);
|
||||
|
||||
# Reset time
|
||||
Time::Fake->reset;
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
};
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
|
|
|
|||
Loading…
Reference in New Issue