Fosome / garb

A Ruby wrapper for the Google Analytics API
http://www.viget.com/extend/
654 stars 89 forks source link

Multiple filters with OR not working #85

Closed nateabbott closed 13 years ago

nateabbott commented 13 years ago

It's very possible that I've missed something very obvious, but it seems like the gem as currently structured, will always default to an AND operator with multiple filters. Take this example:

Class MyReport
  extend Garb::Model
  metrics :pageviews, :uniquePageviews
  dimensions :pagePath
end

MyReport.results(profile, :start_date => Date.today-1.month
                          :end_date => Date.today,
                          :filters => {:page_path.contains => "/index", :page_path.contains => "/home")

It uses an AND between the filters. However, if I change it to:

MyReport.results(profile, :start_date => Date.today-1.month
                          :end_date => Date.today,
                          :filters => [{:page_path.contains => "/index"}, {:page_path.contains => "/home"}])

It breaks, because line 60 of model.rb adds an extra array wrapper (ends up returning [[{:page_path.contains => "/index"}, {:page_path.contains => "/home"}]] when you call filters.parameters in the to_param method in filter_parameters.rb) and the result is it returns "%3B" as the result instead of "ga:pagePath%3D~%2Findex,ga:pagePath%3D~%2Fhome".

I made a quick fix in my branch (https://github.com/nateabbott/garb/commit/1b6218c3dc89c636524fe7ce8a34358f81d2c811) and can submit a pull request, or if I'm doing something totally wrong or missed something let me know as well!

thiagobc commented 13 years ago

This fix seems ok for me, and if no bigger refactory is done for the filters then might be good to use this solution.

tpitale commented 13 years ago

I'm in the process of redoing the filters, but I should fix this, yes. The filter parameters used to work in a different way.

tpitale commented 13 years ago

I've got a fix in master now, and I will release a gem shortly!

thiagobc commented 13 years ago

Awesome! Thanks a lot, Tony!