[4.x] Remove duplication of code in search:results tag, and treat pagination params as integer types

[ryanmitchell]

When working on the zinc search integration I noticed the search:results tag wasn't handling Search/Result being returned by the search index, so this updates it.

I also took the opportunity to ensure pagination params were treated as int, instead of treating them as strings.

[jasonvarga]

I don't quite understand what issue this is fixing. Could you explain some more?

The search results tag seems to work fine without this PR. Maybe it's something specific with your setup.

[ryanmitchell]

I probably haven't given enough detail.

So I'm trying to get things in place for: https://github.com/thoughtco/statamic-zinc-search

The query builder I wrote for it is returning a Statamic\Search\Result for each result, so that it works in the control panel. When trying to use the same on the search:results tag that became an issue. Am I wrong in thinking the same results should work in both places?

[jasonvarga]

The current built-in search query builder used in the search:results tag also returns Result instances. 🤔

Could you provide an example repo showing whatever it is that's broken without this PR?

[ryanmitchell]

Yeah I'll pull something together tomorrow.
Looking at it again I think I may just not be converting things on my side, but leave it with me.

I would still like the pagination params to be ints :)

[jasonvarga]

The int params part is fine 👍

[ryanmitchell]

Ok, I forgot to handle the conversion of items to results in my get() method, sorry for totally wasting your time on that bit.
I've renamed the PR to explain the changes better.

[jasonvarga]

Thanks for updating it.

It looks simple enough, but need to check how it behaves when you do paginate="true", since that's valid.

[ryanmitchell]

Is that not already being handled by setPaginationParameterPrecedence()

So we can safely expect paginate to be an int at the point I changed it?

[jasonvarga]

You're probably right but I'm kinda just making a note for when I get back to review this in depth.

[jasonvarga]

There are a couple of small things I noticed about search, like you have to provide a limit if you do paginate="true", which is a bit unexpected.

Anyway, it was like that before the PR too. What you've added doesn't change any behavior.

Thanks!

[ryanmitchell]

Great. I feel like that should just be clarified in the docs?

[jasonvarga]

Still prefer to guide people to just do paginate="number" instead though. But yeah.