lanej / zendesk2

Zendesk API V2 client using Cistern
http://lanej.io/zendesk2/
MIT License
25 stars 28 forks source link

client.users.search isn't as flexible as required #29

Closed jasonwbarnett closed 10 years ago

jasonwbarnett commented 10 years ago

Because the parameters are merged in 7:searchable.rb it makes it impossible to search for users by domain, e.g. type:user domain1.com domain2.com.

# Example:

require 'zendesk2'
client = Zendesk2::Client.new
client.users.search({"type" => "user domain1.com domain2.com"})

I looked at the code and I couldn't find a great way to do this without dropping the merge completely or a major overhaul.

jasonwbarnett commented 10 years ago

I found an alternative search that works around this but it requires two separate searches.

client.users.search({"email" => "*@domain1.com"})
client.users.search({"email" => "*@domain2.com"})

users.search should be improved.

A possible solution could be the check to see if parameters contains the type key. If not, then merge.

Let me know your thoughts

jasonwbarnett commented 10 years ago

On second thought. Maybe the right thing to do is just change how search params are formed. Rather than using key pair values, it might make sense to just use a string and require people to form their own searches using Zendesk's search reference.

lanej commented 10 years ago

@jasonwbarnett thanks for the issue. i'll take a look.

lanej commented 10 years ago

@jasonwbarnett best solution might be to adapt search to take a hash and form it correctly OR take a string and just pass it through. will make mock search more difficult but possible.

does that work for you ?

jasonwbarnett commented 10 years ago

I thought about the string idea. I think that the search should probably be a single string, period. I don't know your code base that well, so I'm not sure if you're using a hash because you depend on #merge or what.

I actually ended up developing my own wrapper to complete some tasks immediately and in the process I started to understand some of the underlying challenges with search. One being, how to dynamically convert each returned type to a ruby class. I haven't seen how you do that in your code though. I did things way weird and differently though, https://github.com/jasonwbarnett/zendesk-gem.

lanej commented 10 years ago

i was leaning on it being a hash so i can easily merge in the search type https://github.com/lanej/zendesk2/blob/master/lib/zendesk2/searchable.rb#L7.

the contract for the request should definitely always take in a string : https://github.com/lanej/zendesk2/blob/master/lib/zendesk2/client/requests/search.rb#L3. It does not currently do this.

i think that i'll allow a string or a hash to come in via the collection but make the request very raw so you can have nice and "advanced" versions.

jasonwbarnett commented 10 years ago

cool beans. looks good.