Attachment 163367 Details for Bug 36329 – Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType

[patch] Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType

Bug-36329-Make-POST-transferlimitsbatch-honor-Bran.patch (text/plain), 9.63 KB, created by Kyle M Hall (khall) on 2024-03-18 17:48:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2024-03-18 17:48:31 UTC

Size: 9.63 KB

patch

obsolete

>From ae9766fb3751ce127d90b6069133949b7e76a510 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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 <david@davidnind.com>
>Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
>---
> 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.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 36329: 163232 | 163233 | 163234 | 163280 | 163281 | 163282 | 163365 | 163366 | 163367 | 163453