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

fix: make query for filteredRecords when totalRecords was manually set #3157

Merged
merged 3 commits into from
Jul 15, 2024
Merged

fix: make query for filteredRecords when totalRecords was manually set #3157

merged 3 commits into from
Jul 15, 2024

Conversation

JurianArie
Copy link
Contributor

Hi, we always use setTotalRecords(0) since we only show the filtered records count. After upgrading to Laravel 11 we noticed that our pagination stopped working. This seems to be caused by c37494f. This PR changes the logic to always query the filtered count if the total count was set manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes from c37494f to the tests have been reverted, the optimalization should not have changed this behavior.

@yajra
Copy link
Owner

yajra commented Jul 10, 2024

Thank you for the PR, I will review this as soon as I can.

@yajra
Copy link
Owner

yajra commented Jul 10, 2024

@Arkhas it seems like the PR you've sent is being reverted on this change. Would you mind reviewing and approving this? Thanks!

@JurianArie
Copy link
Contributor Author

JurianArie commented Jul 11, 2024

@yajra I have added an assertion to show that the optimization @Arkhas did still works. This PR is not a revert of his work, only a bug fix.

@JurianArie JurianArie changed the title Make query for filteredRecords when totalRecords was manually set fix: make query for filteredRecords when totalRecords was manually set Jul 11, 2024
@yajra yajra merged commit 337685d into yajra:master Jul 15, 2024
6 checks passed
@yajra
Copy link
Owner

yajra commented Jul 15, 2024

I confirmed one less query after the patch. This is a good optimization trick for large datasets. Thanks!

Released on 11.1.3 🚀

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