From d7407055a891e723c7840739ff277b699668a56d Mon Sep 17 00:00:00 2001 From: Ere Maijala 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. Signed-off-by: Mason James Signed-off-by: Martin Renvoize --- api/v1/swagger/x-primitives.json | 2 +- t/db_dependent/api/v1/acquisitions_orders.t | 43 ++++++++++++++++----- 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, 59 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 39b9de61e0..01fe28a92a 100644 --- a/t/db_dependent/api/v1/acquisitions_orders.t +++ b/t/db_dependent/api/v1/acquisitions_orders.t @@ -77,14 +77,24 @@ 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_is( [ $order->to_api, $another_order->to_api ] ); + 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 }; + # 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 +260,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 fda8047d33..535c718401 100644 --- a/t/db_dependent/api/v1/holds.t +++ b/t/db_dependent/api/v1/holds.t @@ -211,10 +211,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 631cd3551d..4e82935f4b 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 0d52f99768..8321f2e4c0 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 90998411fb..2a57516319 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.39.5