From 40b9c8e29cce0e1b5001db397a22fc32d92ce812 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 8 Jun 2026 15:08:50 +0100 Subject: [PATCH] Bug 41898: Update tests for new booking status lifecycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bookings.t: rewrite filter_by_active to verify status-based filtering; new/issued are active, cancelled/completed are not; past-window 'new' bookings (uncollected) remain active - Booking.t: checkout relation uses 'issued'; old_checkout uses 'completed'; clash detection covers issued transition; notice test updated - Circulation.t: AddIssue sets 'issued'; AddReturn sets 'completed'; plan expanded to cover the full checkout → return lifecycle Sponsored-by: OpenFifth Sponsored-by: Westminster City Council Signed-off-by: Hannah Dunne-Howrie Signed-off-by: Paul Derscheid Signed-off-by: Pedro Amorim --- t/db_dependent/Circulation.t | 13 +++-- t/db_dependent/Koha/Booking.t | 14 +++-- t/db_dependent/Koha/Bookings.t | 102 +++++++++------------------------ 3 files changed, 44 insertions(+), 85 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 63883147b9..6b99440cf5 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -7259,8 +7259,8 @@ subtest 'AddIssue records staff who checked out item if appropriate' => sub { ); }; -subtest 'AddIssue | booking status handling' => sub { - plan tests => 5; +subtest 'AddIssue/AddReturn | booking status handling' => sub { + plan tests => 6; my $schema = Koha::Database->schema; $schema->storage->txn_begin; @@ -7270,7 +7270,7 @@ subtest 'AddIssue | booking status handling' => sub { 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 + # Test 1: Patron checks out item with their own booking - should mark as issued my $booking1 = Koha::Booking->new( { patron_id => $patron1->borrowernumber, @@ -7285,10 +7285,13 @@ subtest 'AddIssue | booking status handling' => sub { 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" ); + is( $booking1->status, 'issued', "Patron's own booking marked as issued when item checked out" ); - # Clean up for next test + # Return the item - booking should now be completed AddReturn( $item->barcode, $pickup_library->branchcode ); + $booking1->discard_changes(); + is( $booking1->status, 'completed', "Booking marked as completed when item returned" ); + $booking1->delete(); # Test 2: Another patron checks out item with different patron's booking overlapping actual booking period - should cancel booking diff --git a/t/db_dependent/Koha/Booking.t b/t/db_dependent/Koha/Booking.t index 250a121e71..e1f19495c3 100755 --- a/t/db_dependent/Koha/Booking.t +++ b/t/db_dependent/Koha/Booking.t @@ -138,7 +138,7 @@ subtest 'Relation accessor tests' => sub { value => { patron_id => $patron->borrowernumber, item_id => $item->itemnumber, - status => 'completed' + status => 'issued' } } ); @@ -671,7 +671,7 @@ subtest 'store() tests' => sub { $booking->update( { - status => 'completed', + status => 'issued', } ); @@ -1178,7 +1178,7 @@ subtest 'Integration test: Full optimal selection workflow' => sub { }; subtest 'store() skips clash detection on terminal status transition' => sub { - plan tests => 3; + plan tests => 4; $schema->storage->txn_begin; my $patron = $builder->build_object( { class => "Koha::Patrons" } ); @@ -1211,13 +1211,17 @@ subtest 'store() skips clash detection on terminal status transition' => sub { # false clash when there is only one bookable item. C4::Circulation::AddIssue( $patron, $non_bookable_item->barcode ); - # Without the fix, transitioning to 'completed' runs clash + # Without the fix, transitioning to 'issued' runs clash # detection which sees the non-bookable checkout and throws # Koha::Exceptions::Booking::Clash → 500 error. + lives_ok { $booking->status('issued')->store() } + 'Transition to issued skips clash detection'; + + # Transition to completed (item returned) also skips clash detection lives_ok { $booking->status('completed')->store() } 'Transition to completed skips clash detection'; - # Cancellation from completed is also a terminal transition + # Cancellation is also a terminal transition lives_ok { $booking->status('cancelled')->store() } 'Transition to cancelled skips clash detection'; diff --git a/t/db_dependent/Koha/Bookings.t b/t/db_dependent/Koha/Bookings.t index 24bae2d54b..25d984dc0c 100755 --- a/t/db_dependent/Koha/Bookings.t +++ b/t/db_dependent/Koha/Bookings.t @@ -34,113 +34,65 @@ my $builder = t::lib::TestBuilder->new; subtest 'filter_by_active' => sub { - plan tests => 7; + plan tests => 5; $schema->storage->txn_begin; - my $biblio = $builder->build_sample_biblio; - my $start_ago = dt_from_string->subtract( hours => 1 ); - my $start_hour = dt_from_string->add( hours => 1 ); - my $start_day = dt_from_string->add( days => 1 ); - my $end_ago = dt_from_string->subtract( minutes => 1 ); - my $end_hour = dt_from_string->add( hours => 1 ); - my $end_day = dt_from_string->add( days => 1 ); + my $biblio = $builder->build_sample_biblio; + my $start_ago = dt_from_string->subtract( days => 5 ); + my $end_ago = dt_from_string->subtract( days => 1 ); + my $start_day = dt_from_string->add( days => 1 ); + my $end_day = dt_from_string->add( days => 5 ); + $builder->build_object( { class => 'Koha::Bookings', - value => { - biblio_id => $biblio->biblionumber, - start_date => $start_ago, - end_date => $end_hour, - status => 'new' - - } + value => + { biblio_id => $biblio->biblionumber, start_date => $start_ago, end_date => $end_day, status => 'new' } } ); - is( $biblio->bookings->filter_by_active->count, 1, 'Booking started in past, ending in future is counted' ); + is( $biblio->bookings->filter_by_active->count, 1, 'Active new booking is open' ); + + $builder->build_object( + { + class => 'Koha::Bookings', + value => + { biblio_id => $biblio->biblionumber, start_date => $start_ago, end_date => $end_ago, status => 'new' } + } + ); + is( $biblio->bookings->filter_by_active->count, 2, 'Uncollected past-window new booking is still open' ); $builder->build_object( { class => 'Koha::Bookings', value => { - biblio_id => $biblio->biblionumber, - start_date => $start_ago, - end_date => $end_ago, - status => 'new' - + biblio_id => $biblio->biblionumber, start_date => $start_ago, end_date => $end_day, status => 'issued' } } ); - is( $biblio->bookings->filter_by_active->count, 1, 'Booking started in past, ended now is not counted' ); + is( $biblio->bookings->filter_by_active->count, 3, 'Issued booking is open' ); $builder->build_object( { class => 'Koha::Bookings', value => { - biblio_id => $biblio->biblionumber, - start_date => $start_hour, - end_date => $end_hour, - status => 'new' - + biblio_id => $biblio->biblionumber, start_date => $start_day, end_date => $end_day, + status => 'cancelled' } } ); - is( $biblio->bookings->filter_by_active->count, 2, 'Booking starting soon, ending soon is still counted' ); + is( $biblio->bookings->filter_by_active->count, 3, 'Cancelled booking is not open' ); $builder->build_object( { class => 'Koha::Bookings', value => { - biblio_id => $biblio->biblionumber, - start_date => $start_day, - end_date => $end_day, - status => 'new' + biblio_id => $biblio->biblionumber, start_date => $start_ago, end_date => $end_ago, + status => 'completed' } } ); - is( $biblio->bookings->filter_by_active->count, 3, 'Booking starting tomorrow, ending tomorrow is counted' ); - - $builder->build_object( - { - class => 'Koha::Bookings', - value => { - biblio_id => $biblio->biblionumber, - start_date => $start_day, - end_date => $end_ago, - status => 'new' - } - } - ); - is( - $biblio->bookings->filter_by_active->count, 3, - 'EDGE CASE: Booking starting in future, already ended is not counted - should be impossible, but good check' - ); - - $builder->build_object( - { - class => 'Koha::Bookings', - value => { - biblio_id => $biblio->biblionumber, - start_date => $start_day, - end_date => $end_day, - status => 'cancelled' - } - } - ); - is( $biblio->bookings->filter_by_active->count, 3, 'Cancelled bookings are not counted' ); - - $builder->build_object( - { - class => 'Koha::Bookings', - value => { - biblio_id => $biblio->biblionumber, - start_date => $start_day, - end_date => $end_day, - status => 'completed' - } - } - ); - is( $biblio->bookings->filter_by_active->count, 3, 'Completed bookings are not counted' ); + is( $biblio->bookings->filter_by_active->count, 3, 'Completed booking is not open' ); $schema->storage->txn_rollback; };