[3.x] fix cast paginate per page to int #1997
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1997 +/- ##
=========================================
Coverage 87.04% 87.04%
Complexity 667 667
=========================================
Files 31 31
Lines 1521 1521
=========================================
Hits 1324 1324
Misses 197 197
Continue to review full report at Codecov.
|
Cast per_page parameter Collection::paginate to an integer Added additional test for this case. Resolves #1750
| @@ -327,6 +327,20 @@ public function testPaginate(): void | |||
| $this->assertEquals(1, $results->currentPage()); | |||
| } | |||
|
|
|||
| public function testThatPaginateCastsPerPageParameter() | |||
| { | |||
| $results = User::paginate('2'); | |||
There was a problem hiding this comment.
What is the reason pass $perPage as string, if original method expect int or null ?
There was a problem hiding this comment.
Tests in laravel don't have case with pass string $perPage https://github.com/laravel/framework/blob/0b12ef19623c40e22eff91a4b48cb13b3b415b25/tests/Integration/Database/QueryBuilderTest.php#L181
https://github.com/laravel/framework/blob/0b12ef19623c40e22eff91a4b48cb13b3b415b25/tests/Database/DatabaseQueryBuilderTest.php#L3205
There was a problem hiding this comment.
So... Whats the problem with adding additional tests?
There was a problem hiding this comment.
I don't understand why we pass not int value
There was a problem hiding this comment.
Because it's possible when using the default laravel model without mongodb
There was a problem hiding this comment.
i saw in docs and tests and didn't find examples with passing string $perPage
There was a problem hiding this comment.
To make you happy, i just tested it in a stock laravel installation and confirmed that it actually does work in a stock installation.
And to no surprise it does.
Granted, when i created the PR i didn't fact check the original issue
There was a problem hiding this comment.
- I think that we resolve not our problem, framework don't use
declare(strict_types=1)and type hints - If you know that first argument of method paginate should int type, why doesn't if you try cast to int before call
paginate
divine
changed the title
divine
changed the title
Smolevich
added
the
Needs investigation
Cast per_page parameter Collection::paginate to an integer
Added additional test for this case.
Resolves #1750