gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
(cherry picked from commit d20bc39d75)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 9a4910812f)

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
This commit is contained in:
Tomas Cohen Arazi 2020-08-05 12:04:47 -03:00 committed by Aleisha Amohia
parent d6398f2f08
commit 2b0bd07a2d
2 changed files with 64 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
t/Koha/REST/Plugin/Pagination.t
View File

@ -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 {

14
t/db_dependent/Koha/REST/Plugin/Objects.t
View File

@ -110,7 +110,7 @@ my $t = Test::Mojo->new;
subtest 'objects.search helper' => sub {
plan tests => 44;
plan tests => 50;
$schema->storage->txn_begin;
@ -211,6 +211,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;
};