Bugzilla – Attachment 183745 Details for
Bug 40296
Bookings that are checked out do not have status updated to completed
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40296: Add unit tests for booking status handling
Bug-40296-Add-unit-tests-for-booking-status-handli.patch (text/plain), 6.87 KB, created by
Martin Renvoize (ashimema)
on 2025-07-03 10:55:36 UTC
(
hide
)
Description:
Bug 40296: Add unit tests for booking status handling
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-07-03 10:55:36 UTC
Size:
6.87 KB
patch
obsolete
>From a6f5d312d8d56e4569c8c6363dba386648643e25 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40296
:
183717
|
183743
|
183744
| 183745