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

fixes without_count last_page? behaviour #1009

Merged
merged 8 commits into from Jan 17, 2020

Conversation

montdidier
Copy link
Contributor

I believe this will fix this bug.

After some experimentation I noted that @arel.limit is not always an Integer. In the cases this code fails to perform as expected this type is Arel::Nodes::BindParam. After some inspection of the various types it appears to me this additional type can also be cloned and modified to behave as expected.

@yuki24
Copy link
Member

yuki24 commented Nov 29, 2019

Thanks for your pull request. It would greatly be appreciated if you could add a test and fix the builds for Rails 4.2 and 4.1. We still do support them and we don't drop support for older versions unless it is extremely difficult.

@yuki24
Copy link
Member

yuki24 commented Dec 9, 2019

I just wanted to check in and see if you are still interested in updating the pull request.

  • New tests should be added to demonstrate the existing behavior that should be fixed.
  • The builds for Rails 4.2 and 4.1 should be fixed.

@montdidier
Copy link
Contributor Author

@yuki24 Yes I will fix this. I've just not had a chance to revisit it yet.

@montdidier
Copy link
Contributor Author

@yuki24 Should be fixed .

@yuki24
Copy link
Member

yuki24 commented Dec 20, 2019

Could you add tests? Regression tests are extremely important.

@yuki24 yuki24 merged commit bfcf118 into kaminari:master Jan 17, 2020
@yuki24
Copy link
Member

yuki24 commented Jan 17, 2020

@montdidier Thank you for your contribution! really appreciated.

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.

#empty? breaks subsequent #last_page? if #includes and #without_count are used together
2 participants