bencentra / hubot-giphy-gifme

"gif me" command for Hubot, get random GIFs from Giphy!
MIT License
6 stars 2 forks source link

use query rather than tags for search api #11

Closed patcon closed 8 years ago

patcon commented 8 years ago

We're currently using the random api endpoint filtered by tags: https://github.com/Giphy/GiphyAPI#random-endpoint

Tags are a little restricting, as often they return no results, which is pretty much the worst case scenario when trying to summon a random gif to brighten all your friend's days.

Let's instead take the top of random result from a simpler search query: https://github.com/Giphy/GiphyAPI#search-endpoint

This could perhaps be a flag so that change isn't forced. Thoughts?

bencentra commented 8 years ago

It doesn't look like the search endpoint is random, it always returns the same list for the same query. This makes sense, but makes it so search couldn't be a drop-in replacement for random. You could randomly select one of the many results for search, but it wouldn't quite be the same.

For example, here's the first result from search for "adventure time", limit 1: http://api.giphy.com/v1/gifs/search?q=adventure+time&api_key=dc6zaTOxFJmzC&limit=1&fmt=html.

And here's the random result for tag "adventure time": http://api.giphy.com/v1/gifs/random?api_key=dc6zaTOxFJmzC&tag=adventure+time&fmt=html

If you wanted to make the change, you could:

Does that make sense?

patcon commented 8 years ago

Heh, yeah makes total sense! I like that it changes. i just didn't like that mildly complex tags return no results, when the slack /giphy command used to be much more forgiving :)

I'll add an envvar to choose search vs tag, and search will be random from returned set

(I created this as a stub, and planned to implement it myself when I had time! Thanks though!)

patsissons commented 8 years ago

I have a reasonable fix in mind for this. basically adding an extra endpoint flag, and then a HUBOT_GIPHY_DEFAULT_ENDPOINT=random environment variable that can be overridden. The desired endpoint would then de-reference to a URI composing funciton. This way team operators can choose which type of giphy api behaviour they prefer by default but does not limit team users from being sandboxed into this behaviour.

updating regex pattern would simply look like /(gif me|giphy)( \/(\S+))?\s*(.*)/ so it would not actually alter any existing behaviour.

Thoughts? I can probably get a PR for this in the work sometime tonight maybe...

UPDATE: fixing super incorrect regex pattern.

patcon commented 8 years ago

any fix is better than no fix, so sounds great to me :)