Merge branch 'xray-candidates-grouped-by-schema' of...

Merge branch 'xray-candidates-grouped-by-schema' of github.com:metabase/metabase into automagic-dashboards-improvments1

frontend/src/metabase/components/ExplorePane.jsx

@@ -10,13 +10,13 @@ import Select, { Option } from "metabase/components/Select";
 import { t } from "c-3po";
 import _ from "underscore";
 
-import type { Candidate } from "metabase/meta/types/Auto";
+import type { DatabaseCandidates, Candidate } from "metabase/meta/types/Auto";
 
 const DEFAULT_TITLE = t`Hi, Metabot here.`;
 const DEFAULT_DESCRIPTION = "";
 
 type Props = {
-  candidates?: ?(Candidate[]),
+  candidates?: ?DatabaseCandidates,
   title?: ?string,
   description?: ?string,
 };
@@ -43,20 +43,15 @@ export class ExplorePane extends React.Component {
     let { schemaName, visibleItems } = this.state;
 
     let schemaNames;
+    let tables;
     let hasMore = false;
-    if (candidates) {
-      const candidatesBySchema =
-        candidates &&
-        _.groupBy(
-          candidates,
-          candidate => candidate.table && candidate.table.schema,
-        );
-      schemaNames = Object.keys(candidatesBySchema || {});
+    if (candidates && candidates.length > 0) {
+      schemaNames = candidates.map(schema => schema.schema);
       if (schemaName == null) {
         schemaName = schemaNames[0];
       }
-      candidates = candidatesBySchema[schemaName].slice(0, visibleItems);
-      hasMore = visibleItems < candidatesBySchema[schemaName].length;
+      const schema = _.findWhere(candidates, { schema: schemaName });
+      tables = (schema && schema.tables) || [];
     }
 
     return (
@@ -99,9 +94,9 @@ export class ExplorePane extends React.Component {
               </Select>
             </div>
           )}
-        {candidates && (
+        {tables && (
           <div className="px4">
-            <ExploreList candidates={candidates} />
+            <ExploreList candidates={tables} />
           </div>
         )}
         {hasMore && (

frontend/src/metabase/dashboard/containers/AutomaticDashboardApp.jsx

@@ -75,6 +75,13 @@ const mapStateToProps = (state, props) => ({
 @connect(mapStateToProps, { addUndo, createUndo })
 @DashboardData
 class AutomaticDashboardApp extends React.Component {
+  componentDidUpdate(prevProps) {
+    // scroll to the top when the pathname changes
+    if (prevProps.location.pathname !== this.props.location.pathname) {
+      window.scrollTo(0, 0);
+    }
+  }
+
   save = async () => {
     const { dashboard, addUndo, createUndo } = this.props;
     // remove the transient id before trying to save

frontend/src/metabase/meta/types/Auto.js

@@ -2,8 +2,19 @@
 
 import type { TableId, SchemaName } from "metabase/meta/types/Table";
 
+export type DatabaseCandidates = SchemaCandidates[];
+
+export type SchemaCandidates = {
+  schema: SchemaName,
+  score: number,
+  tables: Candidate[],
+};
+
 export type Candidate = {
   title: string,
+  description: string,
+  score: number,
+  rule: string,
   url: string,
   table?: {
     id: TableId,

frontend/src/metabase/services.js

@@ -14,7 +14,7 @@ import getGAMetadata from "promise-loader?global!metabase/lib/ga-metadata"; // e
 import type { Data, Options } from "metabase/lib/api";
 
 import type { DatabaseId } from "metabase/meta/types/Database";
-import type { Candidate } from "metabase/meta/types/Auto";
+import type { DatabaseCandidates } from "metabase/meta/types/Auto";
 import type { DashboardWithCards } from "metabase/meta/types/Dashboard";
 
 export const ActivityApi = {
@@ -95,7 +95,7 @@ export const EmbedApi = {
 
 type $AutoApi = {
   dashboard: ({ subPath: string }) => DashboardWithCards,
-  db_candidates: ({ id: DatabaseId }) => Candidate[],
+  db_candidates: ({ id: DatabaseId }) => DatabaseCandidates,
 };
 
 export const AutoApi: $AutoApi = {

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

@@ -26,7 +26,7 @@ const QUOTES = [
   t`Metabot is feeling pretty good about all this…`,
 ];
 
-import type { Candidate } from "metabase/meta/types/Auto";
+import type { DatabaseCandidates } from "metabase/meta/types/Auto";
 
 type Props = {
   params: {
@@ -36,8 +36,8 @@ type Props = {
 type State = {
   databaseId: ?number,
   isSample: ?boolean,
-  candidates: ?(Candidate[]),
-  sampleCandidates: ?(Candidate[]),
+  candidates: ?DatabaseCandidates,
+  sampleCandidates: ?DatabaseCandidates,
 };
 
 @withBackground("bg-slate-extra-light")

src/metabase/automagic_dashboards/core.clj

@@ -10,7 +10,9 @@
             [clojure.string :as str]
             [clojure.tools.logging :as log]
             [clojure.walk :as walk]
-            [kixi.stats.core :as stats]
+            [kixi.stats
+             [core :as stats]
+             [math :as math]]
             [medley.core :as m]
             [metabase.automagic-dashboards
              [populate :as populate]
@@ -668,26 +670,27 @@
   [field opts]
   (automagic-dashboard (merge opts (->root field))))
 
-(defn- link-table?
-  "Is the table comprised only of foregin keys and maybe a primary key?"
+(defn- enhanced-table-stats
   [table]
-  (zero? (db/count Field
-           ;; :not-in returns false if field is nil, hence the workaround.
-           {:where [:and [:= :table_id (:id table)]
-                         [:or [:not-in :special_type ["type/FK" "type/PK"]]
-                              [:= :special_type nil]]]})))
-
-(defn- list-like-table?
-  "Is the table comprised of only primary key and single field?"
-  [table]
-  (= 1 (db/count Field
-         ;; :not-in returns false if field is nil, hence the workaround.
-         {:where [:and [:= :table_id (:id table)]
-                  [:not= :special_type "type/PK"]]})))
+  (let [field-types (db/select-field :special_type Field :table_id (:id table))]
+    (assoc table :stats {:num-fields  (count field-types)
+                         :list-like?  (= (count (remove #{:type/PK} field-types)) 1)
+                         :link-table? (every? #{:type/FK :type/PK} field-types)})))
+
+(def ^:private ^:const ^Long max-candidate-tables
+  "Maximal number of tables shown per schema."
+  10)
 
 (defn candidate-tables
   "Return a list of tables in database with ID `database-id` for which it makes sense
-   to generate an automagic dashboard."
+   to generate an automagic dashboard. Results are grouped by schema and ranked
+   acording to interestingness (both schemas and tables within each schema). Each
+   schema contains up to `max-candidate-tables` tables.
+
+   Tables are ranked based on how specific rule has been used, and the number of
+   fields.
+   Schemes are ranked based on the number of distinct entity types and the
+   interestingness of tables they contain (see above)."
   ([database] (candidate-tables database nil))
   ([database schema]
    (let [rules (rules/load-rules "table")]
@@ -695,17 +698,35 @@
                  (cond-> [:db_id           (:id database)
                           :visibility_type nil]
                    schema (concat [:schema schema])))
-          (remove (some-fn link-table? list-like-table?))
-          (keep (fn [table]
-                  (let [root (->root table)]
-                    (when-let [[dashboard rule]
-                               (->> root
-                                    (matching-rules rules)
-                                    (keep (partial apply-rule root))
-                                    first)]
-                      {:url         (format "%stable/%s" public-endpoint (:id table))
-                       :title       (:title dashboard)
-                       :score       (rule-specificity rule)
-                       :description (:description dashboard)
-                       :table       table}))))
+          (map enhanced-table-stats)
+          (remove (comp (some-fn :link-table? :list-like-table?) :stats))
+          (map (fn [table]
+                 (let [root      {:entity       table
+                                  :source-table table
+                                  :database     (:db_id table)
+                                  :rules-prefix "table"}
+                       rule      (->> root
+                                      (matching-rules rules)
+                                      first)
+                       dashboard (make-dashboard root rule {})]
+                   {:url         (format "%stable/%s" public-endpoint (:id table))
+                    :title       (:title dashboard)
+                    :score       (+ (math/sq (rule-specificity rule))
+                                    (math/log (-> table :stats :num-fields)))
+                    :description (:description dashboard)
+                    :table       table
+                    :rule        (:rule rule)})))
+          (group-by (comp :schema :table))
+          (map (fn [[schema tables]]
+                 (let [tables (->> tables
+                                   (sort-by :score >)
+                                   (take max-candidate-tables))]
+                   {:tables tables
+                    :schema schema
+                    :score  (+ (math/sq (transduce (m/distinct-by :rule)
+                                                   stats/count
+                                                   tables))
+                               (math/sqrt (transduce (map (comp math/sq :score))
+                                                     stats/mean
+                                                     tables)))})))
           (sort-by :score >)))))