Coerce pagination string params

[ryansch]

We hit this in our app when we pass in params from a rails controller. This PR causes the limit and offset methods to work like the page (or ::Kaminari.config.page_method_name) method already does.

[morganick]

👍

[AaronRustad]

This may introduce some confusion as any value other than a string representation of a number is likely going to return 0 by calling the #to_i:

"35".to_i   # good
> 35     

"ab".to_i  # not good
> 0

This will cause the next few lines to succeed, with a size of 0 and a per_page of 0, and likely add confusion. I think it should fail loudly. Thoughts?

[ryansch]

I don't know of a way to get ruby to throw up when converting a String to a Fixnum if the conversion doesn't make sense.

[ryansch]

Whatever we decide here needs to also be applied to the #page method above in the same file (it already uses #to_i.

[morganick]

What if instead of just coercing we validated it as well?

@per_page.to_i must be >= 1

If it is < 1, then it has no affect on the query and neither does the page argument. Thus returning all results. The page number is only valid when there is a proper per_page limit.

When I put in a page number that doesn't make sense as a possible value say -1 or boom kaminari just gives me page 1. So following that I would really want it to be ignored and fall back to a default value. Default value for @page is easy. Picking a default value for @per_page would be arbitrary.

Also, I wouldn't want to throw an exception because someone decided to put ?per_page=boom in their address bar; I get enough email as it is.

[AaronRustad]

To fail loudly:

@per_page = Integer(value)

[ryansch]

@AaronRustad That's entirely too obvious. 😏

[ryansch]

I like @morganick's idea of testing for >=1 since that also deals with negative integers.

[ryansch]

@AaronRustad @morganick I've added code/tests for invalid string params.

[ryansch]

Coming back to ES after some time on other projects. This PR still needs a merge. I've rebased the topic branch and pushed it.

[ryansch]

I've signed the CLA.

[karmi]

@ryansch Finally got to merge this, sorry for the ridiculous delay!