prismicio-community / php-kit

Community maintained development kit for Prismic and the PHP language
https://prismic.io
Other
109 stars 83 forks source link

Add SearchForm::count method #26

Closed tremby closed 10 years ago

tremby commented 10 years ago

The SearchForm::submit method is augmented with a $raw parameter which when true causes the raw results object to be returned rather than an array of Document objects.

This allows a count() method on the SearchForm class which copies the form (via pageSize(1)), submits it and returns just the total_results_size property.

tremby commented 10 years ago

@rudyrigot advised a different, backwards-incompatible, approach. I agree that it would be better in the long run, but this is quick (and not even dirty) and shouldn't break anything while still allowing a count query.

dohzya commented 10 years ago

I totally agree that the kit should provide this count() method, but I really don't like the $raw parameter…

What about creating a private method (submitForm or something like this) which would do exactly the same work that your version with $raw=true and making the submit method use self::parseResult on its result? This way the feature would be here but the API would stay consistent.

tremby commented 10 years ago

Fair enough.

Yes, that sounds like a good approach. But I can't spend time on it, since I ended up not using this feature in our application, so I am abandoning this branch for now. Perhaps I or someone else can pick it back up later, or no harm if it's forgotten.

It made more sense for us to do the full query getting all results and just count them, since we were going to do that query and need the full results at some point anyway and this way it is cached on our end. Having queries with different pageSizes would complicate my caching system -- currently I just store results in a hashtable based on the query string, and the query string doesn't include the pageSize parameter. It means it's slower the first time, but all queries seem rather slow (300ms right up to 2 seconds or more, each) and so we actually intend to record/anticipate all queries we need and pre-cache all of them each time the ref changes. A bit heavy handed, I know, but we need low response times.

tremby commented 10 years ago

I'll copy @rudyrigot's message to me here, for reference:


How we did it on the other kits:

You will note that this breaks existing code using submit (or any helper built on top of submit), we've been documenting this very thoroughly in our release notes for the technologies that already have it.

Also, note that you can replace this line by a simpler }, $json->results);. This used to be useful when two versions of our API were out there, which hasn't been the case for long enough to safely get rid of it today.