From e4ef4c5e3d3d328479677b586a0e5c3f1b423b20 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 25 May 2023 11:37:38 +0200 Subject: [PATCH] Bug 33817: Enforce server-side Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 384de4a74adb6af1296ee2ad53f1c9ad1dd0c2eb) Signed-off-by: Martin Renvoize --- Koha/Exceptions/Item/Bundle.pm | 4 ++++ Koha/Item.pm | 9 +++++++++ Koha/REST/V1/Items.pm | 34 +++++++++++++++++++++++++++++----- t/db_dependent/Koha/Item.t | 27 +++++++++++++++++++++------ 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/Koha/Exceptions/Item/Bundle.pm b/Koha/Exceptions/Item/Bundle.pm index fb8c46d22f..3d48c43819 100644 --- a/Koha/Exceptions/Item/Bundle.pm +++ b/Koha/Exceptions/Item/Bundle.pm @@ -27,6 +27,10 @@ use Exception::Class ( isa => 'Koha::Exceptions::Item::Bundle', description => "A bundle cannot be added to a bundle", }, + 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut' => { + isa => 'Koha::Exceptions::Item::Bundle', + description => 'Someone tried to add an item to a checked out bundle', + }, 'Koha::Exceptions::Item::Bundle::ItemIsCheckedOut' => { isa => 'Koha::Exceptions::Item::Bundle', description => 'Someone tried to add a checked out item to a bundle', diff --git a/Koha/Item.pm b/Koha/Item.pm index 6012846c52..fd6d987abf 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1701,6 +1701,9 @@ sub add_to_bundle { try { $schema->txn_do( sub { + + Koha::Exceptions::Item::Bundle::BundleIsCheckedOut->throw if $self->checkout; + my $checkout = $bundle_item->checkout; if ($checkout) { unless ($options->{force_checkin}) { @@ -1786,6 +1789,12 @@ Remove this item from any bundle it may have been attached to. sub remove_from_bundle { my ($self) = @_; + my $bundle_host = $self->bundle_host; + + return 0 unless $bundle_host; # Should not we raise an exception here? + + Koha::Exceptions::Item::Bundle::BundleIsCheckedOut->throw if $bundle_host->checkout; + my $bundle_item_rs = $self->_result->item_bundles_item; if ( $bundle_item_rs ) { $bundle_item_rs->delete; diff --git a/Koha/REST/V1/Items.pm b/Koha/REST/V1/Items.pm index 938ffcd659..0a64e26f89 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -327,6 +327,16 @@ sub add_to_bundle { } ); } + elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut' ) + { + return $c->render( + status => 409, + openapi => { + error => 'Bundle is checked out', + error_code => 'bundle_checked_out' + } + ); + } elsif ( ref($_) eq 'Koha::Exceptions::Item::Bundle::ItemIsCheckedOut' ) { return $c->render( @@ -400,11 +410,25 @@ sub remove_from_bundle { ); } - $bundle_item->remove_from_bundle; - return $c->render( - status => 204, - openapi => q{} - ); + return try { + $bundle_item->remove_from_bundle; + return $c->render( + status => 204, + openapi => q{} + ); + } catch { + if ( ref($_) eq 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut' ) { + return $c->render( + status => 409, + openapi => { + error => 'Bundle is checked out', + error_code => 'bundle_checked_out' + } + ); + } else { + $c->unhandled_exception($_); + } + }; } 1; diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t index 66c4d16830..f63d3677d1 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -214,7 +214,7 @@ subtest 'bundle_host tests' => sub { }; subtest 'add_to_bundle tests' => sub { - plan tests => 11; + plan tests => 12; $schema->storage->txn_begin; @@ -263,6 +263,13 @@ subtest 'add_to_bundle tests' => sub { 'Koha::Exceptions::Item::Bundle::IsBundle', 'Exception thrown if you try to add a bundle host to a bundle item'; + C4::Circulation::AddIssue( $patron->unblessed, $host_item->barcode ); + throws_ok { $host_item->add_to_bundle($bundle_item2) } + 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut', + 'Exception thrown if you try to add an item to a checked out bundle'; + C4::Circulation::AddReturn( $host_item->barcode, $host_item->homebranch ); + $host_item->discard_changes; + C4::Circulation::AddIssue( $patron->unblessed, $bundle_item2->barcode ); throws_ok { $host_item->add_to_bundle($bundle_item2) } 'Koha::Exceptions::Item::Bundle::ItemIsCheckedOut', @@ -286,19 +293,27 @@ subtest 'add_to_bundle tests' => sub { }; subtest 'remove_from_bundle tests' => sub { - plan tests => 3; + plan tests => 4; $schema->storage->txn_begin; my $host_item = $builder->build_sample_item(); my $bundle_item1 = $builder->build_sample_item({ notforloan => 1 }); - $schema->resultset('ItemBundle') - ->create( - { host => $host_item->itemnumber, item => $bundle_item1->itemnumber } ); + $host_item->add_to_bundle($bundle_item1); + + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + t::lib::Mocks::mock_userenv( { branchcode => $patron->branchcode } ); + + C4::Circulation::AddIssue( $patron->unblessed, $host_item->barcode ); + throws_ok { $bundle_item1->remove_from_bundle } + 'Koha::Exceptions::Item::Bundle::BundleIsCheckedOut', + 'Exception thrown if you try to add an item to a checked out bundle'; + my ( $doreturn, $messages, $issue ) = C4::Circulation::AddReturn( $host_item->barcode, $host_item->homebranch ); + $bundle_item1->discard_changes; is($bundle_item1->remove_from_bundle(), 1, 'remove_from_bundle returns 1 when item is removed from a bundle'); is($bundle_item1->notforloan, 0, 'remove_from_bundle resets notforloan to 0'); - $bundle_item1 = $bundle_item1->get_from_storage; + $bundle_item1->discard_changes; is($bundle_item1->remove_from_bundle(), 0, 'remove_from_bundle returns 0 when item is not in a bundle'); $schema->storage->txn_rollback; -- 2.39.5