|
Lines 1-9
Link Here
|
| 1 |
#!/usr/bin/perl |
1 |
#!/usr/bin/perl |
| 2 |
|
2 |
|
| 3 |
use Modern::Perl; |
3 |
use Modern::Perl; |
| 4 |
use Test::More tests => 5; |
4 |
use Test::More tests => 6; |
| 5 |
use Test::NoWarnings; |
5 |
use Test::NoWarnings; |
| 6 |
use Test::MockModule; |
6 |
use Test::MockModule; |
|
|
7 |
use Time::Fake; |
| 7 |
|
8 |
|
| 8 |
use t::lib::Mocks; |
9 |
use t::lib::Mocks; |
| 9 |
use t::lib::TestBuilder; |
10 |
use t::lib::TestBuilder; |
|
Lines 254-257
subtest 'Test handling of cancellation reason if passed' => sub {
Link Here
|
| 254 |
is( $old_reserve->cancellation_reason, 'EXPIRED', "Hold cancellation_reason was set correctly" ); |
255 |
is( $old_reserve->cancellation_reason, 'EXPIRED', "Hold cancellation_reason was set correctly" ); |
| 255 |
}; |
256 |
}; |
| 256 |
|
257 |
|
|
|
258 |
subtest 'Holiday logic edge cases' => sub { |
| 259 |
|
| 260 |
plan tests => 6; |
| 261 |
|
| 262 |
# This test demonstrates the bug where CancelExpiredReserves checks if TODAY is a holiday |
| 263 |
# instead of implementing proper grace period logic. |
| 264 |
# |
| 265 |
# The correct behavior should be: |
| 266 |
# - Don't run cleanup when script runs on holidays (regardless of when holds expired) |
| 267 |
# - Cancel expired holds (including those that expired on holidays) when script runs on business days |
| 268 |
# - This provides a grace period for holds that expired on holidays while ensuring cleanup happens |
| 269 |
|
| 270 |
# Clear any existing holidays from previous tests |
| 271 |
Koha::Caches->get_instance()->flush_all(); |
| 272 |
|
| 273 |
my $builder = t::lib::TestBuilder->new(); |
| 274 |
|
| 275 |
t::lib::Mocks::mock_preference( 'ExpireReservesOnHolidays', 0 ); |
| 276 |
t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 ); |
| 277 |
|
| 278 |
# Create a library for our tests |
| 279 |
my $library = $builder->build( { source => 'Branch' } ); |
| 280 |
my $branchcode = $library->{branchcode}; |
| 281 |
|
| 282 |
# Scenario 1: Hold expired on Monday (non-holiday), script runs on Tuesday (holiday) |
| 283 |
# The hold should NOT be cancelled because we don't run cleanup on holidays |
| 284 |
|
| 285 |
my $monday = dt_from_string('2024-01-15'); # Monday |
| 286 |
my $tuesday = dt_from_string('2024-01-16'); # Tuesday |
| 287 |
|
| 288 |
# Create a hold that expired on Monday |
| 289 |
my $hold_expired_on_monday = $builder->build( |
| 290 |
{ |
| 291 |
source => 'Reserve', |
| 292 |
value => { |
| 293 |
branchcode => $branchcode, |
| 294 |
patron_expiration_date => $monday->ymd, |
| 295 |
expirationdate => $monday->ymd, |
| 296 |
cancellationdate => undef, |
| 297 |
priority => 0, |
| 298 |
found => 'W', |
| 299 |
}, |
| 300 |
} |
| 301 |
); |
| 302 |
|
| 303 |
# Make Tuesday a holiday |
| 304 |
my $tuesday_holiday = $builder->build( |
| 305 |
{ |
| 306 |
source => 'SpecialHoliday', |
| 307 |
value => { |
| 308 |
branchcode => $branchcode, |
| 309 |
day => $tuesday->day, |
| 310 |
month => $tuesday->month, |
| 311 |
year => $tuesday->year, |
| 312 |
title => 'Tuesday Holiday', |
| 313 |
isexception => 0 |
| 314 |
}, |
| 315 |
} |
| 316 |
); |
| 317 |
|
| 318 |
# Clear cache so holiday is recognized |
| 319 |
Koha::Caches->get_instance()->flush_all(); |
| 320 |
|
| 321 |
# Set fake time to Tuesday (the holiday) |
| 322 |
Time::Fake->offset( $tuesday->epoch ); |
| 323 |
|
| 324 |
# The hold should NOT be cancelled because we don't run cleanup on holidays |
| 325 |
# (even though it expired on a non-holiday) |
| 326 |
CancelExpiredReserves(); |
| 327 |
my $hold_check = Koha::Holds->find( $hold_expired_on_monday->{reserve_id} ); |
| 328 |
ok( $hold_check, 'Hold should not be cancelled when script runs on holiday (regardless of when it expired)' ); |
| 329 |
|
| 330 |
# Reset time |
| 331 |
Time::Fake->reset; |
| 332 |
|
| 333 |
# Scenario 2: Hold expired on Wednesday (holiday), script runs on Thursday (non-holiday) |
| 334 |
# The hold SHOULD be cancelled because grace period has ended (running on business day) |
| 335 |
|
| 336 |
my $wednesday = dt_from_string('2024-01-17'); # Wednesday |
| 337 |
my $thursday = dt_from_string('2024-01-18'); # Thursday |
| 338 |
|
| 339 |
# Create a hold that expired on Wednesday |
| 340 |
my $hold_expired_on_wednesday = $builder->build( |
| 341 |
{ |
| 342 |
source => 'Reserve', |
| 343 |
value => { |
| 344 |
branchcode => $branchcode, |
| 345 |
patron_expiration_date => $wednesday->ymd, |
| 346 |
expirationdate => $wednesday->ymd, |
| 347 |
cancellationdate => undef, |
| 348 |
priority => 0, |
| 349 |
found => 'W', |
| 350 |
}, |
| 351 |
} |
| 352 |
); |
| 353 |
|
| 354 |
# Make Wednesday a holiday |
| 355 |
my $wednesday_holiday = $builder->build( |
| 356 |
{ |
| 357 |
source => 'SpecialHoliday', |
| 358 |
value => { |
| 359 |
branchcode => $branchcode, |
| 360 |
day => $wednesday->day, |
| 361 |
month => $wednesday->month, |
| 362 |
year => $wednesday->year, |
| 363 |
title => 'Wednesday Holiday', |
| 364 |
isexception => 0 |
| 365 |
}, |
| 366 |
} |
| 367 |
); |
| 368 |
|
| 369 |
# Clear cache so holiday is recognized |
| 370 |
Koha::Caches->get_instance()->flush_all(); |
| 371 |
|
| 372 |
# Set fake time to Thursday (non-holiday) |
| 373 |
Time::Fake->offset( $thursday->epoch ); |
| 374 |
|
| 375 |
# The hold SHOULD be cancelled because we're running on a business day |
| 376 |
# The hold SHOULD be cancelled because it expired in the past (not today) |
| 377 |
# Even though it expired on a holiday, we're running on a business day |
| 378 |
CancelExpiredReserves(); |
| 379 |
$hold_check = Koha::Holds->find( $hold_expired_on_wednesday->{reserve_id} ); |
| 380 |
is( |
| 381 |
$hold_check, undef, |
| 382 |
'Hold that expired on holiday in the past should be cancelled when script runs on business day' |
| 383 |
); |
| 384 |
|
| 385 |
# Reset time |
| 386 |
Time::Fake->reset; |
| 387 |
|
| 388 |
# Scenario 3: Multiple holds expired on different days, script runs on holiday |
| 389 |
# Only holds that expired on non-holidays should be cancelled |
| 390 |
|
| 391 |
my $friday = dt_from_string('2024-01-19'); # Friday (non-holiday) |
| 392 |
my $saturday = dt_from_string('2024-01-20'); # Saturday (holiday) |
| 393 |
my $sunday = dt_from_string('2024-01-21'); # Sunday (script runs today, also holiday) |
| 394 |
|
| 395 |
# Hold that expired on Friday (non-holiday) |
| 396 |
my $hold_expired_friday = $builder->build( |
| 397 |
{ |
| 398 |
source => 'Reserve', |
| 399 |
value => { |
| 400 |
branchcode => $branchcode, |
| 401 |
patron_expiration_date => $friday->ymd, |
| 402 |
expirationdate => $friday->ymd, |
| 403 |
cancellationdate => undef, |
| 404 |
priority => 0, |
| 405 |
found => 'W', |
| 406 |
}, |
| 407 |
} |
| 408 |
); |
| 409 |
|
| 410 |
# Hold that expired on Saturday (holiday) |
| 411 |
my $hold_expired_saturday = $builder->build( |
| 412 |
{ |
| 413 |
source => 'Reserve', |
| 414 |
value => { |
| 415 |
branchcode => $branchcode, |
| 416 |
patron_expiration_date => $saturday->ymd, |
| 417 |
expirationdate => $saturday->ymd, |
| 418 |
cancellationdate => undef, |
| 419 |
priority => 0, |
| 420 |
found => 'W', |
| 421 |
}, |
| 422 |
} |
| 423 |
); |
| 424 |
|
| 425 |
# Make Saturday and Sunday holidays |
| 426 |
my $saturday_holiday = $builder->build( |
| 427 |
{ |
| 428 |
source => 'SpecialHoliday', |
| 429 |
value => { |
| 430 |
branchcode => $branchcode, |
| 431 |
day => $saturday->day, |
| 432 |
month => $saturday->month, |
| 433 |
year => $saturday->year, |
| 434 |
title => 'Saturday Holiday', |
| 435 |
isexception => 0 |
| 436 |
}, |
| 437 |
} |
| 438 |
); |
| 439 |
|
| 440 |
my $sunday_holiday = $builder->build( |
| 441 |
{ |
| 442 |
source => 'SpecialHoliday', |
| 443 |
value => { |
| 444 |
branchcode => $branchcode, |
| 445 |
day => $sunday->day, |
| 446 |
month => $sunday->month, |
| 447 |
year => $sunday->year, |
| 448 |
title => 'Sunday Holiday', |
| 449 |
isexception => 0 |
| 450 |
}, |
| 451 |
} |
| 452 |
); |
| 453 |
|
| 454 |
# Clear cache so holidays are recognized |
| 455 |
Koha::Caches->get_instance()->flush_all(); |
| 456 |
|
| 457 |
# Set fake time to Sunday (holiday) |
| 458 |
Time::Fake->offset( $sunday->epoch ); |
| 459 |
|
| 460 |
CancelExpiredReserves(); |
| 461 |
|
| 462 |
# Hold that expired on Friday (non-holiday) should NOT be cancelled because script runs on holiday |
| 463 |
my $friday_hold_check = Koha::Holds->find( $hold_expired_friday->{reserve_id} ); |
| 464 |
ok( |
| 465 |
$friday_hold_check, |
| 466 |
'Hold should not be cancelled when script runs on Sunday (holiday), regardless of when it expired' |
| 467 |
); |
| 468 |
|
| 469 |
# Hold that expired on Saturday (holiday) should NOT be cancelled |
| 470 |
my $saturday_hold_check = Koha::Holds->find( $hold_expired_saturday->{reserve_id} ); |
| 471 |
ok( |
| 472 |
$saturday_hold_check, |
| 473 |
'Hold that expired on Saturday (holiday) should not be cancelled when script runs on Sunday (holiday)' |
| 474 |
); |
| 475 |
|
| 476 |
# Reset time |
| 477 |
Time::Fake->reset; |
| 478 |
|
| 479 |
# Scenario 4: Demonstrate grace period - hold expired on holiday, script runs on business day |
| 480 |
# This shows that holds that expired on holidays DO get cancelled when script runs on business days |
| 481 |
|
| 482 |
my $monday_holiday = dt_from_string('2024-01-22'); # Monday (holiday) |
| 483 |
my $tuesday_business = dt_from_string('2024-01-23'); # Tuesday (business day) |
| 484 |
|
| 485 |
# Create a hold that expired on Monday (holiday) |
| 486 |
my $hold_expired_monday_holiday = $builder->build( |
| 487 |
{ |
| 488 |
source => 'Reserve', |
| 489 |
value => { |
| 490 |
branchcode => $branchcode, |
| 491 |
patron_expiration_date => $monday_holiday->ymd, |
| 492 |
expirationdate => $monday_holiday->ymd, |
| 493 |
cancellationdate => undef, |
| 494 |
priority => 0, |
| 495 |
found => 'W', |
| 496 |
}, |
| 497 |
} |
| 498 |
); |
| 499 |
|
| 500 |
# Make Monday a holiday |
| 501 |
my $monday_holiday_record = $builder->build( |
| 502 |
{ |
| 503 |
source => 'SpecialHoliday', |
| 504 |
value => { |
| 505 |
branchcode => $branchcode, |
| 506 |
day => $monday_holiday->day, |
| 507 |
month => $monday_holiday->month, |
| 508 |
year => $monday_holiday->year, |
| 509 |
title => 'Monday Holiday', |
| 510 |
isexception => 0 |
| 511 |
}, |
| 512 |
} |
| 513 |
); |
| 514 |
|
| 515 |
# Clear cache so holiday is recognized |
| 516 |
Koha::Caches->get_instance()->flush_all(); |
| 517 |
|
| 518 |
# Set fake time to Tuesday (business day) |
| 519 |
Time::Fake->offset( $tuesday_business->epoch ); |
| 520 |
|
| 521 |
# The hold SHOULD be cancelled because it expired in the past (not today) |
| 522 |
# Even though it expired on a holiday, we're running on a business day |
| 523 |
CancelExpiredReserves(); |
| 524 |
my $monday_hold_check = Koha::Holds->find( $hold_expired_monday_holiday->{reserve_id} ); |
| 525 |
is( |
| 526 |
$monday_hold_check, undef, |
| 527 |
'Hold that expired on Monday (holiday) should be cancelled when script runs on Tuesday (business day)' |
| 528 |
); |
| 529 |
|
| 530 |
# Reset time |
| 531 |
Time::Fake->reset; |
| 532 |
|
| 533 |
# Scenario 5: CRITICAL BUG - Hold expired on holiday LONG AGO should be cancelled |
| 534 |
# This exposes the bug where holds that expired on holidays are never cancelled |
| 535 |
|
| 536 |
my $christmas_2023 = dt_from_string('2023-12-25'); # Christmas 2023 (holiday) |
| 537 |
my $august_2024 = dt_from_string('2024-08-15'); # August 2024 (business day) |
| 538 |
|
| 539 |
# Create a hold that expired on Christmas 2023 (8 months ago!) |
| 540 |
my $hold_expired_long_ago = $builder->build( |
| 541 |
{ |
| 542 |
source => 'Reserve', |
| 543 |
value => { |
| 544 |
branchcode => $branchcode, |
| 545 |
patron_expiration_date => $christmas_2023->ymd, |
| 546 |
expirationdate => $christmas_2023->ymd, |
| 547 |
cancellationdate => undef, |
| 548 |
priority => 0, |
| 549 |
found => 'W', |
| 550 |
}, |
| 551 |
} |
| 552 |
); |
| 553 |
|
| 554 |
# Make Christmas 2023 a holiday |
| 555 |
my $christmas_holiday = $builder->build( |
| 556 |
{ |
| 557 |
source => 'SpecialHoliday', |
| 558 |
value => { |
| 559 |
branchcode => $branchcode, |
| 560 |
day => $christmas_2023->day, |
| 561 |
month => $christmas_2023->month, |
| 562 |
year => $christmas_2023->year, |
| 563 |
title => 'Christmas', |
| 564 |
isexception => 0 |
| 565 |
}, |
| 566 |
} |
| 567 |
); |
| 568 |
|
| 569 |
# Clear cache so holiday is recognized |
| 570 |
Koha::Caches->get_instance()->flush_all(); |
| 571 |
|
| 572 |
# Set fake time to August 2024 (business day, 8 months later) |
| 573 |
Time::Fake->offset( $august_2024->epoch ); |
| 574 |
|
| 575 |
# This hold SHOULD be cancelled because it expired 8 months ago |
| 576 |
# Even though it expired on a holiday, it should not remain active forever |
| 577 |
CancelExpiredReserves(); |
| 578 |
my $old_hold_check = Koha::Holds->find( $hold_expired_long_ago->{reserve_id} ); |
| 579 |
is( |
| 580 |
$old_hold_check, undef, |
| 581 |
'Hold that expired on holiday 8 months ago should be cancelled (not remain active forever)' |
| 582 |
); |
| 583 |
|
| 584 |
# Reset time |
| 585 |
Time::Fake->reset; |
| 586 |
}; |
| 587 |
|
| 257 |
$schema->storage->txn_rollback; |
588 |
$schema->storage->txn_rollback; |
| 258 |
- |
|
|