From 0a0b1eb68bdff791060f3f9daa404bd102aa2910 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 14 Jul 2023 10:07:20 -0300 Subject: [PATCH] Bug 30979: (QA follow-up) Tidy things Signed-off-by: Tomas Cohen Arazi --- .../data/mysql/atomicupdate/bug_30979.pl | 4 +- t/db_dependent/api/v1/checkouts.t | 173 +++++++----------- 2 files changed, 65 insertions(+), 112 deletions(-) mode change 100644 => 100755 installer/data/mysql/atomicupdate/bug_30979.pl diff --git a/installer/data/mysql/atomicupdate/bug_30979.pl b/installer/data/mysql/atomicupdate/bug_30979.pl old mode 100644 new mode 100755 index cb5b6ce0fbe..6e11f9186e9 --- a/installer/data/mysql/atomicupdate/bug_30979.pl +++ b/installer/data/mysql/atomicupdate/bug_30979.pl @@ -9,8 +9,8 @@ return { $dbh->do( q{ INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES - ('OpacTrustedCheckout', '0', NULL, 'Allow logged in OPAC users to check out to themselves', 'YesNo') - } + ('OpacTrustedCheckout', '0', NULL, 'Allow logged in OPAC users to check out to themselves', 'YesNo') + } ); }, }; diff --git a/t/db_dependent/api/v1/checkouts.t b/t/db_dependent/api/v1/checkouts.t index d73113c827a..1d83f8916bf 100755 --- a/t/db_dependent/api/v1/checkouts.t +++ b/t/db_dependent/api/v1/checkouts.t @@ -264,8 +264,7 @@ subtest 'get_availability' => sub { } ); my $unauth_password = 'thePassword000'; - $patron->set_password( - { password => $unauth_password, skip_validattion => 1 } ); + $patron->set_password( { password => $unauth_password, skip_validattion => 1 } ); my $unauth_userid = $patron->userid; my $patron_id = $patron->borrowernumber; @@ -278,7 +277,7 @@ subtest 'get_availability' => sub { my %needsconfirmation = (); my %alerts = (); my %messages = (); - my $mocked_circ = Test::MockModule->new('C4::Circulation'); + my $mocked_circ = Test::MockModule->new('C4::Circulation'); $mocked_circ->mock( 'CanBookBeIssued', sub { @@ -287,60 +286,48 @@ subtest 'get_availability' => sub { ); $t->get_ok( -"//$unauth_userid:$unauth_password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(403)->json_is( + "//$unauth_userid:$unauth_password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id") + ->status_is(403)->json_is( { - error => "Authorization failure. Missing required permission(s).", - required_permissions => - { circulate => "circulate_remaining_permissions" } + error => "Authorization failure. Missing required permission(s).", + required_permissions => { circulate => "circulate_remaining_permissions" } } - ); + ); # Available - $t->get_ok( -"//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200)->json_is( '/blockers' => {} ) - ->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) - ->json_has( '/confirmation_token'); + $t->get_ok("//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id") + ->status_is(200)->json_is( '/blockers' => {} )->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) + ->json_has('/confirmation_token'); # Blocked %issuingimpossible = ( GNA => 1 ); - $t->get_ok( -"//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200)->json_is( '/blockers' => { GNA => 1 } ) - ->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) - ->json_has( '/confirmation_token'); + $t->get_ok("//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id") + ->status_is(200)->json_is( '/blockers' => { GNA => 1 } )->json_is( '/confirms' => {} ) + ->json_is( '/warnings' => {} )->json_has('/confirmation_token'); %issuingimpossible = (); # Warnings/Info %alerts = ( alert1 => "this is an alert" ); %messages = ( message1 => "this is a message" ); - $t->get_ok( -"//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200)->json_is( '/blockers' => {} ) - ->json_is( '/confirms' => {} ) - ->json_is( '/warnings' => - { alert1 => "this is an alert", message1 => "this is a message" } ) - ->json_has( '/confirmation_token'); + $t->get_ok("//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id") + ->status_is(200)->json_is( '/blockers' => {} )->json_is( '/confirms' => {} ) + ->json_is( '/warnings' => { alert1 => "this is an alert", message1 => "this is a message" } ) + ->json_has('/confirmation_token'); %alerts = (); %messages = (); # Needs confirm %needsconfirmation = ( confirm1 => 1, confirm2 => 'please' ); - my $token = Koha::Token->new->generate_jwt( { id => $librarian->id . ":" . $item1_id . ":confirm1:confirm2" }); - $t->get_ok( -"//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200)->json_is( '/blockers' => {} ) - ->json_is( '/confirms' => { confirm1 => 1, confirm2 => 'please' } ) - ->json_is( '/warnings' => {} ) - ->json_has( '/confirmation_token'); + my $token = Koha::Token->new->generate_jwt( { id => $librarian->id . ":" . $item1_id . ":confirm1:confirm2" } ); + $t->get_ok("//$userid:$password@/api/v1/checkouts/availability?item_id=$item1_id&patron_id=$patron_id") + ->status_is(200)->json_is( '/blockers' => {} ) + ->json_is( '/confirms' => { confirm1 => 1, confirm2 => 'please' } )->json_is( '/warnings' => {} ) + ->json_has('/confirmation_token'); my $confirmation_token = $t->tx->res->json('/confirmation_token'); ok( Koha::Token->new->check_jwt( { - id => $librarian->id . ":" - . $item1_id - . ":confirm1:confirm2", + id => $librarian->id . ":" . $item1_id . ":confirm1:confirm2", token => $confirmation_token } ), @@ -352,20 +339,15 @@ subtest 'get_availability' => sub { plan tests => 18; # Available, Not authentication required - $t->get_ok( -"/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200)->json_is( '/blockers' => {} ) - ->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) - ->json_has('/confirmation_token'); + $t->get_ok("/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id")->status_is(200) + ->json_is( '/blockers' => {} )->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) + ->json_has('/confirmation_token'); # Needs confirmation upgrade to blocker %needsconfirmation = ( TOO_MANY => 1, ISSUED_TO_ANOTHER => 1 ); - $t->get_ok( -"/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200) - ->json_is( '/blockers' => { TOO_MANY => 1, ISSUED_TO_ANOTHER => 1 } ) - ->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) - ->json_has('/confirmation_token'); + $t->get_ok("/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id")->status_is(200) + ->json_is( '/blockers' => { TOO_MANY => 1, ISSUED_TO_ANOTHER => 1 } )->json_is( '/confirms' => {} ) + ->json_is( '/warnings' => {} )->json_has('/confirmation_token'); %needsconfirmation = (); # Remove personal information from public endpoint @@ -412,11 +394,9 @@ subtest 'get_availability' => sub { ressurname => 'private', item_notforloan => 'private' ); - $t->get_ok( -"/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id" - )->status_is(200)->json_is( '/blockers' => {} ) - ->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) - ->json_has('/confirmation_token'); + $t->get_ok("/api/v1/public/checkouts/availability?item_id=$item1_id&patron_id=$patron_id")->status_is(200) + ->json_is( '/blockers' => {} )->json_is( '/confirms' => {} )->json_is( '/warnings' => {} ) + ->json_has('/confirmation_token'); %issuingimpossible = (); %alerts = (); %needsconfirmation = (); @@ -447,8 +427,7 @@ subtest 'add checkout' => sub { } ); my $unauth_password = 'thePassword000'; - $patron->set_password( - { password => $unauth_password, skip_validattion => 1 } ); + $patron->set_password( { password => $unauth_password, skip_validattion => 1 } ); my $unauth_userid = $patron->userid; my $patron_id = $patron->borrowernumber; @@ -461,7 +440,7 @@ subtest 'add checkout' => sub { my %needsconfirmation = (); my %alerts = (); my %messages = (); - my $mocked_circ = Test::MockModule->new('C4::Circulation'); + my $mocked_circ = Test::MockModule->new('C4::Circulation'); $mocked_circ->mock( 'CanBookBeIssued', sub { @@ -469,95 +448,69 @@ subtest 'add checkout' => sub { } ); - $t->post_ok( - "//$unauth_userid:$unauth_password@/api/v1/checkouts" => json => - { item_id => $item1_id, patron_id => $patron_id } )->status_is(403) - ->json_is( + $t->post_ok( "//$unauth_userid:$unauth_password@/api/v1/checkouts" => json => + { item_id => $item1_id, patron_id => $patron_id } )->status_is(403)->json_is( { - error => "Authorization failure. Missing required permission(s).", - required_permissions => - { circulate => "circulate_remaining_permissions" } + error => "Authorization failure. Missing required permission(s).", + required_permissions => { circulate => "circulate_remaining_permissions" } } - ); + ); - $t->post_ok( "//$userid:$password@/api/v1/checkouts" => json => - { item_id => $item1_id, patron_id => $patron_id } )->status_is(201); + $t->post_ok( "//$userid:$password@/api/v1/checkouts" => json => { item_id => $item1_id, patron_id => $patron_id } ) + ->status_is(201); # Needs confirm %needsconfirmation = ( confirm1 => 1, confirm2 => 'please' ); $t->post_ok( "//$userid:$password@/api/v1/checkouts" => json => { - item_id => $item1_id, - patron_id => $patron_id, + item_id => $item1_id, + patron_id => $patron_id, } )->status_is(412); - my $token = Koha::Token->new->generate_jwt( - { - id => $librarian->id . ":" - . $item1_id - . ":confirm1:confirm2" - } - ); + my $token = Koha::Token->new->generate_jwt( { id => $librarian->id . ":" . $item1_id . ":confirm1:confirm2" } ); $t->post_ok( "//$userid:$password@/api/v1/checkouts?confirmation=$token" => json => { - item_id => $item1_id, - patron_id => $patron_id + item_id => $item1_id, + patron_id => $patron_id } - )->status_is(201)->or(sub { diag $t->tx->res->body }); + )->status_is(201)->or( sub { diag $t->tx->res->body } ); %needsconfirmation = (); subtest 'public add' => sub { plan tests => 14; my $useridp = $patron->userid; - $patron->set_password( - { password => $password, skip_validation => 1 } ); + $patron->set_password( { password => $password, skip_validation => 1 } ); # Feature disabled t::lib::Mocks::mock_preference( 'OpacTrustedCheckout', 0 ); - $t->post_ok( "/api/v1/public/patrons/$patron_id/checkouts" => json => - { item_id => $item1_id, patron_id => $patron_id } ) - ->status_is(401)->json_is( - { - error => "Authentication failure." - } - ); - $t->post_ok( - "//$useridp:$password@/api/v1/public/patrons/$patron_id/checkouts" - => json => { item_id => $item1_id, patron_id => $patron_id } ) - ->status_is(405) - ->json_is( - { error => "Feature disabled", error_code => "FEATURE_DISABLED" } ); + "/api/v1/public/patrons/$patron_id/checkouts" => json => { item_id => $item1_id, patron_id => $patron_id } ) + ->status_is(401)->json_is( { error => "Authentication failure." } ); + + $t->post_ok( "//$useridp:$password@/api/v1/public/patrons/$patron_id/checkouts" => json => + { item_id => $item1_id, patron_id => $patron_id } )->status_is(405) + ->json_is( { error => "Feature disabled", error_code => "FEATURE_DISABLED" } ); # Feature enabled t::lib::Mocks::mock_preference( 'OpacTrustedCheckout', 1 ); - $t->post_ok( "/api/v1/public/patrons/$patron_id/checkouts" => json => - { item_id => $item1_id, patron_id => $patron_id } ) - ->status_is(401)->json_is( - { - error => "Authentication failure." - } - ); - $t->post_ok( - "//$userid:$password@/api/v1/public/patrons/$patron_id/checkouts" - => json => { item_id => $item1_id, patron_id => $patron_id } ) - ->status_is(403)->json_is( + "/api/v1/public/patrons/$patron_id/checkouts" => json => { item_id => $item1_id, patron_id => $patron_id } ) + ->status_is(401)->json_is( { error => "Authentication failure." } ); + + $t->post_ok( "//$userid:$password@/api/v1/public/patrons/$patron_id/checkouts" => json => + { item_id => $item1_id, patron_id => $patron_id } )->status_is(403)->json_is( { - error => - "Authorization failure. Missing required permission(s).", + error => "Authorization failure. Missing required permission(s).", required_permissions => undef } - ); + ); - $t->post_ok( - "//$useridp:$password@/api/v1/public/patrons/$patron_id/checkouts" - => json => { item_id => $item1_id, patron_id => $patron_id } ) - ->status_is(201); + $t->post_ok( "//$useridp:$password@/api/v1/public/patrons/$patron_id/checkouts" => json => + { item_id => $item1_id, patron_id => $patron_id } )->status_is(201); }; $schema->storage->txn_rollback; -- 2.41.0