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

Use the max_results env value in the paginator #10797

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

snipe
Copy link
Owner

@snipe snipe commented Mar 8, 2022

This is a small change, and while it's gross to mix PHP and javascript, it was previously setup a little confusing. If you don't have this set, it will default to 500 - but the bootstrap-tables results list (10, 20, 50, 100, 200, etc) would go up to 1000 - but since this setting exists, it wouldn't actually return 1000, and the export wouldn't actually export higher than 500 rows at a time.

This isn't a flawless solution - and generally speaking if you're returning over 500, you're gonna have a bad time - not because Snipe-IT can't handle it, but because jamming that much stuff into any browser's DOM is going to be sloooooow - but at least now it doesn't show an option that shouldn't be shown.

snipe-it/config/app.php

Lines 40 to 50 in 3e2fe10

|--------------------------------------------------------------------------
| Result Limit
|--------------------------------------------------------------------------
|
| This value determines the max number of results to return, even if a higher limit
| is passed in the API request. This is done to prevent server timeouts when
| custom scripts are requesting 100k assets at a time.
|
*/
'max_results' => env('MAX_RESULTS', 500),

Screen Shot 2022-03-07 at 6 50 01 PM

Screen Shot 2022-03-07 at 6 50 44 PM

Down the line, we probably want to create a helper method that sanely walks through increments a little better and just use that to determine what the page amounts should be, but this fixes the issue for the vast majority of use cases.

Fixes SC-17768 and FD-24875.

Signed-off-by: snipe [email protected]

Copy link
Collaborator

@uberbrady uberbrady left a comment

Choose a reason for hiding this comment

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

This looks great!

@snipe snipe merged commit 33ee63f into master Mar 8, 2022
@snipe snipe deleted the features/use_max_results_in_pagination branch March 8, 2022 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants