From 900f6b2ce078affd3b3c3457c486fa9bcd2d8439 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 15 Mar 2024 09:56:22 -0300 Subject: [PATCH] Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType This patch adds tests for the different cases of `BranchTransferLimitsType`. It also adds tests for the situation of the consumer sending both limit criterias on the request. The controller gets adjusted for this new behavior and the spec gets documentation added about this. Bonus: tests are added the right guidelines code, and BranchTransferLimitsType gets mocked to avoid failures due to existing data. To test: 1. Apply this patches 2. Run: $ ktd --shell k$ qa => SUCCESS: All green, and tests pass! 3. Sign off :-D Signed-off-by: David Nind Signed-off-by: Kyle M Hall Fixed a typo in one of the return messages Signed-off-by: Katrin Fischer (cherry picked from commit e846641eddb98d63f2fb9b78b7fe5fce00cd8569) Signed-off-by: Fridolin Somers (cherry picked from commit f39baa0b9814a7a90c1819291284fcbe4027b2c5) Signed-off-by: Lucas Gass --- Koha/REST/V1/TransferLimits.pm | 24 ++++++- t/db_dependent/api/v1/transfer_limits.t | 83 +++++++++++++++---------- 2 files changed, 73 insertions(+), 34 deletions(-) diff --git a/Koha/REST/V1/TransferLimits.pm b/Koha/REST/V1/TransferLimits.pm index b38c3e87eb..a9abcb1e80 100644 --- a/Koha/REST/V1/TransferLimits.pm +++ b/Koha/REST/V1/TransferLimits.pm @@ -128,6 +128,28 @@ sub batch_add { return try { my $params = $c->req->json; + if ( $params->{item_type} && $params->{collection_code} ) { + return $c->render( + status => 400, + openapi => { + error => "You can only pass 'item_type' or 'collection_code' at a time", + } + ); + } + + if ( ( C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' && $params->{collection_code} ) + || ( C4::Context->preference("BranchTransferLimitsType") eq 'ccode' && $params->{item_type} ) ) + { + return $c->render( + status => 409, + openapi => { + error => $params->{collection_code} + ? "You passed 'collection_code' but configuration expects 'item_type'" + : "You passed 'item_type' but configuration expects 'collection_code'" + } + ); + } + my ( @from_branches, @to_branches ); if ( $params->{from_library_id} ) { @from_branches = ( $params->{from_library_id} ); @@ -195,7 +217,7 @@ sub batch_delete { Koha::Item::Transfer::Limits->search($search_params)->delete; - return $c->render( status => 204, openapi => ''); + return $c->render( status => 204, openapi => q{} ); } catch { $c->unhandled_exception($_); diff --git a/t/db_dependent/api/v1/transfer_limits.t b/t/db_dependent/api/v1/transfer_limits.t index 0862d1e0ca..0db8f52ba8 100755 --- a/t/db_dependent/api/v1/transfer_limits.t +++ b/t/db_dependent/api/v1/transfer_limits.t @@ -158,67 +158,86 @@ subtest 'delete() tests' => sub { }; subtest 'batch_add() and batch_delete() tests' => sub { - plan tests => 26; + + plan tests => 38; $schema->storage->txn_begin; + t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); + Koha::Item::Transfer::Limits->delete; #my $library = $builder->build_object({ class => 'Koha::Libraries' }); - my $library = Koha::Libraries->search->next; + my $library = Koha::Libraries->search->next; my $itemtype = Koha::ItemTypes->search->next; - my $authorized_patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { flags => 1 } - }); + my $authorized_patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 1 } + } + ); my $password = 'thePassword123'; - $authorized_patron->set_password({ password => $password, skip_validation => 1 }); + $authorized_patron->set_password( { password => $password, skip_validation => 1 } ); my $auth_userid = $authorized_patron->userid; - my $unauthorized_patron = $builder->build_object({ - class => 'Koha::Patrons', - value => { flags => 4 } - }); - $unauthorized_patron->set_password({ password => $password, skip_validation => 1 }); + my $unauthorized_patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 4 } + } + ); + $unauthorized_patron->set_password( { password => $password, skip_validation => 1 } ); my $unauth_userid = $unauthorized_patron->userid; - my $limit_hashref = { - item_type => $itemtype->id - }; + my $limit_hashref = { item_type => $itemtype->id }; # Unauthorized attempt to write - $t->post_ok( "//$unauth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is(403); + $t->post_ok( "//$unauth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref )->status_is(403); # Authorized attempt to write invalid data - my $limit_with_invalid_field = {'invalid' => 'invalid'}; + my $limit_with_invalid_field = { 'invalid' => 'invalid' }; $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_with_invalid_field ) - ->status_is(400) - ->json_is( + ->status_is(400)->json_is( "/errors" => [ { message => "Properties not allowed: invalid.", path => "/body" } ] - ); + ); + + # Create all combinations of to/from libraries + $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => + { item_type => 'X', collection_code => 'Y' } )->status_is(400) + ->json_is( '/error' => "Only one of 'item_type' and 'collecion_code' can be passed at a time" ); + + t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'ccode' ); + + # Create all combinations of to/from libraries + $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => { item_type => 'X' } ) + ->status_is(409)->json_is( '/error' => "You passed 'item_type' but configuration expects 'collection_code'" ); + + t::lib::Mocks::mock_preference( 'BranchTransferLimitsType', 'itemtype' ); + + # Create all combinations of to/from libraries + $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => { collection_code => 'X' } ) + ->status_is(409)->json_is( '/error' => "You passed 'collection_code' but configuration expects 'item_type'" ); # Create all combinations of to/from libraries $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is( 201, 'SWAGGER3.2.1' ) - ->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' ); + ->status_is( 201, 'SWAGGER3.2.1' )->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' ); my $limits = Koha::Item::Transfer::Limits->search; my $libraries_count = Koha::Libraries->search->count; - is( $limits->count, $libraries_count * ($libraries_count - 1 ), "Created the correct number of limits" ); + is( $limits->count, $libraries_count * ( $libraries_count - 1 ), "Created the correct number of limits" ); # Delete all combinations of to/from libraries $t->delete_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is( 204, 'SWAGGER3.2.1' ); + ->status_is( 204, 'SWAGGER3.2.4' )->content_is( '', 'SWAGGER3.3.4' ); $limits = Koha::Item::Transfer::Limits->search; @@ -227,16 +246,15 @@ subtest 'batch_add() and batch_delete() tests' => sub { # Create all combinations of 'to' libraries $limit_hashref->{to_library_id} = $library->id; $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is( 201, 'SWAGGER3.2.1' ) - ->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' ); + ->status_is( 201, 'SWAGGER3.2.1' )->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' ); $limits = Koha::Item::Transfer::Limits->search; - is( $limits->count, $libraries_count - 1 , "Created the correct number of limits" ); + is( $limits->count, $libraries_count - 1, "Created the correct number of limits" ); # Delete all combinations of 'to' libraries $t->delete_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is( 204, 'SWAGGER3.2.1' ); + ->status_is( 204, 'SWAGGER3.2.4' )->content_is( '', 'SWAGGER3.3.4' ); $limits = Koha::Item::Transfer::Limits->search; @@ -248,17 +266,16 @@ subtest 'batch_add() and batch_delete() tests' => sub { delete $limit_hashref->{to_library_id}; $limit_hashref->{from_library_id} = $library->id; $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is( 201, 'SWAGGER3.2.1' ) - ->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' ); + ->status_is( 201, 'SWAGGER3.2.1' )->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' ); $limits = Koha::Item::Transfer::Limits->search; $libraries_count = Koha::Libraries->search->count; - is( $limits->count, $libraries_count - 1 , "Created the correct number of limits" ); + is( $limits->count, $libraries_count - 1, "Created the correct number of limits" ); # Delete all combinations of 'from' libraries $t->delete_ok( "//$auth_userid:$password@/api/v1/transfer_limits/batch" => json => $limit_hashref ) - ->status_is( 204, 'SWAGGER3.2.1' ); + ->status_is( 204, 'SWAGGER3.2.4' )->content_is( '', 'SWAGGER3.3.4' ); $limits = Koha::Item::Transfer::Limits->search; -- 2.39.5