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 <mason@kohaaloha.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

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": {

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;

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" )

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 );
+            }
         }
     };
 

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!');
 

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/"