From 4652a8b1702e1ff2ccc7beeb1d4ab87c2c93f765 Mon Sep 17 00:00:00 2001
From: Simon Belak <sb@Simons-MacBook-Pro.local>
Date: Wed, 7 Mar 2018 20:33:47 +0100
Subject: [PATCH] Add remaining comparison combinations

---
 src/metabase/api/automagic_dashboards.clj     | 52 ++++++++--
 .../automagic_dashboards/comparison.clj       | 96 ++++++++++---------
 src/metabase/models/dashboard.clj             | 11 ++-
 3 files changed, 102 insertions(+), 57 deletions(-)

diff --git a/src/metabase/api/automagic_dashboards.clj b/src/metabase/api/automagic_dashboards.clj
index e7a60655337..3650480d74b 100644
--- a/src/metabase/api/automagic_dashboards.clj
+++ b/src/metabase/api/automagic_dashboards.clj
@@ -1,5 +1,5 @@
 (ns metabase.api.automagic-dashboards
-  (:require [compojure.core :refer [GET]]
+  (:require [compojure.core :refer [GET POST]]
             [metabase.api.common :as api]
             [metabase.automagic-dashboards
              [core :as magic]
@@ -23,15 +23,51 @@
   [id]
   (magic/automagic-analysis (Metric id)))
 
-(api/defendpoint GET "/compare/dashboard/:dashboard-id/segments/:left-id/:right-id"
-  "Return an automagic comparison dashboard based on dashboard with ID
-   `dashboard-id`, comparing segments with IDs `left-id` and `right-id`."
-  [dashboard-id left-id right-id]
-  (-> (Dashboard dashboard-id)
+(def ^:private valid-comparison-pair?
+  #{["segment" "segment"]
+    ["segment" "table"]
+    ["segment" "adhoc"]
+    ["table" "segment"]
+    ["table" "adhoc"]
+    ["adhoc" "table"]
+    ["adhoc" "segment"]
+    ["adhoc" "adhoc"]})
+
+(defmulti
+  ^{:private true
+    :doc "Turn `x` into segment-like."
+    :arglists '([x])}
+  ->segment (comp keyword :type))
+
+(defmethod ->segment :table
+  [_]
+  {:name "entire dataset"})
+
+(defmethod ->segment :segment
+  [{:keys [id]}]
+  (-> id
+      Segment
       api/check-404
-      (hydrate [:ordered_cards [:card :in_public_dashboard] :series])
-      (magic.comparison/comparison-dashboard (Segment left-id) (Segment right-id))))
+      (update :name (partial str "segment "))))
+
+(defmethod ->segment :adhoc
+  [{:keys [filter name]}]
+  {:definition {:filter filter}
+   :name       (or name "adhoc segment")})
 
+(api/defendpoint POST "/compare"
+  "Return an automagic comparison dashboard based on given dashboard."
+  [:as {{:keys [dashboard left right]} :body}]
+  (api/check-404 (valid-comparison-pair? (map :type [left right])))
+  (magic.comparison/comparison-dashboard (if (number? dashboard)
+                                           (-> (Dashboard dashboard)
+                                               api/check-404
+                                               (hydrate [:ordered_cards
+                                                         [:card :in_public_dashboard]
+                                                         :series]))
+                                           dashboard)
+                                         (->segment left)
+                                         (->segment right)))
 
 ;; ----------------------------------------- for testing convinience ----------------
 
diff --git a/src/metabase/automagic_dashboards/comparison.clj b/src/metabase/automagic_dashboards/comparison.clj
index 986e5373bc7..ebb417ae7cd 100644
--- a/src/metabase/automagic_dashboards/comparison.clj
+++ b/src/metabase/automagic_dashboards/comparison.clj
@@ -5,7 +5,7 @@
 (defn- dashboard->cards
   [dashboard]
   (->> dashboard
-       :orderd_cards
+       :ordered_cards
        (map (fn [{:keys [sizeY card col row]}]
               (assoc card
                 :height   sizeY
@@ -16,20 +16,25 @@
   [segment card]
   (update-in card [:dataset_query :query :filter]
              (fn [filter-clause]
-               (cond->> (-> segment :definition :filter)
-                 (not-empty filter-clause) (vector :and filter-clause)))))
+               (let [segment-definition (-> segment :definition :filter)]
+                 (cond
+                   (empty? segment-definition) filter-clause
+                   (empty? filter-clause)      segment-definition
+                   :else                       [:and filter-clause
+                                                     segment-definition])))))
 
 (defn- clone-card
   [card]
   (-> card
-      (dissoc :height :position :id)
+      (select-keys [:dataset_query :description :display :name :result_metadata
+                    :visualization_settings])
       (assoc :creator_id    api/*current-user-id*
              :collection_id (-> populate/automagic-collection deref :id))))
 
 (defn- place-row
   [dashboard row height left right]
-  [(if (-> left :display (#{:bar :line}))
-     (update dashboard :orderd_cards conj {:col                    0
+  (if (-> left :display (#{:bar :line}))
+    (update dashboard :ordered_cards conj {:col                    0
                                            :row                    row
                                            :sizeX                  populate/grid-width
                                            :sizeY                  height
@@ -37,23 +42,22 @@
                                            :series                 [right]
                                            :visualization_settings {}
                                            :id                     (gensym)})
-     (let [width (/ populate/grid-width 2)]
-       (-> dashboard
-           (update :ordered_cards conj {:col                    0
-                                        :row                    row
-                                        :sizeX                  width
-                                        :sizeY                  height
-                                        :card                   left
-                                        :visualization_settings {}
-                                        :id                     (gensym)})
-           (update :ordered_cards conj {:col                    width
-                                        :row                    row
-                                        :sizeX                  width
-                                        :sizeY                  height
-                                        :card                   right
-                                        :visualization_settings {}
-                                        :id                     (gensym)}))))
-   (+ row height)])
+    (let [width (/ populate/grid-width 2)]
+      (-> dashboard
+          (update :ordered_cards conj {:col                    0
+                                       :row                    row
+                                       :sizeX                  width
+                                       :sizeY                  height
+                                       :card                   left
+                                       :visualization_settings {}
+                                       :id                     (gensym)})
+          (update :ordered_cards conj {:col                    width
+                                       :row                    row
+                                       :sizeX                  width
+                                       :sizeY                  height
+                                       :card                   right
+                                       :visualization_settings {}
+                                       :id                     (gensym)})))))
 
 (def ^:private ^Long title-height 2)
 
@@ -74,26 +78,28 @@
 
 (defn comparison-dashboard
   "Create a comparison dashboard based on dashboard `dashboard` comparing subsets of
-   the dataset defined by filter expressions `left` and `right`."
+   the dataset defined by segments `left` and `right`."
   [dashboard left right]
-  (let [dashboard {:name        (format "Comparison of %s and %s"
-                                        (:name left)
-                                        (:name right))
-                   :description (format "Automatically generated comparison dashboard comparing segments %s and %s"
-                                        (:name left)
-                                        (:name right))
-                   :creator_id  api/*current-user-id*
-                   :parameters  []}]
-    (->> dashboard
-         dashboard->cards
-         (mapcat unroll-multiseries)
-         (map (juxt (partial inject-segment left)
-                    (partial inject-segment right)))
-         (reduce (fn [[dashboard row] [left right]]
-                   (place-row dashboard row (:height left)
-                              (clone-card left)
-                              (clone-card right)))
-                 [(-> dashboard
-                      (add-col-title (:name left)  0)
-                      (add-col-title (:name right) (/ populate/grid-width 2)))
-                  title-height]))))
+  (->> dashboard
+       dashboard->cards
+       (mapcat unroll-multiseries)
+       (map (juxt (partial inject-segment left)
+                  (partial inject-segment right)))
+       (reduce (fn [[dashboard row] [left right]]
+                 (let [height (:height left)]
+                   [(place-row dashboard row height
+                               (clone-card left)
+                               (clone-card right))
+                    (+ row height)]))
+               [(-> {:name        (format "Comparison of %s and %s"
+                                          (:name left)
+                                          (:name right))
+                     :description (format "Automatically generated comparison dashboard comparing %s and %s"
+                                          (:name left)
+                                          (:name right))
+                     :creator_id  api/*current-user-id*
+                     :parameters  []}
+                    (add-col-title (:name left) 0)
+                    (add-col-title (:name right) (/ populate/grid-width 2)))
+                title-height])
+       first))
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index dca1d1bde45..94dbaac81b0 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -234,12 +234,15 @@
   (let [dashcards (:ordered_cards dashboard)
         dashboard (db/insert! Dashboard (dissoc dashboard :ordered_cards))]
     (doseq [dashcard dashcards]
-      (let [card     (db/insert! 'Card (:card dashcard))
+      (let [card     (some->> dashcard :card (db/insert! 'Card))
+            series   (some->> dashcard :series (map (partial db/insert! 'Card)))
             dashcard (-> dashcard
                          (dissoc :card :id)
                          (update :parameter_mappings
-                                 (partial map #(assoc % :card_id (:id card)))))]
-        (events/publish-event! :card-create card)
-        (hydrate card :creator :dashboard_count :labels :can_write :collection)
+                                 (partial map #(assoc % :card_id (:id card))))
+                         (assoc :series series))]
+        (doseq [card (concat series (some-> card vector))]
+          (events/publish-event! :card-create card)
+          (hydrate card :creator :dashboard_count :labels :can_write :collection))
         (add-dashcard! dashboard card dashcard)))
     dashboard))
-- 
GitLab