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

make Model::paginate() use default perPage from Config\Pager->perPage if $perPage parameter not passed #2782

Merged

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Mar 30, 2020

Currently, the $perPage is default to 20 in Model::paginate() function. I think it should read from config('Pager')->perPage instead. I apply make default to null, and check if it is not passed, it will use the default config('Pager')->perPage

Checklist:

  • Securely signed commits
  • Unit testing, with >80% coverage

@samsonasik
Copy link
Member Author

samsonasik commented Mar 30, 2020

I've added unit test for use case change config('Pager')->perPage and passed parameter

@samsonasik samsonasik changed the title make Model::paginate() use default perPage from Config\Pager->perPage if not passed make Model::paginate() use default perPage from Config\Pager->perPage if $perPage parameter not passed Mar 30, 2020
@samsonasik
Copy link
Member Author

travis build green 🎉

@samsonasik samsonasik force-pushed the model-paginate-default-perpage branch from 331f72b to 95165e4 Compare March 31, 2020 02:41
@samsonasik
Copy link
Member Author

I've added check in Pager class to read $perPage from Config('Pager')->perPage to handle it when used in outside Model/Db related pagination.

@samsonasik
Copy link
Member Author

I moved the handle per page null check to Pager class, use $this->pager->getPerPage() pulled from Pager instance to get final perPage value that will be used to make a limit query in the Model::paginate()

@samsonasik
Copy link
Member Author

I updated CodeIgniter\Config\Services::pager() to use config('Pager') as well if no $config passed (null), instead of manual "new Config\Pager()" like in other services which uses config() function. It also make it testable in paginate in model test.

@samsonasik
Copy link
Member Author

travis build green 🎉

@MGatner
Copy link
Member

MGatner commented Mar 31, 2020

Definitely a good change, and a solid PR. Thanks as always @samsonasik

@MGatner MGatner merged commit 089231d into codeigniter4:develop Mar 31, 2020
@samsonasik samsonasik deleted the model-paginate-default-perpage branch March 31, 2020 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants