From 5da8ca335782849edd1e3c9c78a4c824c7a60120 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 --- 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 b38c3e87eb7..292282e4c3c 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 => "Only one of 'item_type' and 'collecion_code' can be passed 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 0862d1e0ca7..0db8f52ba8a 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.44.0