Skip to content

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
@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