From 9e71c42a1ebe2cda5e3c796b703b4c52426e43d9 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 (cherry picked from commit e4ef4c5e3d3d328479677b586a0e5c3f1b423b20) Signed-off-by: Matt Blenkinsop --- 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 8c42fd2cb2..669564ae2b 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 5e2e9f77a5..aa46feac46 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1704,6 +1704,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}) { @@ -1782,6 +1785,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 ea5632d9d1..c57f52aeb1 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -232,6 +232,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( @@ -296,11 +306,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 5e4e0c89b5..c70b14d270 100755 --- a/t/db_dependent/Koha/Item.t +++ b/t/db_dependent/Koha/Item.t @@ -213,7 +213,7 @@ subtest 'bundle_host tests' => sub { }; subtest 'add_to_bundle tests' => sub { - plan tests => 10; + plan tests => 11; $schema->storage->txn_begin; @@ -248,6 +248,13 @@ subtest 'add_to_bundle tests' => sub { 'Exception thrown if you try to add a bundle host to a bundle item'; my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + 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', @@ -271,19 +278,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