Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Paging) Ensure page validity #5125

Merged

Conversation

puschie286
Copy link
Contributor

An invalid page lead to an exception when using the model paginate function.
page::store validates the page but it doesnt update the page used in the model.

So we need to read the page value again after calling pager::store in paginate

~ get valid page after validation in pager
+ apply style
Copy link
Member

@paulbalandan paulbalandan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you are overriding the $page variable in L1082 for the computation of $offset. Can you add some tests to cover this?

+ add test function for page lower/higher than available pages
~ use reference instead of (uninitialized) variable
- remove comments
@puschie286
Copy link
Contributor Author

@paulbalandan something like this ?
not sure why the PHP CS Fixer keeps failing...

Copy link
Contributor

@mostafakhudair mostafakhudair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may fix CS Fixer failure

@kenjis
Copy link
Member

kenjis commented Sep 25, 2021

@puschie286 Please run composer cs-fix to fix coding style.

puschie286 and others added 2 commits October 4, 2021 15:17
Co-authored-by: Mostafa Khudair <59371810+mostafakhudair@users.noreply.github.com>
~ cs-fix
@lonnieezell lonnieezell merged commit baf4bb7 into codeigniter4:develop Oct 5, 2021
@puschie286 puschie286 deleted the fix-paging-out-of-range branch October 5, 2021 07:47
@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants