From 65cdef74ef77cd86e98b46f57ea02c0d346da2ec Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Tue, 17 Aug 2021 16:21:11 -0300
Subject: [PATCH] Bug 24857: (QA follow-up) Adding items should check volume
 linked to biblio

I noticed some tests included /biblios/{biblio_id}/volumes... in the
URL. And they passed :-D This means the biblio_id is never tested.

This patch fixes those two tests so they have valid biblio_id in the
path, and adds a proper regression test for the expected behavior (409).

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/volumes.t
=> FAIL: Notice the tests pass, even with bad data on the request
2. Apply this patch, notice a test is added for the bad biblio_id case
   and the other two tests are fixed so they have the intended
   biblio_id.
3. Repeat 1
=> SUCCESS: Tests pass! This means things are correctly tested in the
   controller and the unit tests cover teh 'bad case' returning 409.
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 Koha/REST/V1/Biblios/Volumes/Items.pm |  9 +++++++++
 t/db_dependent/api/v1/volumes.t       | 12 ++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Koha/REST/V1/Biblios/Volumes/Items.pm b/Koha/REST/V1/Biblios/Volumes/Items.pm
index 831690773c..fd6ad08882 100644
--- a/Koha/REST/V1/Biblios/Volumes/Items.pm
+++ b/Koha/REST/V1/Biblios/Volumes/Items.pm
@@ -56,6 +56,15 @@ sub add {
             );
         }
 
+        unless ( $volume->biblionumber eq $c->validation->param('biblio_id') ) {
+            return $c->render(
+                status  => 409,
+                openapi => {
+                    error => 'Volume does not belong to passed biblio_id'
+                }
+            );
+        }
+
         # All good, add the item
         my $body    = $c->validation->param('body');
         my $item_id = $body->{item_id};
diff --git a/t/db_dependent/api/v1/volumes.t b/t/db_dependent/api/v1/volumes.t
index 2b6b29e201..2bfe28adf3 100755
--- a/t/db_dependent/api/v1/volumes.t
+++ b/t/db_dependent/api/v1/volumes.t
@@ -210,7 +210,7 @@ subtest 'volumes delete() tests' => sub {
 };
 
 subtest 'volume items add() + delete() tests' => sub {
-    plan tests => 11;
+    plan tests => 14;
 
     $schema->storage->txn_begin;
 
@@ -234,8 +234,12 @@ subtest 'volume items add() + delete() tests' => sub {
     my $item_1 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
     my $item_1_id = $item_1->id;
 
-    $t->post_ok( "//$userid:$password@/api/v1/biblios/{biblio_id}/volumes/$volume_id/items" => json => { item_id => $item_1->id } )
-        ->status_is( 201, 'SWAGGER3.2.1' );
+    $t->post_ok( "//$userid:$password@/api/v1/biblios/XXX/volumes/$volume_id/items" => json => { item_id => $item_1->id } )
+      ->status_is( 409 )
+      ->json_is( { error => 'Volume does not belong to passed biblio_id' } );
+
+    $t->post_ok( "//$userid:$password@/api/v1/biblios/$biblio_id/volumes/$volume_id/items" => json => { item_id => $item_1->id } )
+      ->status_is( 201, 'SWAGGER3.2.1' );
 
     @items = $volume->items;
     is( scalar(@items), 1, 'Volume now has one item');
@@ -243,7 +247,7 @@ subtest 'volume items add() + delete() tests' => sub {
     my $item_2 = $builder->build_sample_item({ biblionumber => $biblio->biblionumber });
     my $item_2_id = $item_2->id;
 
-    $t->post_ok( "//$userid:$password@/api/v1/biblios/{biblio_id}/volumes/$volume_id/items" => json => { item_id => $item_2->id } )
+    $t->post_ok( "//$userid:$password@/api/v1/biblios/$biblio_id/volumes/$volume_id/items" => json => { item_id => $item_2->id } )
       ->status_is( 201, 'SWAGGER3.2.1' );
 
     @items = $volume->items;
-- 
2.30.2