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 erroneous offset/start behavior #264

Merged
merged 1 commit into from
Feb 17, 2018
Merged

Conversation

GUI
Copy link
Contributor

@GUI GUI commented Nov 23, 2017

This changes ajax-datatables-rails to use the passed in start parameter for the SQL OFFSET value, rather than basing the offset on the calculated page number. I think this is the correct behavior, and mirrors what DataTable's own server side example does.

If the start value wasn't an exact multiple of the limit, then previously the database query would not acknowledge intermediate offset values. So, for example, if a request with start=20&limit=100 was made, ajax-datatables-rails would perform a LIMIT 100 OFFSET 0 query, instead of LIMIT 100 OFFSET 20 (which is what is believe should happen).

I suspect you wouldn't see this issue if you were using explicit page-based pagination where start was always a multiple of limit. However, if you're using DataTables with infinite scrolling pagination, then the start value might be fairly random. With the previous behavior, infinite scrolling would appear to do strange things, since new requests like start=20&limit=100 and start=55&limit=100 might be made, but the responses would continue to contain the same identical 1-100 results.

I think I've adjusted the Oracle tests appropriately, but since these don't seem to be passing on Travis CI currently, I'm not entirely sure. But the other databases all seem to be passing.

If the start value wasn't an exact multiple of the limit, then
previously the database query would not acknowledge intermediate offset
values. So, for example, if a request with start=20&limit=100 was made,
ajax-datatables-rails would perform a "LIMIT 100 OFFSET 0" query,
instead of "LIMIT 100 OFFSET 20" (which is what is intended).
@n-rodriguez n-rodriguez merged commit a94a0d3 into jbox-web:master Feb 17, 2018
@n-rodriguez
Copy link
Member

Thank you!

@n-rodriguez n-rodriguez self-assigned this Feb 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants