Merge pull request #9904 from nikita2206/fix-google-analytics-dates

Fix GA date off by one errors + limited support for AND with GA dates

modules/drivers/googleanalytics/src/metabase/driver/googleanalytics/query_processor.clj

@@ -50,7 +50,9 @@
     (and (= unit :day) (= amount 0))  "today"
     (and (= unit :day) (= amount -1)) "yesterday"
     (and (= unit :day) (< amount -1)) (str (- amount) "daysAgo")
-    :else                             (du/format-date "yyyy-MM-dd" (du/date-trunc unit (du/relative-date unit amount)))))
+    :else                             (du/format-date
+                                        "yyyy-MM-dd"
+                                        (du/date-trunc unit (du/relative-date unit amount)))))
 
 (defmethod ->rvalue :value [[_ value _]]
   value)
@@ -175,6 +177,26 @@
 
 ;;; ----------------------------------------------- filter (intervals) -----------------------------------------------
 
+
+(defn- date-sub-day [[clause time-component unit :as value]]
+  (case clause
+    :absolute-datetime [:absolute-datetime (du/relative-date :day -1 (du/date-trunc unit time-component)) :day]
+    :relative-datetime (if (= unit :day)
+                         [clause (- time-component 1) unit]
+                         [:absolute-datetime
+                          (du/relative-date :day -1 (du/date-trunc unit (du/relative-date unit time-component))) :day])
+    value))
+
+(defn- date-add-day [[clause time-component unit :as value]]
+  (case clause
+    :absolute-datetime [:absolute-datetime (du/relative-date :day 1 (du/date-trunc unit time-component)) unit]
+    :relative-datetime (if (= unit :day)
+                         [clause (+ time-component 1) unit]
+                         [:absolute-datetime
+                          (du/relative-date :day 1 (du/date-trunc unit (du/relative-date unit time-component))) :day])
+    value))
+
+
 (defmulti ^:private parse-filter:interval mbql.u/dispatch-by-clause-name-or-class)
 
 (defmethod parse-filter:interval :default [_] nil)
@@ -183,12 +205,16 @@
   {:start-date (->rvalue min-val), :end-date (->rvalue max-val)})
 
 (defmethod parse-filter:interval :> [[_ field value]]
-  {:start-date (->rvalue value), :end-date latest-date})
+  {:start-date (->rvalue (date-add-day value))})
 
 (defmethod parse-filter:interval :< [[_ field value]]
-  {:start-date earliest-date, :end-date (->rvalue value)})
+  {:end-date (->rvalue (date-sub-day value))})
 
-;; TODO - why we don't support `:>=` or `:<=` in GA?
+(defmethod parse-filter:interval :>= [[_ field value]]
+  {:start-date (->rvalue value)})
+
+(defmethod parse-filter:interval :<= [[_ field value]]
+  {:end-date (->rvalue value)})
 
 (defmethod parse-filter:interval := [[_ field value]]
   {:start-date (->rvalue value)
@@ -205,8 +231,16 @@
       (throw (Exception. (str (tru "Multiple date filters are not supported")))))
     (first filters)))
 
+(defn- try-reduce-filters [[filter1 filter2]]
+  (merge-with
+    (fn [_ _] (throw (Exception. (str (tru "Multiple date filters are not supported in filters: ") filter1 filter2))))
+    filter1 filter2))
+
 (defmethod parse-filter:interval :and [[_ & subclauses]]
-  (maybe-get-only-filter-or-throw (map parse-filter:interval subclauses)))
+  (let [filters (map parse-filter:interval subclauses)]
+    (if (= (count filters) 2)
+      (try-reduce-filters filters)
+      (maybe-get-only-filter-or-throw filters))))
 
 (defmethod parse-filter:interval :or [[_ & subclauses]]
   (maybe-get-only-filter-or-throw (map parse-filter:interval subclauses)))
@@ -219,18 +253,36 @@
   [filter-clause]
   (mbql.u/replace filter-clause
     ;; we don't support any of the following as datetime filters
-    #{:!= :<= :>= :starts-with :ends-with :contains}
+    #{:!= :starts-with :ends-with :contains}
     nil
 
-    [(_ :guard #{:< :> :between :=}) [(_ :guard (partial not= :datetime-field)) & _] & _]
+    [(_ :guard #{:< :> :<= :>= :between :=}) [(_ :guard (partial not= :datetime-field)) & _] & _]
     nil))
 
+(defn- normalize-unit [unit]
+  (if (= unit :default) :day unit))
+
+(defn- normalize-datetime-units
+  "Replace all unsupported datetime units with the default"
+  [filter-clause]
+  (mbql.u/replace filter-clause
+
+    [:datetime-field field unit]        [:datetime-field field (normalize-unit unit)]
+    [:absolute-datetime timestamp unit] [:absolute-datetime timestamp (normalize-unit unit)]
+    [:relative-datetime amount unit]    [:relative-datetime amount (normalize-unit unit)]))
+
+(defn- add-start-end-dates [filter-clause]
+  (merge {:start-date earliest-date, :end-date latest-date} filter-clause))
+
 (defn- handle-filter:interval
   "Handle datetime filter clauses. (Anything that *isn't* a datetime filter will be removed by the
   `handle-builtin-segment` logic)."
   [{filter-clause :filter}]
   (or (when filter-clause
-        (parse-filter:interval (remove-non-datetime-filter-clauses filter-clause)))
+        (add-start-end-dates
+          (parse-filter:interval
+            (normalize-datetime-units
+              (remove-non-datetime-filter-clauses filter-clause)))))
       {:start-date earliest-date, :end-date latest-date}))
 
 

modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj

@@ -87,6 +87,31 @@
   (ga-query {:start-date "2016-11-08", :end-date "2016-11-08"})
   (mbql->native {:query {:filter [:= (ga-date-field :day) [:absolute-datetime #inst "2016-11-08" :day]]}}))
 
+;; tests off by one day correction for gt/lt operators (GA doesn't support exclusive ranges)
+(expect
+  (ga-query {:start-date "2016-11-09", :end-date "today"})
+  (mbql->native {:query {:filter [:> (ga-date-field :day) [:absolute-datetime #inst "2016-11-08" :day]]}}))
+
+(expect
+  (ga-query {:start-date "2005-01-01", :end-date "2016-10-01"})
+  (mbql->native {:query {:filter [:< (ga-date-field :day) [:absolute-datetime #inst "2016-10-02" :day]]}}))
+
+(expect
+  (ga-query {:start-date "2005-01-01", :end-date "2016-10-02"})
+  (mbql->native {:query {:filter [:<= (ga-date-field :day) [:absolute-datetime #inst "2016-10-02" :day]]}}))
+
+(expect
+  (ga-query {:start-date "2016-09-10", :end-date "2016-10-01"})
+  (mbql->native {:query {:filter [:and
+                                  [:< (ga-date-field :day) [:absolute-datetime #inst "2016-10-02" :day]]
+                                  [:> (ga-date-field :day) [:absolute-datetime #inst "2016-09-09" :day]]]}}))
+
+(expect
+  (ga-query {:start-date "2016-09-10", :end-date "2016-10-02"})
+  (mbql->native {:query {:filter [:and
+                                  [:<= (ga-date-field :day) [:absolute-datetime #inst "2016-10-02" :day]]
+                                  [:> (ga-date-field :day) [:absolute-datetime #inst "2016-09-09" :day]]]}}))
+
 ;; relative date -- last month
 (expect
   (ga-query {:start-date (du/format-date "yyyy-MM-01" (du/relative-date :month -1))
@@ -117,6 +142,23 @@
              :end-date   (du/format-date "yyyy-01-01")})
   (mbql->native {:query {:filter [:= (ga-date-field :year) [:relative-datetime -1 :year]]}}))
 
+;; a range starting from 30 days ago excluding that first day until today
+(expect
+  (ga-query {:start-date "29daysAgo"
+             :end-date   "today"})
+  (mbql->native {:query {:filter [:> (ga-date-field :day) [:relative-datetime -30 :day]]}}))
+
+(expect
+  (ga-query {:start-date "30daysAgo"
+             :end-date   "today"})
+  (mbql->native {:query {:filter [:>= (ga-date-field :day) [:relative-datetime -30 :day]]}}))
+
+;; last year excluding the first day of the range
+(expect
+  (ga-query {:start-date (du/format-date "yyyy-MM-dd" (du/relative-date :day 1 (du/date-trunc :year (du/relative-date :year -1))))
+             :end-date   "today"})
+  (mbql->native {:query {:filter [:> (ga-date-field :year) [:relative-datetime -1 :year]]}}))
+
 ;; limit
 (expect
   (ga-query {:max-results 25})