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

When searching, duplicate the search query into the _object_clone... #1

Closed
wants to merge 1 commit into from

Conversation

eblount
Copy link

@eblount eblount commented Aug 27, 2012

... so that the count returned is correct. Please note: I'm sure there are other ways to accomplish this. I chose this way so that a specific order of operations (force users to call _search() before _limit(), etc.) isn't required.

@morgan
Copy link
Owner

morgan commented Aug 28, 2012

Hello Eric,

Paginate::$_count_total is expecting the aggregate total without any limiting or searching.

Below is a use case taking DataTables into consideration:

http://datatables.net/release-datatables/examples/data_sources/server_side.html

If you search and look at the response, you will see something along the lines of:

{
    "sEcho":10,
    "iTotalRecords":"57",
    "iTotalDisplayRecords":"31",
    
}

The expectation is iTotalRecords is a count of the entire data set while iTotalDisplayRecords is the aggregate total being paged. Without searching, if the total records is 57 than both iTotalRecords and iTotalDisplayRecords should be 57. The only time iTotalDisplayRecords should be less than the total (in this case 57) is when searching.

With this in mind, I am seeing a defect relating to search. Specifically, iTotalDisplayRecords should contain the search count which as of current it contains the page count.

Obviously, above is taking DataTables into mind. Paginate is designed and used in more contexts than just DataTables. The solution needs to be deriving a search total and adding testing for this particular piece of logic.

Do keep in mind that if I accepted your pull request, the search unit test for Database and ORM will fail (https://github.com/morgan/kohana-paginate/blob/3.2/master/tests/kohana/PaginateTest.php#L91) as outlined above. Have opened an issue for this defect (#2). Will also update DataTables module once a new aggregate count for search is available (to be used in conjunction with iTotalDisplayRecords).

Appreciate your pull request. Thank you for your time on this.

Micheal Morgan

@morgan morgan closed this Aug 28, 2012
@eblount
Copy link
Author

eblount commented Aug 28, 2012

Sure thing - I knew there were potentially other ways to solve this, and I'm sure you'll find a way to correct the DataTables issue while keeping your other requirements for Paginate.

@morgan
Copy link
Owner

morgan commented Sep 2, 2012

Hello Eric,

Released 0.2.0 of Paginate resolving issue #2 as outlined above.

Please let me know if you see anything else or have any questions.

Thank you,

Micheal Morgan

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.

None yet

2 participants