From a6f5d312d8d56e4569c8c6363dba386648643e25 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 3 Jul 2025 11:21:20 +0100 Subject: [PATCH] Bug 40296: Add unit tests for booking status handling This patch adds test coverage for the new booking status management in AddIssue: Test scenarios covered: - Patron checking out their own booked item (marks as completed) - Different patron checking out booked item during actual booking period (cancels booking) - Checkout conflicts only during lead period (preserves booking, respects librarian decision) - Checkout period encompassing entire booking period (cancels booking) - No booking present (normal operation) Tests ensure proper booking lifecycle management and validate that librarian override scenarios work as intended. To test: 1. Apply all patches 2. Run: prove t/db_dependent/Circulation.t 3. Verify all booking status handling scenarios pass The test plans now provide: - Step-by-step instructions for manual testing - Multiple scenarios covering different use cases - Clear expectations for what should happen in each case - Practical workflow that staff would actually follow - Edge cases like lead period conflicts and overrides --- t/db_dependent/Circulation.t | 127 ++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index e10f7a01c9c..263b5acb5b1 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -19,7 +19,7 @@ use Modern::Perl; use utf8; use Test::NoWarnings; -use Test::More tests => 76; +use Test::More tests => 77; use Test::Exception; use Test::MockModule; use Test::Deep qw( cmp_deeply ); @@ -6882,6 +6882,131 @@ subtest 'AddIssue records staff who checked out item if appropriate' => sub { ); }; +subtest 'AddIssue | booking status handling' => sub { + plan tests => 5; + + my $schema = Koha::Database->schema; + $schema->storage->txn_begin; + + my $patron1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron2 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $pickup_library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $item = $builder->build_sample_item( { bookable => 1 } ); + + # Test 1: Patron checks out item with their own booking - should mark as completed + my $booking1 = Koha::Booking->new( + { + patron_id => $patron1->borrowernumber, + pickup_library_id => $pickup_library->branchcode, + item_id => $item->itemnumber, + biblio_id => $item->biblio->biblionumber, + start_date => dt_from_string(), + end_date => dt_from_string()->add( days => 7 ), + } + )->store(); + + my $issue1 = AddIssue( $patron1, $item->barcode, dt_from_string()->add( days => 7 ) ); + $booking1->discard_changes(); + + is( $booking1->status, 'completed', "Patron's own booking marked as completed when item checked out" ); + + # Clean up for next test + AddReturn( $item->barcode, $pickup_library->branchcode ); + $booking1->delete(); + + # Test 2: Another patron checks out item with different patron's booking overlapping actual booking period - should cancel booking + my $booking2 = Koha::Booking->new( + { + patron_id => $patron1->borrowernumber, + pickup_library_id => $pickup_library->branchcode, + item_id => $item->itemnumber, + biblio_id => $item->biblio->biblionumber, + start_date => dt_from_string()->add( days => 1 ), + end_date => dt_from_string()->add( days => 5 ), + } + )->store(); + + my $issue2 = AddIssue( $patron2, $item->barcode, dt_from_string()->add( days => 3 ) ); + $booking2->discard_changes(); + + is( + $booking2->status, 'cancelled', + "Another patron's booking cancelled when checkout overlaps actual booking period" + ); + + # Clean up for next test + AddReturn( $item->barcode, $pickup_library->branchcode ); + $booking2->delete(); + + # Test 3: Another patron checks out item - checkout period only in lead period - should NOT cancel booking + # First set up a lead period rule + Koha::CirculationRules->set_rules( + { + branchcode => '*', + itemtype => $item->effective_itemtype, + rules => { + bookings_lead_period => 3, + }, + } + ); + + my $booking3 = Koha::Booking->new( + { + patron_id => $patron1->borrowernumber, + pickup_library_id => $pickup_library->branchcode, + item_id => $item->itemnumber, + biblio_id => $item->biblio->biblionumber, + start_date => dt_from_string()->add( days => 5 ), + end_date => dt_from_string()->add( days => 10 ), + } + )->store(); + + # Issue that would return before booking starts (only conflicts due to lead period) + my $issue3 = AddIssue( $patron2, $item->barcode, dt_from_string()->add( days => 4 ) ); + $booking3->discard_changes(); + + is( + $booking3->status, 'new', + "Another patron's booking NOT cancelled when checkout only conflicts with lead period" + ); + + # Clean up for next test + AddReturn( $item->barcode, $pickup_library->branchcode ); + $booking3->delete(); + + # Test 4: Edge case - checkout period starts before booking and ends after booking (encompasses booking) + my $booking4 = Koha::Booking->new( + { + patron_id => $patron1->borrowernumber, + pickup_library_id => $pickup_library->branchcode, + item_id => $item->itemnumber, + biblio_id => $item->biblio->biblionumber, + start_date => dt_from_string()->add( days => 2 ), + end_date => dt_from_string()->add( days => 4 ), + } + )->store(); + + # Issue that encompasses the entire booking period + my $issue4 = AddIssue( $patron2, $item->barcode, dt_from_string()->add( days => 6 ) ); + $booking4->discard_changes(); + + is( + $booking4->status, 'cancelled', + "Another patron's booking cancelled when checkout encompasses entire booking period" + ); + + # Clean up for next test + AddReturn( $item->barcode, $pickup_library->branchcode ); + $booking4->delete(); + + # Test 5: No booking found - should not cause errors + my $issue5 = AddIssue( $patron1, $item->barcode, dt_from_string()->add( days => 7 ) ); + + ok( $issue5, "AddIssue works normally when no booking is found" ); + + $schema->storage->txn_rollback; +}; + subtest "Item's onloan value should be set if checked out item is checked out to a different patron" => sub { plan tests => 2; -- 2.50.0