Bug 26143: Regression tests

This patch introduces tests for the per_page=-1 handling use case. From
now on per_page=-1 means 'all resources'.

On writing this I noticed that we always paginate results no matter
what, but there was a weird condition under which on pagination headers
were sent back to the API consumer. This is highlighted in the precedent
patch, which is not the -1 situation this one tries to tackle.

Both pagination and searching are broken with per_page=-1, which is a
standard, and we actually didn't explicitly set a way to request all
resources.

To verify this:
1. Apply the previous tests patch and this one
2. Run:
   $ kshell
  k$ prove t/Koha/REST/Plugin/Pagination.t \
           t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Things are damn broken

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/t/Koha/REST/Plugin/Pagination.t b/t/Koha/REST/Plugin/Pagination.t
index adf3001..dff13fb 100644 (file)
--- a/t/Koha/REST/Plugin/Pagination.t
+++ b/t/Koha/REST/Plugin/Pagination.t
@@ -70,6 +70,28 @@ get '/pagination_headers_without_page' => sub {
     $c->render( json => { ok => 1 }, status => 200 );
 };
 
+get '/pagination_headers_with_minus_one' => sub {
+    my $c = shift;
+    $c->add_pagination_headers(
+        {
+            total => 10,
+            params => { _per_page => -1, firstname => 'Jonathan' }
+        }
+    );
+    $c->render( json => { ok => 1 }, status => 200 );
+};
+
+get '/pagination_headers_with_minus_one_and_invalid_page' => sub {
+    my $c = shift;
+    $c->add_pagination_headers(
+        {
+            total  => 10,
+            params => { page => 100, _per_page => -1, firstname => 'Jonathan' }
+        }
+    );
+    $c->render( json => { ok => 1 }, status => 200 );
+};
+
 # The tests
 
 use Test::More tests => 2;
@@ -79,7 +101,7 @@ use t::lib::Mocks;
 
 subtest 'add_pagination_headers() tests' => sub {
 
-    plan tests => 75;
+    plan tests => 101;
 
     my $t = Test::Mojo->new;
 
@@ -164,6 +186,34 @@ subtest 'add_pagination_headers() tests' => sub {
       ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=3.*>; rel="last"/ )
       ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=4.*>; rel="last"/ )
       ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="last"/ );
+
+    $t->get_ok('/pagination_headers_with_minus_one')
+      ->status_is( 200 )
+      ->header_is( 'X-Total-Count' => 10, 'X-Total-Count header present, with per_page=-1' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="prev",/, 'First page, no previous' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="prev",/, 'First page, no previous' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="prev",/, 'First page, no previous' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="next",/, 'No next page, all resources are fetched' )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="first",/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="first",/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="first",/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="last"/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="last"/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="last"/ );
+
+    $t->get_ok('/pagination_headers_with_minus_one_and_invalid_page')
+      ->status_is( 200 )
+      ->header_is( 'X-Total-Count' => 10, 'X-Total-Count header present, with per_page=-1' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="prev",/, 'First page, no previous' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="prev",/, 'First page, no previous' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="prev",/, 'First page, no previous' )
+      ->header_unlike( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="next",/, 'No next page, all resources are fetched' )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="first",/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="first",/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="first",/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*per_page=-1.*>; rel="last"/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*page=1.*>; rel="last"/ )
+      ->header_like( 'Link' => qr/<http:\/\/.*\?.*firstname=Jonathan.*>; rel="last"/ );
 };
 
 subtest 'dbic_merge_pagination() tests' => sub {

diff --git a/t/db_dependent/Koha/REST/Plugin/Objects.t b/t/db_dependent/Koha/REST/Plugin/Objects.t
index 4445bde..d5db5e7 100644 (file)
--- a/t/db_dependent/Koha/REST/Plugin/Objects.t
+++ b/t/db_dependent/Koha/REST/Plugin/Objects.t
@@ -91,7 +91,7 @@ my $builder = t::lib::TestBuilder->new;
 
 subtest 'objects.search helper' => sub {
 
-    plan tests => 44;
+    plan tests => 50;
 
     $schema->storage->txn_begin;
 
@@ -192,6 +192,18 @@ subtest 'objects.search helper' => sub {
     $response_count = scalar @{ $t->tx->res->json };
     is( $response_count, 5, 'RESTdefaultPageSize is honoured by default (5)' );
 
+    $t->get_ok('/cities?_page=1&_per_page=-1')
+      ->status_is(200);
+
+    $response_count = scalar @{ $t->tx->res->json };
+    is( $response_count, 24, '_per_page=-1 means all resources' );
+
+    $t->get_ok('/cities?_page=100&_per_page=-1')
+      ->status_is(200);
+
+    $response_count = scalar @{ $t->tx->res->json };
+    is( $response_count, 24, 'When _per_page=-1 the page param is not considered' );
+
     $schema->storage->txn_rollback;
 };