From ebf7b6471fcf9d622d48cf6e3c1999148c2f113e Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 26 Dec 2022 16:01:42 -0300 Subject: [PATCH] Bug 31797: Add DELETE /items/:item_id endpoint This patch adds the mentioned endpoint. The controller relies on Koha::Item->safe_to_delete for checks and uses `safe_delete` as additem.pl does. The required permissions are edit_catalogue. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/items.t => SUCCESS: Tests pass! 3. Play with item deletion using a REST tool like Postman => SUCCESS: All works as expected 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi Signed-off-by: Lucas Gass Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- Koha/REST/V1/Items.pm | 66 ++++++++++++++++++++++++++- api/v1/swagger/paths/items.yaml | 52 ++++++++++++++++++++++ t/db_dependent/api/v1/items.t | 79 ++++++++++++++++++++++++++++++++- 3 files changed, 195 insertions(+), 2 deletions(-) diff --git a/Koha/REST/V1/Items.pm b/Koha/REST/V1/Items.pm index 515fcd91bf..70abdfeeb2 100644 --- a/Koha/REST/V1/Items.pm +++ b/Koha/REST/V1/Items.pm @@ -58,7 +58,6 @@ sub list { }; } - =head3 get Controller function that handles retrieving a single Koha::Item @@ -83,6 +82,71 @@ sub get { }; } +=head3 delete + +Controller function that handles deleting a single Koha::Item + +=cut + +sub delete { + my $c = shift->openapi->valid_input or return; + + return try { + my $item = Koha::Items->find($c->validation->param('item_id')); + unless ( $item ) { + return $c->render( + status => 404, + openapi => { error => 'Item not found'} + ); + } + + my $safe_to_delete = $item->safe_to_delete; + + if ( !$safe_to_delete ) { + + # Pick the first error, if any + my ( $error ) = grep { $_->type eq 'error' } @{ $safe_to_delete->messages }; + + unless ( $error ) { + Koha::Exception->throw('Koha::Item->safe_to_delete returned false but carried no error message'); + } + + my $errors = { + book_on_loan => { code => 'checked_out', description => 'The item is checked out' }, + book_reserved => { code => 'found_hold', description => 'Waiting or in-transit hold for the item' }, + last_item_for_hold => { code => 'last_item_for_hold', description => 'The item is the last one on a record on which a biblio-level hold is placed' }, + linked_analytics => { code => 'linked_analytics', description => 'The item has linked analytic records' }, + not_same_branch => { code => 'not_same_branch', description => 'The item is blocked by independent branches' }, + }; + + if ( any { $error->message eq $_ } keys %{$errors} ) { + + my $code = $error->message; + + return $c->render( + status => 409, + openapi => { + error => $errors->{ $code }->{description}, + error_code => $errors->{ $code }->{code}, + } + ); + } else { + Koha::Exception->throw( 'Koha::Patron->safe_to_delete carried an unexpected message: ' . $error->message ); + } + } + + $item->safe_delete; + + return $c->render( + status => 204, + openapi => q{} + ); + } + catch { + $c->unhandled_exception($_); + }; +} + =head3 pickup_locations Method that returns the possible pickup_locations for a given item diff --git a/api/v1/swagger/paths/items.yaml b/api/v1/swagger/paths/items.yaml index 7c947f6e3f..e68d98bd41 100644 --- a/api/v1/swagger/paths/items.yaml +++ b/api/v1/swagger/paths/items.yaml @@ -93,6 +93,58 @@ x-koha-authorization: permissions: catalogue: "1" + delete: + x-mojo-to: Items#delete + operationId: deleteItem + tags: + - items + summary: Delete item + parameters: + - $ref: "../swagger.yaml#/parameters/item_id_pp" + consumes: + - application/json + produces: + - application/json + responses: + "204": + description: Deleted item + "400": + description: Missing or wrong parameters + schema: + $ref: "../swagger.yaml#/definitions/error" + "403": + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + "404": + description: Item not found + schema: + $ref: "../swagger.yaml#/definitions/error" + "409": + description: | + Conflict. Possible `error_code` attribute values: + + * book_on_loan: The item is checked out + * book_reserved: Waiting or in-transit hold for the item + * last_item_for_hold: The item is the last one on a record on which a biblio-level hold is placed + * linked_analytics: The item has linked analytic records + * not_same_branch: The item is blocked by independent branches + schema: + $ref: "../swagger.yaml#/definitions/error" + "500": + description: | + Internal server error. Possible `error_code` attribute values: + + * `internal_server_error` + schema: + $ref: "../swagger.yaml#/definitions/error" + "503": + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + editcatalogue: edit_catalogue "/items/{item_id}/bundled_items": post: x-mojo-to: Items#add_to_bundle diff --git a/t/db_dependent/api/v1/items.t b/t/db_dependent/api/v1/items.t index 5dd047937d..aa012c532c 100755 --- a/t/db_dependent/api/v1/items.t +++ b/t/db_dependent/api/v1/items.t @@ -19,7 +19,8 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 4; +use Test::MockModule; use Test::Mojo; use Test::Warn; @@ -187,6 +188,82 @@ subtest 'get() tests' => sub { $schema->storage->txn_rollback; }; +subtest 'delete() tests' => sub { + + plan tests => 23; + + $schema->storage->txn_begin; + + my $fail = 0; + my $expected_error; + + # we want to control all the safe_to_delete use cases + my $item_class = Test::MockModule->new('Koha::Item'); + $item_class->mock( 'safe_to_delete', sub { + if ( $fail ) { + return Koha::Result::Boolean->new(0)->add_message({ message => $expected_error }); + } + else { + return Koha::Result::Boolean->new(1); + } + }); + + my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2**9 } # catalogue flag = 2 + } + ); + my $password = 'thePassword123'; + $librarian->set_password( { password => $password, skip_validation => 1 } ); + my $userid = $librarian->userid; + + my $item = $builder->build_sample_item; + + my $errors = { + book_on_loan => { code => 'checked_out', description => 'The item is checked out' }, + book_reserved => { code => 'found_hold', description => 'Waiting or in-transit hold for the item' }, + last_item_for_hold => { code => 'last_item_for_hold', description => 'The item is the last one on a record on which a biblio-level hold is placed' }, + linked_analytics => { code => 'linked_analytics', description => 'The item has linked analytic records' }, + not_same_branch => { code => 'not_same_branch', description => 'The item is blocked by independent branches' }, + }; + + $fail = 1; + + foreach my $error_code ( keys %{$errors} ) { + + $expected_error = $error_code; + + $t->delete_ok( "//$userid:$password@/api/v1/items/" . $item->id ) + ->status_is(409) + ->json_is( + { error => $errors->{$error_code}->{description}, + error_code => $errors->{$error_code}->{code}, + } + ); + } + + $expected_error = 'unknown_error'; + $t->delete_ok( "//$userid:$password@/api/v1/items/" . $item->id ) + ->status_is(500, 'unhandled error case generated default unhandled exception message') + ->json_is( + { error => 'Something went wrong, check Koha logs for details.', + error_code => 'internal_server_error', + } + ); + + $fail = 0; + + $t->delete_ok("//$userid:$password@/api/v1/items/" . $item->id) + ->status_is(204, 'SWAGGER3.2.4') + ->content_is('', 'SWAGGER3.3.4'); + + $t->delete_ok("//$userid:$password@/api/v1/items/" . $item->id) + ->status_is(404); + + $schema->storage->txn_rollback; +}; + subtest 'pickup_locations() tests' => sub { plan tests => 16; -- 2.20.1