From 89fc242e97a83fc83748e4a5480c648ffe6dcb66 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 7 Feb 2024 17:58:11 +0000 Subject: [PATCH] Bug 35248: Add tests for Koha::Booking->store This patch adds tests for the Koha::Booking->store method. Test plan 1) Run t/db_dependent/Koha/Booking.t and confirm they all pass Signed-off-by: Nick Clemens Signed-off-by: Katrin Fischer --- Koha/Booking.pm | 9 +- t/db_dependent/Koha/Booking.t | 190 +++++++++++++++++++++++++++++++++- 2 files changed, 194 insertions(+), 5 deletions(-) diff --git a/Koha/Booking.pm b/Koha/Booking.pm index 2aabe72a95..44e41ab485 100644 --- a/Koha/Booking.pm +++ b/Koha/Booking.pm @@ -74,7 +74,10 @@ sub item { =head3 store -Booking specific store method to catch booking clashes +Booking specific store method to catch booking clashes and ensure we have an item assigned + +We assume that if an item is passed, it's bookability has already been checked. This is to allow +overrides in the future. =cut @@ -121,6 +124,8 @@ sub store { } ); + # FIXME: We should be able to combine the above two functions into one + # Assign item at booking time if ( !$self->item_id ) { $self->item_id( @@ -133,8 +138,6 @@ sub store { ); } - # FIXME: We should be able to combine the above two functions into one - $self = $self->SUPER::store; } ); diff --git a/t/db_dependent/Koha/Booking.t b/t/db_dependent/Koha/Booking.t index 78bb21d39d..77f4006920 100755 --- a/t/db_dependent/Koha/Booking.t +++ b/t/db_dependent/Koha/Booking.t @@ -20,7 +20,11 @@ use Modern::Perl; use utf8; -use Test::More tests => 1; +use Test::More tests => 2; + +use Test::Exception; + +use Koha::DateUtils qw( dt_from_string ); use t::lib::TestBuilder; use t::lib::Mocks; @@ -76,7 +80,7 @@ subtest 'Relation accessor tests' => sub { plan tests => 3; $schema->storage->txn_begin; - my $item = $builder->build_object( { class => "Koha::Items" } ); + my $item = $builder->build_sample_item( { bookable => 1 } ); my $booking = $builder->build_object( { class => 'Koha::Bookings', value => { item_id => $item->itemnumber } } ); @@ -93,5 +97,187 @@ subtest 'Relation accessor tests' => sub { $schema->storage->txn_rollback; }; +}; + +subtest 'store() tests' => sub { + plan tests => 12; + $schema->storage->txn_begin; + + my $patron = $builder->build_object( { class => "Koha::Patrons" } ); + my $biblio = $builder->build_sample_biblio(); + my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + my $start_0 = dt_from_string->subtract( days => 2 )->truncate( to => 'day' ); + my $end_0 = $start_0->clone()->add( days => 6 ); + + my $deleted_item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber } ); + $deleted_item->delete; + + my $wrong_item = $builder->build_sample_item(); + + my $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $deleted_item->itemnumber, + start_date => $start_0, + end_date => $end_0 + } + ); + + throws_ok { $booking->store() } 'Koha::Exceptions::Object::FKConstraint', + 'Throws exception if passed a deleted item'; + + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $wrong_item->itemnumber, + start_date => $start_0, + end_date => $end_0 + } + ); + + throws_ok { $booking->store() } 'Koha::Exceptions::Object::FKConstraint', + "Throws exception if item passed doesn't match biblio passed"; + + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_1->itemnumber, + start_date => $start_0, + end_date => $end_0 + } + ); + + # FIXME: Should this be allowed if an item is passed specifically? + throws_ok { $booking->store() } 'Koha::Exceptions::Booking::Clash', + 'Throws exception when there are no items marked bookable for this biblio'; + + $item_1->bookable(1)->store(); + $booking->store(); + ok( $booking->in_storage, 'First booking on item 1 stored OK' ); + + # Bookings + # ✓ Item 1 |----| + # ✗ Item 1 |----| + + my $start_1 = dt_from_string->truncate( to => 'day' ); + my $end_1 = $start_1->clone()->add( days => 6 ); + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_1->itemnumber, + start_date => $start_1, + end_date => $end_1 + } + ); + throws_ok { $booking->store } 'Koha::Exceptions::Booking::Clash', + 'Throws exception when passed booking start_date falls inside another booking for the item passed'; + + # Bookings + # ✓ Item 1 |----| + # ✗ Item 1 |----| + $start_1 = dt_from_string->subtract( days => 4 )->truncate( to => 'day' ); + $end_1 = $start_1->clone()->add( days => 6 ); + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_1->itemnumber, + start_date => $start_1, + end_date => $end_1 + } + ); + throws_ok { $booking->store } 'Koha::Exceptions::Booking::Clash', + 'Throws exception when passed booking end_date falls inside another booking for the item passed'; + + # Bookings + # ✓ Item 1 |----| + # ✗ Item 1 |--------| + $start_1 = dt_from_string->subtract( days => 4 )->truncate( to => 'day' ); + $end_1 = $start_1->clone()->add( days => 10 ); + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_1->itemnumber, + start_date => $start_1, + end_date => $end_1 + } + ); + throws_ok { $booking->store } 'Koha::Exceptions::Booking::Clash', + 'Throws exception when passed booking dates would envelope another booking for the item passed'; + + # Bookings + # ✓ Item 1 |----| + # ✗ Item 1 |--| + $start_1 = dt_from_string->truncate( to => 'day' ); + $end_1 = $start_1->clone()->add( days => 4 ); + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_1->itemnumber, + start_date => $start_1, + end_date => $end_1 + } + ); + throws_ok { $booking->store } 'Koha::Exceptions::Booking::Clash', + 'Throws exception when passed booking dates would fall wholly inside another booking for the item passed'; + + my $item_2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 1 } ); + my $item_3 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, bookable => 0 } ); + + # Bookings + # ✓ Item 1 |----| + # ✓ Item 2 |--| + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + item_id => $item_2->itemnumber, + start_date => $start_1, + end_date => $end_1 + } + )->store(); + ok( + $booking->in_storage, + 'First booking on item 2 stored OK, even though it would overlap with a booking on item 1' + ); + + # Bookings + # ✓ Item 1 |----| + # ✓ Item 2 |--| + # ✘ Any |--| + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + start_date => $start_1, + end_date => $end_1 + } + ); + throws_ok { $booking->store } 'Koha::Exceptions::Booking::Clash', + 'Throws exception when passed booking dates would fall wholly inside all existing bookings when no item specified'; + + # Bookings + # ✓ Item 1 |----| + # ✓ Item 2 |--| + # ✓ Any |--| + $start_1 = dt_from_string->add( days => 5 )->truncate( to => 'day' ); + $end_1 = $start_1->clone()->add( days => 4 ); + $booking = Koha::Booking->new( + { + patron_id => $patron->borrowernumber, + biblio_id => $biblio->biblionumber, + start_date => $start_1, + end_date => $end_1 + } + )->store(); + ok( $booking->in_storage, 'Booking stored OK when item not specified and the booking slot is available' ); + ok( $booking->item_id, 'An item was assigned to the booking' ); + $schema->storage->txn_rollback; }; -- 2.39.5