Bugzilla – Attachment 98559 Details for
Bug 22522
API authentication breaks with updated Mojolicious version
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 22522: Fix several REST API tests
Bug-22522-Fix-several-REST-API-tests.patch (text/plain), 8.59 KB, created by
Ere Maijala
on 2020-02-07 11:51:21 UTC
(
hide
)
Description:
Bug 22522: Fix several REST API tests
Filename:
MIME Type:
Creator:
Ere Maijala
Created:
2020-02-07 11:51:21 UTC
Size:
8.59 KB
patch
obsolete
>From cfbcad9699fba9eb5099752a54f4e545ec2a7351 Mon Sep 17 00:00:00 2001 >From: Ere Maijala <ere.maijala@helsinki.fi> >Date: Fri, 7 Feb 2020 13:49:46 +0200 >Subject: [PATCH] Bug 22522: Fix several REST API tests > >Fixes among others the invalid use of json_has() which caused broken tests to pass with older Mojolicious versions. >--- > api/v1/swagger/x-primitives.json | 2 +- > t/db_dependent/api/v1/acquisitions_orders.t | 45 ++++++++++++++++----- > t/db_dependent/api/v1/holds.t | 6 ++- > t/db_dependent/api/v1/libraries.t | 13 +++--- > t/db_dependent/api/v1/patrons.t | 14 +++++-- > t/db_dependent/api/v1/patrons_password.t | 4 +- > 6 files changed, 61 insertions(+), 23 deletions(-) > >diff --git a/api/v1/swagger/x-primitives.json b/api/v1/swagger/x-primitives.json >index ee15fa6aa1..19bc8dc012 100644 >--- a/api/v1/swagger/x-primitives.json >+++ b/api/v1/swagger/x-primitives.json >@@ -35,7 +35,7 @@ > "description": "primary phone number for patron's primary address" > }, > "surname": { >- "type": "string", >+ "type": ["string", "null"], > "description": "patron's last name" > }, > "vendor_id": { >diff --git a/t/db_dependent/api/v1/acquisitions_orders.t b/t/db_dependent/api/v1/acquisitions_orders.t >index 4fae8e440d..fce636f45a 100644 >--- a/t/db_dependent/api/v1/acquisitions_orders.t >+++ b/t/db_dependent/api/v1/acquisitions_orders.t >@@ -77,14 +77,26 @@ subtest 'list() tests' => sub { > > my $size = keys %{$fields}; > >- plan tests => $size * 3; >+ plan tests => $size * (2 + 2 * $size); > > foreach my $field ( keys %{$fields} ) { > my $model_field = $fields->{ $field }; >- my $result = >- $t->get_ok("//$userid:$password@/api/v1/acquisitions/orders?$field=" . $order->$model_field) >- ->status_is(200) >- ->json_has( [ $order, $another_order ] ); >+ my $result = $t->get_ok("//$userid:$password@/api/v1/acquisitions/orders?$field=" . $order->$model_field) >+ ->status_is(200); >+ >+ foreach my $key ( keys %{$fields} ) { >+ my $key_field = $fields->{ $key }; >+ # Uh oh. We use budget_id in the query but return fund_id. >+ $key = 'fund_id' if ($key eq 'budget_id'); >+ # Check the result order first since it's not predefined. >+ if ($result->tx->res->json->[0]->{$key} eq $order->$key_field) { >+ $result->json_is( "/0/$key", $order->$key_field ); >+ $result->json_is( "/1/$key", $another_order->$key_field ); >+ } else { >+ $result->json_is( "/0/$key", $another_order->$key_field ); >+ $result->json_is( "/1/$key", $order->$key_field ); >+ } >+ } > } > }; > >@@ -250,11 +262,26 @@ subtest 'update() tests' => sub { > address1 => "New library address", > }; > >- $t->put_ok( "//$auth_userid:$password@/api/v1/libraries/$library_id" => json => $library_with_missing_field ) >- ->status_is(400) >- ->json_has( "/errors" => >- [ { message => "Missing property.", path => "/body/address2" } ] >+ my $result = $t->put_ok( "//$auth_userid:$password@/api/v1/libraries/$library_id" => json => $library_with_missing_field ) >+ ->status_is(400); >+ # Check the result order first since it's not predefined. >+ if ($result->tx->res->json->{errors}->[0]->{path} eq '/body/name') { >+ $result->json_is( >+ "/errors", >+ [ >+ {message => "Missing property.", path => "/body/name"}, >+ {message => "Missing property.", path => "/body/library_id"} >+ ] >+ ); >+ } else { >+ $result->json_is( >+ "/errors", >+ [ >+ {message => "Missing property.", path => "/body/library_id"}, >+ {message => "Missing property.", path => "/body/name"} >+ ] > ); >+ } > > my $deleted_library = $builder->build_object( { class => 'Koha::Libraries' } ); > my $library_with_updated_field = $deleted_library->to_api; >diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t >index 95c70e00ad..42ee0ebe31 100644 >--- a/t/db_dependent/api/v1/holds.t >+++ b/t/db_dependent/api/v1/holds.t >@@ -197,10 +197,14 @@ subtest "Test endpoints with permission" => sub { > ->json_is('/0/patron_id', $patron_2->borrowernumber) > ->json_hasnt('/1'); > >+ # While suspended_until is date-time, it's always set to midnight. >+ my $expected_suspended_until = $suspended_until->strftime('%FT00:00:00%z'); >+ substr($expected_suspended_until, -2, 0, ':'); >+ > $t->put_ok( "//$userid_1:$password@/api/v1/holds/$reserve_id" => json => $put_data ) > ->status_is(200) > ->json_is( '/hold_id', $reserve_id ) >- ->json_is( '/suspended_until', output_pref({ dt => $suspended_until, dateformat => 'rfc3339' }) ) >+ ->json_is( '/suspended_until', $expected_suspended_until ) > ->json_is( '/priority', 2 ); > > $t->delete_ok( "//$userid_3:$password@/api/v1/holds/$reserve_id" ) >diff --git a/t/db_dependent/api/v1/libraries.t b/t/db_dependent/api/v1/libraries.t >index 590bbdfc63..b7618d4a81 100644 >--- a/t/db_dependent/api/v1/libraries.t >+++ b/t/db_dependent/api/v1/libraries.t >@@ -85,14 +85,17 @@ subtest 'list() tests' => sub { > > my $size = keys %{$fields}; > >- plan tests => $size * 3; >+ plan tests => $size * (2 + 2 * $size); > > foreach my $field ( keys %{$fields} ) { > my $model_field = $fields->{ $field }; >- my $result = >- $t->get_ok("//$userid:$password@/api/v1/libraries?$field=" . $library->$model_field) >- ->status_is(200) >- ->json_has( [ $library, $another_library ] ); >+ my $result = $t->get_ok("//$userid:$password@/api/v1/libraries?$field=" . $library->$model_field) >+ ->status_is(200); >+ foreach my $key ( keys %{$fields} ) { >+ my $key_field = $fields->{ $key }; >+ $result->json_is( "/0/$key", $library->$key_field ); >+ $result->json_is( "/1/$key", $another_library->$key_field ); >+ } > } > }; > >diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t >index 86f6bd5e90..8d1fcd4520 100644 >--- a/t/db_dependent/api/v1/patrons.t >+++ b/t/db_dependent/api/v1/patrons.t >@@ -310,7 +310,7 @@ subtest 'update() tests' => sub { > warning_like { > $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) > ->status_is(409) >- ->json_has( '/error' => "Fails when trying to update to an existing cardnumber or userid") >+ ->json_has( '/error', "Fails when trying to update to an existing cardnumber or userid") > ->json_like( '/conflict' => qr/(borrowers\.)?cardnumber/ ); } > qr/^DBD::mysql::st execute failed: Duplicate entry '(.*?)' for key '(borrowers\.)?cardnumber'/; > >@@ -318,9 +318,15 @@ subtest 'update() tests' => sub { > $newpatron->{ userid } = "user" . $patron_1->id.$patron_2->id; > $newpatron->{ surname } = "user" . $patron_1->id.$patron_2->id; > >- $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) >- ->status_is(200, 'Patron updated successfully') >- ->json_has($newpatron); >+ my $result = $t->put_ok( "//$userid:$password@/api/v1/patrons/" . $patron_2->borrowernumber => json => $newpatron ) >+ ->status_is(200, 'Patron updated successfully'); >+ >+ # Put back the RO attributes >+ $newpatron->{patron_id} = $unauthorized_patron->to_api->{patron_id}; >+ $newpatron->{restricted} = $unauthorized_patron->to_api->{restricted}; >+ $newpatron->{anonymized} = $unauthorized_patron->to_api->{anonymized}; >+ is_deeply($result->tx->res->json, $newpatron, 'Returned patron from update matches expected'); >+ > is(Koha::Patrons->find( $patron_2->id )->cardnumber, > $newpatron->{ cardnumber }, 'Patron is really updated!'); > >diff --git a/t/db_dependent/api/v1/patrons_password.t b/t/db_dependent/api/v1/patrons_password.t >index 10b1821930..504fcbe64e 100644 >--- a/t/db_dependent/api/v1/patrons_password.t >+++ b/t/db_dependent/api/v1/patrons_password.t >@@ -175,9 +175,7 @@ subtest 'set_public() (unprivileged user tests)' => sub { > $tx->req->env( { REMOTE_ADDR => '127.0.0.1' } ); > $t->request_ok($tx) > ->status_is(403) >- ->json_has({ >- error => "Authorization failure. Missing required permission(s)." >- }); >+ ->json_is('/error', "Authorization failure. Missing required permission(s)."); > > $tx = $t->ua->build_tx( > POST => "/api/v1/public/patrons/" >-- >2.17.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 22522
:
86756
|
93592
|
98061
|
98089
|
98372
|
98557
|
98558
|
98559
|
98665
|
98666
|
98667
|
99181
|
99182
|
99183
|
105591
|
105639
|
105657
|
105658
|
105659
|
105692