Merge branch 'automagic-dashboards-improvments1' of...

Merge branch 'automagic-dashboards-improvments1' of github.com:metabase/metabase into automagic-dashboards-improvments1

frontend/src/metabase/setup/containers/PostSetupApp.jsx

@@ -163,7 +163,7 @@ export default class PostSetupApp extends Component {
                   description={
                     isSample
                       ? t`Once you connect your own data, I can show you some automatic explorations called x-rays. Here are some examples with sample data.`
-                      : t`I took a look at the data you just connected, and I put together some explorations of interesting metrics I found. Hope you like them!`
+                      : t`I took a look at the data you just connected, and I have some explorations of interesting thing I found. Hope you like them!`
                   }
                 />
               </BorderedPanel>

resources/automagic_dashboards/table/TransactionTable/ByCountry.yaml

-title: "A dive into [[GenericTable]] per country"
+title: "[[this]] per country"
 description: A deeper look at how different countries are performing for you.
 applies_to: TransactionTable
 metrics:

resources/automagic_dashboards/table/TransactionTable/ByProduct.yaml

-title: "Here's a breakdown of your [[GenericTable]] per product"
+title: "[[this]] per product"
 description: How your different products are performing.
 applies_to: TransactionTable
 metrics:

resources/automagic_dashboards/table/TransactionTable/BySource.yaml

-title: "An exploration of [[GenericTable]] per source"
+title: "[[GenericTable]] per source"
 description: Where most of your traffic is coming from.
 applies_to: TransactionTable
 metrics:
@@ -61,10 +61,12 @@ filters:
   - NewUsers:
       filter: ["time-interval", [dimension, Cohort], -30, day]
 groups:
+- Overview:
+    title: Transactions per source over time
 - Financial:
-    title: Financial
+    title: Orders and income per source
 - UserAcquisition:
-    title: User acquisition
+    title: Where users are coming from
 dashboard_filters:
 - SourceMedium
 - SourceLarge
@@ -75,7 +77,8 @@ dashboard_filters:
 - ProductCategory
 cards:
 - OrdersBySourceTimeseries:
-    title: Sales by source
+    group: Overview
+    title: Transactions per source
     visualization:
       area:
         stackable.stack_type: stacked
@@ -88,7 +91,7 @@ cards:
     height: 8
 - OrderBySource:
     group: Financial
-    title: Total orders by source
+    title: Total orders per source
     visualization: row
     dimensions: SourceLarge
     metrics: TotalOrders
@@ -98,7 +101,7 @@ cards:
     height: 8
 - IncomeBySource:
     group: Financial
-    title: Total income by source
+    title: Total income per source
     visualization: row
     dimensions: SourceMedium
     metrics: TotalIncome
@@ -108,7 +111,7 @@ cards:
     height: 8
 - AvgQuantityBySource:
     group: Financial
-    title: Average qunatity by source
+    title: Average quantity per source
     visualization: row
     dimensions: SourceMedium
     metrics: AvgQuantity
@@ -118,7 +121,7 @@ cards:
     height: 8
 - AvgIncomeBySource:
    group: Financial
-   title: Average income by source
+   title: Average income per source
    visualization: row
    dimensions: SourceMedium
    metrics: AvgIncome

resources/automagic_dashboards/table/TransactionTable/ByState.yaml

-title: "A look at [[GenericTable]] per state in the US"
+title: "[[this]] per state in the US"
 description: Which US states are bringing you the most business.
 applies_to: TransactionTable
 metrics:

resources/automagic_dashboards/table/TransactionTable/Seasonality.yaml

-title: "A closer look at [[GenericTable]] over time"
+title: "[[this]] over time"
 description: Whether or not there are any patterns to when they happen.
 applies_to: TransactionTable
 metrics:

src/metabase/api/automagic_dashboards.clj

@@ -25,8 +25,6 @@
 
 (def ^:private Show (s/maybe (s/enum "all")))
 
-(def ^:private Rule s/Str)
-
 (def ^:private Prefix (->> ["table" "metric" "field"]
                            (mapcat rules/load-rules)
                            (filter :indepth)
@@ -106,8 +104,8 @@
   (-> id
       Card
       api/check-404
-      (magic/automagic-analysis {:show        (keyword show)
-                                 :refinements (decode-base64-json cell-query)})))
+      (magic/automagic-analysis {:show       (keyword show)
+                                 :cell-query (decode-base64-json cell-query)})))
 
 (api/defendpoint GET "/question/:id/cell/:cell-query/:prefix/:rule"
   "Return an automagic dashboard analyzing cell in question  with id `id` defined by
@@ -120,9 +118,9 @@
   (-> id
       Card
       api/check-404
-      (magic/automagic-analysis {:show        (keyword show)
-                                 :rule        (load-rule "table" prefix rule)
-                                 :refinements (decode-base64-json cell-query)})))
+      (magic/automagic-analysis {:show       (keyword show)
+                                 :rule       (load-rule "table" prefix rule)
+                                 :cell-query (decode-base64-json cell-query)})))
 
 (api/defendpoint GET "/metric/:id"
   "Return an automagic dashboard analyzing metric with id `id`."
@@ -162,8 +160,8 @@
         cell-query (decode-base64-json cell-query)]
     (-> query
         query/adhoc-query
-        (magic/automagic-analysis {:show        (keyword show)
-                                   :refinements cell-query}))))
+        (magic/automagic-analysis {:show       (keyword show)
+                                   :cell-query cell-query}))))
 
 (def ^:private valid-comparison-pair?
   #{["segment" "segment"]

src/metabase/automagic_dashboards/core.clj

@@ -108,12 +108,12 @@
                     :else              [:field-id id])]
     (cond
       (isa? base_type :type/DateTime)
-      [:datetime-field reference (or aggregation (optimal-datetime-resolution field))]
+      [:datetime-field reference (or aggregation
+                                     (optimal-datetime-resolution field))]
 
       (and aggregation
            ; We don't handle binning on non-analyzed fields gracefully
-           (or (not (isa? base_type :type/Number))
-               (-> fingerprint :type :type/Number :min)))
+           (-> fingerprint :type :type/Number :min))
       [:binning-strategy reference aggregation]
 
       :else
@@ -145,7 +145,9 @@
 
 (defmethod ->reference :default
   [_ form]
-  form)
+  (or (cond-> form
+        (map? form) :name)
+      form))
 
 (defn- field-isa?
   [{:keys [base_type special_type]} t]
@@ -476,7 +478,7 @@
            :tables       (map #(assoc % :fields (table->fields %)) tables)
            :database     (:database root)
            :query-filter (merge-filter-clauses (:query-filter root)
-                                               (:refinements root))} context
+                                               (:cell-query root))} context
       (assoc context :dimensions (bind-dimensions context (:dimensions rule)))
       (assoc context :metrics (resolve-overloading context (:metrics rule)))
       (assoc context :filters (resolve-overloading context (:filters rule)))
@@ -501,11 +503,8 @@
   ([root rule context]
    (-> rule
        (select-keys [:title :description :groups :transient_title])
-       (cond->
-           (not (instance? (type Table) (:entity root)))
-         (assoc :title (str "Here's a closer look at your " (:full-name root))))
        (instantiate-metadata context {"this" (:entity root)})
-       (assoc :refinements (:refinements root)))))
+       (assoc :refinements (:cell-query root)))))
 
 (defn- apply-rule
   [root rule]
@@ -568,44 +567,42 @@
 
 (defn- automagic-dashboard
   "Create dashboards for table `root` using the best matching heuristics."
-  [{:keys [rule show rules-prefix] :as root}]
-  (if-let [[dashboard rule]
-           (if rule
-             (apply-rule root rule)
-             (->> root
-                  (matching-rules (rules/load-rules rules-prefix))
-                  (keep (partial apply-rule root))
-                  ;; `matching-rules` returns an `ArraySeq` (via `sort-by`) so
-                  ;; `first` realises one element at a time (no chunking).
-                  first))]
-    (do
-      (log/info (format "Applying heuristic %s to %s."
-                        (:rule rule)
-                        (:full-name root)))
-      (log/info (format "Dimensions bindings:\n%s"
-                        (->> dashboard
-                             :context
-                             :dimensions
-                             (m/map-vals #(update % :matches (partial map :name)))
-                             u/pprint-to-str)))
-      (log/info (format "Using definitions:\nMetrics:\n%s\nFilters:\n%s"
-                        (-> dashboard :context :metrics u/pprint-to-str)
-                        (-> dashboard :context :filters u/pprint-to-str)))
-      (-> (populate/create-dashboard dashboard (or show max-cards))
-          (assoc :related (-> (related root rule)
-                              (assoc :more (if (and (-> dashboard
-                                                        :cards
-                                                        count
-                                                        (> max-cards))
-                                                    (not= show :all))
-                                             [{:title       "Show more"
-                                               :description nil
-                                               :table       (:source-table root)
-                                               :url         (format "%s#show=all"
-                                                                    (:url root))}]
-                                             []))))))
-    (log/info (format "Skipping %s: no cards fully match bound dimensions."
-                      (:full-name root)))))
+  [{:keys [rule show rules-prefix query-filter cell-query full-name] :as root}]
+  (let [[dashboard rule] (if rule
+                           (apply-rule root rule)
+                           (->> root
+                                (matching-rules (rules/load-rules rules-prefix))
+                                (keep (partial apply-rule root))
+                                ;; `matching-rules` returns an `ArraySeq`
+                                ;; (via `sort-by`) so `first` realises one element
+                                ;; at a time (no chunking).
+                                first))]
+    (log/info (format "Applying heuristic %s to %s." (:rule rule) full-name))
+    (log/info (format "Dimensions bindings:\n%s"
+                      (->> dashboard
+                           :context
+                           :dimensions
+                           (m/map-vals #(update % :matches (partial map :name)))
+                           u/pprint-to-str)))
+    (log/info (format "Using definitions:\nMetrics:\n%s\nFilters:\n%s"
+                      (-> dashboard :context :metrics u/pprint-to-str)
+                      (-> dashboard :context :filters u/pprint-to-str)))
+    (-> (cond-> dashboard
+          (or query-filter cell-query)
+          (assoc :title (str "Here's a closer look at your " full-name)))
+        (populate/create-dashboard (or show max-cards))
+        (assoc :related (-> (related root rule)
+                            (assoc :more (if (and (-> dashboard
+                                                      :cards
+                                                      count
+                                                      (> max-cards))
+                                                  (not= show :all))
+                                           [{:title       "Show more"
+                                             :description nil
+                                             :table       (:source-table root)
+                                             :url         (format "%s#show=all"
+                                                                  (:url root))}]
+                                           [])))))))
 
 (def ^:private ^{:arglists '([card])} table-like?
   (comp empty? :aggregation :query :dataset_query))
@@ -628,39 +625,49 @@
   [metric opts]
   (automagic-dashboard (merge opts (->root metric))))
 
+(def ^:private ^{:arglists '([x])} endocde-base64-json
+  (comp codec/base64-encode codecs/str->bytes json/encode))
+
 (defmethod automagic-analysis (type Card)
-  [card opts]
-  (if (table-like? card)
+  [card {:keys [cell-query] :as opts}]
+  (if (or (table-like? card)
+          cell-query)
     (let [table (-> card :table_id Table)]
       (automagic-dashboard
        (merge opts
-              {:entity       card
+              {:entity       table
                :source-table table
                :query-filter (-> card :dataset_query :query :filter)
                :database     (:db_id table)
                :full-name    (str (:name card) " question")
-               :url          (format "%squestion/%s" public-endpoint (:id card))
+               :url          (if cell-query
+                               (format "%squestion/%s/cell/%s" public-endpoint
+                                       (:id card)
+                                       (endocde-base64-json cell-query))
+                               (format "%squestion/%s" public-endpoint (:id card)))
                :rules-prefix "table"})))
     nil))
 
 (defmethod automagic-analysis (type Query)
-  [query opts]
-  (if (table-like? query)
+  [query {:keys [cell-query] :as opts}]
+  (if (or (table-like? query)
+          (:cell-query opts))
     (let [table (-> query :table-id Table)]
       (automagic-dashboard
-       (merge (update opts :refinements merge-filter-clauses (-> query
-                                                                 :dataset_query
-                                                                 :query
-                                                                 :filter))
-              {:entity       query
+       (merge (update opts :cell-query merge-filter-clauses (-> query
+                                                                :dataset_query
+                                                                :query
+                                                                :filter))
+              {:entity       table
                :source-table table
                :database     (:db_id table)
                :full-name    (:display_name table)
-               :url          (format "%sadhoc/%s" public-endpoint
-                                     (-> query
-                                         json/encode
-                                         codecs/str->bytes
-                                         codec/base64-encode))
+               :url          (if cell-query
+                               (format "%sadhoc/%s/cell/%s" public-endpoint
+                                       (endocde-base64-json query)
+                                       (endocde-base64-json cell-query))
+                               (format "%sadhoc/%s" public-endpoint
+                                                   (endocde-base64-json query)))
                :rules-prefix "table"})))
     nil))
 

src/metabase/automagic_dashboards/populate.clj

@@ -205,8 +205,6 @@
   [dashboard grid group cards]
   (let [start-row (bottom-row grid)
         start-row (cond-> start-row
-                    ;; First row doesn't need empty space above
-                    (pos? start-row) inc
                     group            (+ group-heading-height))]
     (reduce (fn [[dashboard grid] card]
               (let [xy (card-position grid start-row card)]
@@ -217,10 +215,10 @@
             (if group
               (let [xy   [(- start-row 2) 0]
                     card {:text                   (format "# %s" (:title group))
-                          :width                  (* 2  default-card-width)
+                          :width                  grid-width
                           :height                 group-heading-height
                           :visualization-settings {:dashcard.background false
-                                                   :text.align_vertical :center}}]
+                                                   :text.align_vertical :bottom}}]
                 [(add-text-card dashboard card xy)
                  (fill-grid grid xy card)])
               [dashboard grid])