jbox-web / ajax-datatables-rails

A wrapper around DataTable's ajax methods that allow synchronization with server-side pagination in a Rails app
MIT License
590 stars 227 forks source link

Fix integer out of range #284

Closed PhilippMeissner closed 6 years ago

PhilippMeissner commented 6 years ago

Hi folks! We've lately had a few users trying to search for values that were not meant to be searched but still caused datatables to try and search for it. This lead to this error

9056216999123 is out of range for ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Integer with limit 4

This issue only applies to :eq searches and only if it's an integer value that's stored in this very column.

Maybe this PR is of use. If not, have it removed please 👍

n-rodriguez commented 6 years ago

Hi! Thanks for your contribution! There are some changes to do before I can merge it. Also can you please rebase on master? Thank you!

PhilippMeissner commented 6 years ago

Hey @n-rodriguez - What do you mean by rebase? Do you want me to squash the three commits of mine so that it is only one? That should be available in the dropdown when you merge the PR through the UI. But if you mean something else, hit me up. What changes do you need to be done? Maybe I can spend some more time.

n-rodriguez commented 6 years ago

Hi! I mean merging the last commits I've made into your branch. But since it's a rebase it's gonna take my commits first then apply yours. It works like that :

git remote add upstream https://github.com/jbox-web/ajax-datatables-rails.git
git fefch upstream/master # or 'upstream master' don't remember
git checkout <your branch>
git rebase upstream/master

You'll have to do git push -f -u origin <your branch>to push your changes.

PhilippMeissner commented 6 years ago

Hey @n-rodriguez. Thanks for the small tour on rebasing - I've followed it and pushed everything :)

PhilippMeissner commented 6 years ago

All change requests have been added and the tests are still running fine (on my end: bundle exec rspec). Thanks for your suggestions and time 👍

n-rodriguez commented 6 years ago

Hi! Tests are failing for Rails < 5.x. Can you please take a look?

n-rodriguez commented 6 years ago

Hi! There is something wrong with your implementation.

n-rodriguez commented 6 years ago

Actually your PR does nothing.

n-rodriguez commented 6 years ago

Ok, I think you forgot to create a DB field with a limited int size. By default Rails use int(11) on 4.2 and bigint on 5.x so no error is triggered with your tests and the default config.

n-rodriguez commented 6 years ago

Ok, it turns out you're using the wrong datatable in tests so it's doesn't use :eq condition but :like ;)

PhilippMeissner commented 6 years ago

Hey. Thank you for the effort. I am sorry I did not reply, was quite busy these days (plus there were a lot of holidays in Germany this month ;)). Thanks for fixing and merging!

n-rodriguez commented 6 years ago

Thanks for fixing and merging!

Thank you too, you found it :tada: I realized latter I could have merged your PR and make the fixes right after... sorry