Fix GA date off by one errors [ci all]

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

@@ -175,6 +175,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 +203,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))})
+
+(defmethod parse-filter:interval :>= [[_ field value]]
+  {:start-date (->rvalue value)})
 
-;; TODO - why we don't support `:>=` or `:<=` in GA?
+(defmethod parse-filter:interval :<= [[_ field value]]
+  {:end-date (->rvalue value)})
 
 (defmethod parse-filter:interval := [[_ field value]]
   {:start-date (->rvalue value)
@@ -205,8 +229,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 +251,21 @@
   [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- 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 (remove-non-datetime-filter-clauses filter-clause))))
       {:start-date earliest-date, :end-date latest-date}))
 
 

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

@@ -17,7 +17,8 @@
             [metabase.test.util :as tu]
             [metabase.util.date :as du]
             [toucan.db :as db]
-            [toucan.util.test :as tt]))
+            [toucan.util.test :as tt]
+            [clj-time.core :as t]))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                        MBQL->NATIVE (QUERY -> GA QUERY)                                        |
@@ -87,6 +88,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 day-off 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 +143,22 @@
              :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]]}}))
+
+(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})