Check for _per_page existence

[VincentLanglet]

Subject

I got this weird error recently.

I am targeting this branch, because BC.

Changelog

### Fixed
- Stop throwing an exception when the `_per_page`  filter parameter is not set

[franmomu]

I got this weird error recently.

Why is it weird? Can you please add a test covering that case?

[VincentLanglet]

I got this weird error recently.

Why is it weird? Can you please add a test covering that case?

Weird, become I never got it before and I didn't change something on my admin ^^'.

I tried to look at how to add a test, it's easy in master (and the check is already added https://github.com/sonata-project/SonataAdminBundle/blob/master/src/Admin/AbstractAdmin.php#L726) but in 3.x there is theoricaly a default value https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Admin/AbstractAdmin.php#L184

[franmomu]

Have you debug where the problem come from? A way to test it is in an Admin class create a method that overrides $datagridValues removing _per_page key.

[VincentLanglet]

Have you debug where the problem come from? A way to test it is in an Admin class create a method that overrides $datagridValues removing _per_page key.

After debugging, there was no _per_page in the request because of some try (related to another PR and filters).

This test https://github.com/sonata-project/SonataAdminBundle/blob/master/tests/Admin/AdminTest.php#L1484 will check for this bug in master. I'm not sure about writing a new test on 3.x with a NEXT_MAJOR: Remove this test...

[franmomu]

This test https://github.com/sonata-project/SonataAdminBundle/blob/master/tests/Admin/AdminTest.php#L1484 will check for this bug in master. I'm not sure about writing a new test on 3.x with a NEXT_MAJOR: Remove this test...

Hmm I think that test is testing it right now because this line:

SonataAdminBundle/src/Admin/AbstractAdmin.php

Line 2473 in 9eb2c5f

// $defaultSortValues = ['_page' => 1, '_per_page' => 25];

is not resolved yet, once there are default values, is the same situation than now and that test will not longer cover this case.

[phansys]

IMO, we should add a test covering this case.

[VincentLanglet]

I looked again and the function getDefaultSortValues will be final in 4.0 and always return a _per_page so the test won't be possible.

final protected function getDefaultSortValues(): array
{
    $defaultSortValues = ['_page' => 1, '_per_page' => 25];
}

[franmomu]

I looked again and the function getDefaultSortValues will be final in 4.0 and always return a _per_page so the test won't be possible.

final protected function getDefaultSortValues(): array
{
    $defaultSortValues = ['_page' => 1, '_per_page' => 25];
}

You can use configureDefaultSortValues to override these values.

[VincentLanglet]

I looked again and the function getDefaultSortValues will be final in 4.0 and always return a _per_page so the test won't be possible.

final protected function getDefaultSortValues(): array

{

$defaultSortValues = ['_page' => 1, '_per_page' => 25];

}

You can use configureDefaultSortValues to override these values.

I can override it, but it still will be set then.