Attachment 112928 Details for Bug 26633 – Bug 26633: (QA follow-up) Tests on error condition

[patch] Bug 26633: (QA follow-up) Tests on error condition

Bug-26633-QA-follow-up-Tests-on-error-condition.patch (text/plain), 2.62 KB, created by Tomás Cohen Arazi (tcohen) on 2020-11-03 14:16:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-11-03 14:16:30 UTC

Size: 2.62 KB

patch

obsolete

>From fc9ecf366304a90ff0c3c5ac8891aaa96b677342 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Tue, 3 Nov 2020 11:06:58 -0300
>Subject: [PATCH] Bug 26633: (QA follow-up) Tests on error condition
>
>This patch highlights a problem with the try/catch conditions in the
>ad() controlled method. Tests are added, and the controller is fixed to
>reflect the right behaviour.
>
>To test:
>1. Apply this patch
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/api/v1/transfer_limits.t
>=> SUCCESS: Tests pass! 409 is returned as appropriate!
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/REST/V1/TransferLimits.pm          |  6 +++---
> t/db_dependent/api/v1/transfer_limits.t | 10 +++++++++-
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/Koha/REST/V1/TransferLimits.pm b/Koha/REST/V1/TransferLimits.pm
>index c70baf7d8f2..8ce82911926 100644
>--- a/Koha/REST/V1/TransferLimits.pm
>+++ b/Koha/REST/V1/TransferLimits.pm
>@@ -72,7 +72,7 @@ sub add {
>         if ( Koha::Item::Transfer::Limits->search( $transfer_limit->attributes_from_api($params) )->count == 0 ) {
>             $transfer_limit->store;
>         } else {
>-            Koha::Exceptions::TransferLimit::Duplicate->throw()
>+            Koha::Exceptions::TransferLimit::Duplicate->throw();
>         }
> 
>         return $c->render(
>@@ -81,10 +81,10 @@ sub add {
>         );
>     }
>     catch {
>-        if ( blessed $_ && $_->isa('Koha::Exceptions::Object::DuplicateID') ) {
>+        if ( blessed $_ && $_->isa('Koha::Exceptions::TransferLimit::Duplicate') ) {
>             return $c->render(
>                 status  => 409,
>-                openapi => { error => $_->error, conflict => $_->duplicate_id }
>+                openapi => { error => "$_" }
>             );
>         }
> 
>diff --git a/t/db_dependent/api/v1/transfer_limits.t b/t/db_dependent/api/v1/transfer_limits.t
>index 35eec83baa7..6e05c4e1a57 100755
>--- a/t/db_dependent/api/v1/transfer_limits.t
>+++ b/t/db_dependent/api/v1/transfer_limits.t
>@@ -62,7 +62,7 @@ subtest 'list() tests' => sub {
> 
> subtest 'add() tests' => sub {
> 
>-    plan tests => 8;
>+    plan tests => 11;
> 
>     $schema->storage->txn_begin;
> 
>@@ -109,6 +109,14 @@ subtest 'add() tests' => sub {
>       ->status_is( 201, 'SWAGGER3.2.1' )
>       ->json_has( '' => $limit_hashref, 'SWAGGER3.3.1' );
> 
>+    $t->post_ok( "//$auth_userid:$password@/api/v1/transfer_limits" => json => $limit_hashref )
>+      ->status_is( 409, 'Conflict creating the resource' )
>+      ->json_is(
>+        {
>+            error => "[A transfer limit with the given parameters already exists!]"
>+        }
>+      );
>+
>     $schema->storage->txn_rollback;
> };
> 
>-- 
>2.29.2

Actions: View | Diff | Splinter Review

Attachments on bug 26633: 111380 | 111381 | 112327 | 112328 | 112330 | 112437 | 112438 | 112587 | 112778 | 112779 | 112780 | 112781 | 112782 | 112923 | 112924 | 112925 | 112926 | 112927 | 112928 | 112929 | 113053 | 113265 | 113266 | 113267 | 113268 | 113269 | 113270 | 113271 | 113272 | 113273 | 113274 | 113275 | 113676 | 113677 | 113961 | 116612 | 116613 | 116952 | 116953 | 116954 | 116955 | 116956 | 116957 | 116958 | 116959 | 116960 | 116961 | 116962 | 116963 | 116964