From e340b2587271516c5d2709a224b2eee644735798 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 --- 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.33.0