From 366c2a52d153961e911445cc97c6d0b9028ae1e0 Mon Sep 17 00:00:00 2001
From: Simon Belak <simon@metabase.com>
Date: Wed, 12 Jun 2019 10:15:02 -0700
Subject: [PATCH] xrays: Add smart-row visualization option (#9625)

Add smart-row visualization option
---
 .../table/EventTable.yaml                     | 18 +-----
 .../table/GenericTable.yaml                   | 20 +------
 .../table/TransactionTable.yaml               | 58 ++-----------------
 .../table/TransactionTable/ByProduct.yaml     | 18 +-----
 .../table/TransactionTable/BySource.yaml      | 24 ++------
 .../automagic_dashboards/table/UserTable.yaml | 22 +------
 src/metabase/automagic_dashboards/core.clj    | 42 ++++++++------
 .../visualization_macros.clj                  | 29 ++++++++++
 8 files changed, 72 insertions(+), 159 deletions(-)
 create mode 100644 src/metabase/automagic_dashboards/visualization_macros.clj

diff --git a/resources/automagic_dashboards/table/EventTable.yaml b/resources/automagic_dashboards/table/EventTable.yaml
index 906db58574b..ca460c5bf50 100644
--- a/resources/automagic_dashboards/table/EventTable.yaml
+++ b/resources/automagic_dashboards/table/EventTable.yaml
@@ -13,10 +13,6 @@ dimensions:
   - GenericCategoryMedium:
       field_type: GenericTable.Category
       score: 75
-      max_cardinality: 10
-  - GenericCategoryLarge:
-      field_type: GenericTable.Category
-      score: 70
   - State: GenericTable.State
   - Country: GenericTable.Country
   - Long: GenericTable.Longitude
@@ -90,22 +86,10 @@ cards:
       title: Events per [[GenericCategoryMedium]]
       dimensions: GenericCategoryMedium
       metrics: Count
-      visualization: row
+      visualization: smart-row
       score: 80
       height: 8
       group: General
-      order_by:
-        - Count: descending
-  - CountByCategoryLarge:
-      title: Events per [[GenericCategoryLarge]]
-      dimensions: GenericCategoryLarge
-      metrics: Count
-      visualization: table
-      height: 8
-      score: 70
-      group: General
-      order_by:
-        - Count: descending
 # Geographical
   - EventsByCountry:
       title: Events per country
diff --git a/resources/automagic_dashboards/table/GenericTable.yaml b/resources/automagic_dashboards/table/GenericTable.yaml
index 91a46c20e52..05c500cc048 100644
--- a/resources/automagic_dashboards/table/GenericTable.yaml
+++ b/resources/automagic_dashboards/table/GenericTable.yaml
@@ -5,8 +5,7 @@ metrics:
   - Count: ["count"]
   - CountDistinctFKs: [distinct, [dimension, FK]]
   - Sum: [sum, [dimension, GenericNumber]]
-  - Avg:
-      metric: [avg, [dimension, GenericNumber]]
+  - Avg: [avg, [dimension, GenericNumber]]
 dimensions:
   - Country:
       field_type: GenericTable.Country
@@ -24,9 +23,6 @@ dimensions:
       field_type: GenericTable.Category
       score: 75
       max_cardinality: 10
-  - GenericCategoryLarge:
-      field_type: GenericTable.Category
-      score: 70
   - Singleton:
       field_type: GenericTable.Category
       max_cardinality: 1
@@ -143,22 +139,10 @@ cards:
       title: "[[this.short-name]] per [[GenericCategoryMedium]]"
       dimensions: GenericCategoryMedium
       metrics: Count
-      visualization: row
+      visualization: smart-row
       score: 80
       height: 8
       group: General
-      order_by:
-        - Count: descending
-  - CountByCategoryLarge:
-      title: "[[this.short-name]] per [[GenericCategoryLarge]]"
-      dimensions: GenericCategoryLarge
-      metrics: Count
-      visualization: table
-      height: 8
-      score: 70
-      group: General
-      order_by:
-        - Count: descending
 # Geographical
   - CountByCountry:
       title: "[[this.short-name]] per country"
diff --git a/resources/automagic_dashboards/table/TransactionTable.yaml b/resources/automagic_dashboards/table/TransactionTable.yaml
index e2892ee66a4..ef256f11133 100644
--- a/resources/automagic_dashboards/table/TransactionTable.yaml
+++ b/resources/automagic_dashboards/table/TransactionTable.yaml
@@ -42,10 +42,7 @@ dimensions:
     max_cardinality: 5
 - SourceMedium:
     field_type: GenericTable.Source
-    max_cardinality: 10
     score: 90
-- SourceLarge:
-    field_type: GenericTable.Source
 - Cohort:
     field_type: UserTable.JoinTimestamp
     score: 100
@@ -64,22 +61,14 @@ dimensions:
     max_cardinality: 10
 - ProductMedium:
     field_type: ProductTable.Title
-    max_cardinality: 10
-- ProductLarge:
-    field_type: Product
-    score: 90
-- ProductLarge:
+- ProductMedium:
     field_type: ProductTable.Name
-- ProductLarge:
+- ProductMedium:
     field_type: ProductTable.Title
 - ProductCategoryMedium:
     field_type: ProductTable.Category
     named: category
-    max_cardinality: 10
     score: 90
-- ProductCategoryLarge:
-    field_type: ProductTable.Category
-    named: category
 - Long: GenericTable.Longitude
 - Lat: GenericTable.Latitude
 filters:
@@ -162,45 +151,19 @@ cards:
       group: General
   - OrdersByProduct:
       title: Sales per product
-      visualization: row
+      visualization: smart-row
       dimensions:
         - ProductMedium
       metrics: TotalOrders
-      order_by:
-        - TotalOrders: descending
-      score: 90
-      height: 8
-      group: General
-  - OrdersByProduct:
-      title: Sales per product
-      visualization: table
-      dimensions:
-        - ProductLarge
-      metrics: TotalOrders
-      order_by:
-        - TotalOrders: descending
       score: 90
       height: 8
       group: General
   - OrdersByProductCategory:
       title: Sales for each product category
-      visualization: row
+      visualization: smart-row
       dimensions:
         - ProductCategoryMedium
       metrics: TotalOrders
-      order_by:
-        - TotalOrders: descending
-      score: 90
-      height: 8
-      group: General
-  - OrdersByProductCategory:
-      title: Sales for each product category
-      visualization: table
-      dimensions:
-        - ProductCategoryLarge
-      metrics: TotalOrders
-      order_by:
-        - TotalOrders: descending
       score: 90
       height: 8
       group: General
@@ -219,23 +182,12 @@ cards:
       group: General
   - OrdersBySource:
       title: Sales per source
-      visualization: row
+      visualization: smart-row
       dimensions: SourceMedium
       metrics: TotalOrders
-      order_by:
-        - TotalOrders: descending
       score: 90
       height: 8
       group: General
-  - OrdersBySource:
-      title: Sales per source
-      visualization: table
-      dimensions: SourceLarge
-      metrics: TotalOrders
-      order_by:
-        - TotalOrders: descending
-      score: 90
-      group: General
 # Geographical
   - CountByCountry:
       title: Sales per country
diff --git a/resources/automagic_dashboards/table/TransactionTable/ByProduct.yaml b/resources/automagic_dashboards/table/TransactionTable/ByProduct.yaml
index 8b9d34f0715..178a6ee2fcf 100644
--- a/resources/automagic_dashboards/table/TransactionTable/ByProduct.yaml
+++ b/resources/automagic_dashboards/table/TransactionTable/ByProduct.yaml
@@ -7,9 +7,6 @@ metrics:
 dimensions:
 - ProductCategoryMedium:
     field_type: ProductTable.Category
-    max_cardinality: 10
-- ProductCategoryLarge:
-    field_type: ProductTable.Category
 - Rating: ProductTable.Score
 - Country: GenericTable.Country
 - State: GenericTable.State
@@ -47,19 +44,8 @@ cards:
     height: 8
 - OrdersByProductCategoryMedium:
     title: Sales per product [[ProductCategoryMedium]]
-    visualization: row
+    visualization: smart-row
     dimensions:
       - ProductCategoryMedium
     metrics: TotalOrders
-    order_by:
-     - TotalOrders: descending
-    height: 8
-- OrdersByProductCategoryLarge:
-    title: Sales per product [[ProductCategoryLarge]]
-    visualization: table
-    dimensions:
-      - ProductCategoryLarge
-    metrics: TotalOrders
-    order_by:
-     - TotalOrders: descending
-    height: 8
+    height: 8
\ No newline at end of file
diff --git a/resources/automagic_dashboards/table/TransactionTable/BySource.yaml b/resources/automagic_dashboards/table/TransactionTable/BySource.yaml
index 0f2a4793a85..db1b7f52516 100644
--- a/resources/automagic_dashboards/table/TransactionTable/BySource.yaml
+++ b/resources/automagic_dashboards/table/TransactionTable/BySource.yaml
@@ -101,64 +101,52 @@ cards:
 - OrderBySource:
     group: Financial
     title: Total orders per source
-    visualization: row
+    visualization: smart-row
     dimensions: SourceLarge
     metrics: TotalOrders
-    order_by:
-      - TotalOrders: descending
     score: 80
     height: 8
 - IncomeBySource:
     group: Financial
     title: Total income per source
-    visualization: row
+    visualization: smart-row
     dimensions: SourceMedium
     metrics: TotalIncome
-    order_by:
-      - TotalIncome: descending
     score: 80
     height: 8
 - AvgQuantityBySource:
     group: Financial
     title: Average quantity per source
-    visualization: row
+    visualization: smart-row
     dimensions: SourceMedium
     metrics: AvgQuantity
-    order_by:
-      - AvgQuantity: descending
     score: 80
     height: 8
 - AvgIncomeBySource:
    group: Financial
    title: Average income per source
-   visualization: row
+   visualization: smart-row
    dimensions: SourceMedium
    metrics: AvgIncome
-   order_by:
-     - AvgIncome: descending
    score: 80
    height: 8
 - UsersBySource:
     group: UserAcquisition
-    visualization: row
+    visualization: smart-row
     title: Number of users per source
     dimensions: SourceMedium
     metrics: TotalUsers
-    order_by:
-      - TotalUsers: descending
     score: 90
     height: 8
 - NewUsersBySource:
     group: UserAcquisition
-    visualization: row
+    visualization: smart-row
     title: New users per source in the last 30 days
     dimensions: SourceMedium
     metrics: TotalUsers
     filters: NewUsers
     score: 90
     height: 8
-    order_by:
-      - TotalUsers: descending
 - IncomeBySource:
     group: Financial
     title: Total income per source
diff --git a/resources/automagic_dashboards/table/UserTable.yaml b/resources/automagic_dashboards/table/UserTable.yaml
index c18b191c0af..81f82c21a78 100644
--- a/resources/automagic_dashboards/table/UserTable.yaml
+++ b/resources/automagic_dashboards/table/UserTable.yaml
@@ -31,10 +31,6 @@ dimensions:
 - GenericCategoryMedium:
     field_type: GenericTable.Category
     score: 75
-    max_cardinality: 10
-- GenericCategoryLarge:
-    field_type: GenericTable.Category
-    score: 70
 - State:
     field_type: State
 - Country: Country
@@ -142,12 +138,10 @@ cards:
       title: "Per [[GenericCategoryMedium]]"
       dimensions: GenericCategoryMedium
       metrics: Count
-      visualization: row
-      score: 80
+      visualization: smart-row
+      score: 70
       height: 8
       group: General
-      order_by:
-        - Count: descending
   - CountBySource:
       title: "Per [[Source]]"
       dimensions: Source
@@ -157,14 +151,4 @@ cards:
       height: 8
       group: General
       order_by:
-        - Count: descending
-  - CountByCategoryLarge:
-      title: "Per [[GenericCategoryLarge]]"
-      dimensions: GenericCategoryLarge
-      metrics: Count
-      visualization: table
-      height: 8
-      score: 70
-      group: General
-      order_by:
-        - Count: descending
+        - Count: descending
\ No newline at end of file
diff --git a/src/metabase/automagic_dashboards/core.clj b/src/metabase/automagic_dashboards/core.clj
index aac9ba0fb6b..58c2995c8ac 100644
--- a/src/metabase/automagic_dashboards/core.clj
+++ b/src/metabase/automagic_dashboards/core.clj
@@ -22,7 +22,8 @@
             [metabase.automagic-dashboards
              [filters :as filters]
              [populate :as populate]
-             [rules :as rules]]
+             [rules :as rules]
+             [visualization-macros :as visualization]]
             [metabase.mbql
              [normalize :as normalize]
              [util :as mbql.u]]
@@ -488,9 +489,9 @@
               {})))
 
 (defn- build-order-by
-  [dimensions metrics order-by]
-  (let [dimensions (set dimensions)]
-    (for [[identifier ordering] (map first order-by)]
+  [{:keys [dimensions metrics order_by]}]
+  (let [dimensions (into #{} (map ffirst) dimensions)]
+    (for [[identifier ordering] (map first order_by)]
       [(if (= ordering "ascending")
          :asc
          :desc)
@@ -605,9 +606,8 @@
 (defn- card-candidates
   "Generate all potential cards given a card definition and bindings for
    dimensions, metrics, and filters."
-  [context {:keys [metrics filters dimensions score limit order_by query] :as card}]
-  (let [order_by        (build-order-by dimensions metrics order_by)
-        metrics         (map (partial get (:metrics context)) metrics)
+  [context {:keys [metrics filters dimensions score limit query] :as card}]
+  (let [metrics         (map (partial get (:metrics context)) metrics)
         filters         (cond-> (map (partial get (:filters context)) filters)
                           (:query-filter context) (conj {:filter (:query-filter context)}))
         score           (if query
@@ -633,23 +633,29 @@
                         (every? (every-pred valid-breakout-dimension?
                                             (complement (comp cell-dimension? id-or-name)))))))
          (map (fn [bindings]
-                (let [query (if query
-                              (build-query context bindings query)
-                              (build-query context bindings
-                                           filters
-                                           metrics
-                                           dimensions
-                                           limit
-                                           order_by))]
+                (let [metrics (for [metric metrics]
+                                {:name   ((some-fn :name (comp metric-name :metric)) metric)
+                                 :metric (:metric metric)
+                                 :op     (-> metric :metric metric-op)})
+                      card    (visualization/expand-visualization
+                               card
+                               (map (comp bindings second) dimensions)
+                               metrics)
+                      query   (if query
+                                (build-query context bindings query)
+                                (build-query context bindings
+                                             filters
+                                             metrics
+                                             dimensions
+                                             limit
+                                             (build-order-by card)))]
                   (-> card
                       (instantiate-metadata context (->> metrics
                                                          (map :name)
                                                          (zipmap (:metrics card))
                                                          (merge bindings)))
                       (assoc :dataset_query query
-                             :metrics       (for [metric metrics]
-                                              {:name ((some-fn :name (comp metric-name :metric)) metric)
-                                               :op   (-> metric :metric metric-op)})
+                             :metrics       metrics
                              :dimensions    (map (comp :name bindings second) dimensions)
                              :score         score))))))))
 
diff --git a/src/metabase/automagic_dashboards/visualization_macros.clj b/src/metabase/automagic_dashboards/visualization_macros.clj
new file mode 100644
index 00000000000..9fc43d3e977
--- /dev/null
+++ b/src/metabase/automagic_dashboards/visualization_macros.clj
@@ -0,0 +1,29 @@
+(ns metabase.automagic-dashboards.visualization-macros)
+
+(defmulti expand-visualization
+  "Expand visualization macro."
+  (fn [card _ _]
+    (-> card :visualization first)))
+
+(def ^:private ^:const ^Long smart-row-table-threshold 10)
+
+(defmethod expand-visualization "smart-row"
+  [card dimensions metrics]
+  (let [[display settings] (:visualization card)]
+    (-> card
+        (assoc :visualization (if (->> dimensions
+                                       (keep #(get-in % [:fingerprint :global :distinct-count]))
+                                       (apply max 0)
+                                       (>= smart-row-table-threshold))
+                                ["row" settings]
+                                ["table" (merge {:column_settings {(->> metrics
+                                                                        first
+                                                                        :op
+                                                                        (format "[\"name\",\"%s\"]")
+                                                                        keyword) {:show_mini_bar true}}}
+                                                settings)]))
+        (update :order_by #(or % [{(-> card :metrics first) "descending"}])))))
+
+(defmethod expand-visualization :default
+  [card _ _]
+  card)
-- 
GitLab