Merge pull request #8118 from metabase/save-xray-dashboards-to-collections

Save transient dashboard as a new (sub) collection

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

@@ -21,7 +21,6 @@ import DashboardData from "metabase/dashboard/hoc/DashboardData";
 import Parameters from "metabase/parameters/components/Parameters";
 
 import { getMetadata } from "metabase/selectors/metadata";
-import { getUserIsAdmin } from "metabase/selectors/user";
 
 import Dashboards from "metabase/entities/dashboards";
 import * as Urls from "metabase/lib/urls";
@@ -37,7 +36,6 @@ const getDashboardId = (state, { params: { splat }, location: { hash } }) =>
   `/auto/dashboard/${splat}${hash.replace(/^#?/, "?")}`;
 
 const mapStateToProps = (state, props) => ({
-  isAdmin: getUserIsAdmin(state),
   metadata: getMetadata(state),
   dashboardId: getDashboardId(state, props),
 });
@@ -99,7 +97,6 @@ class AutomaticDashboardApp extends React.Component {
       parameterValues,
       setParameterValue,
       location,
-      isAdmin,
     } = this.props;
     const { savedDashboardId } = this.state;
     // pull out "more" related items for displaying as a button at the bottom of the dashboard
@@ -125,7 +122,7 @@ class AutomaticDashboardApp extends React.Component {
               </div>
               {savedDashboardId != null ? (
                 <Button className="ml-auto" disabled>{t`Saved`}</Button>
-              ) : isAdmin ? (
+              ) : (
                 <ActionButton
                   className="ml-auto"
                   success
@@ -134,7 +131,7 @@ class AutomaticDashboardApp extends React.Component {
                 >
                   {t`Save this`}
                 </ActionButton>
-              ) : null}
+              )}
             </div>
           </div>
 

src/metabase/api/dashboard.clj

@@ -2,6 +2,7 @@
   "/api/dashboard endpoints."
   (:require [clojure.tools.logging :as log]
             [compojure.core :refer [DELETE GET POST PUT]]
+            [metabase.automagic-dashboards.populate :as magic.populate]
             [metabase
              [events :as events]
              [query-processor :as qp]
@@ -399,12 +400,22 @@
 
 ;;; ---------------------------------------------- Transient dashboards ----------------------------------------------
 
+(api/defendpoint POST "/save/collection/:parent-collection-id"
+  "Save a denormalized description of dashboard into collection with ID `:parent-collection-id`."
+  [parent-collection-id :as {dashboard :body}]
+  (collection/check-write-perms-for-collection parent-collection-id)
+  (->> (dashboard/save-transient-dashboard! dashboard parent-collection-id)
+       (events/publish-event! :dashboard-create)))
+
 (api/defendpoint POST "/save"
   "Save a denormalized description of dashboard."
   [:as {dashboard :body}]
-  (api/check-superuser)
-  (->> (dashboard/save-transient-dashboard! dashboard)
-       (events/publish-event! :dashboard-create)))
+  (let [parent-collection-id (if api/*is-superuser?*
+                               (:id (magic.populate/get-or-create-root-container-collection))
+                               (db/select-one-field :id 'Collection
+                                 :personal_owner_id api/*current-user-id*))]
+    (->> (dashboard/save-transient-dashboard! dashboard parent-collection-id)
+         (events/publish-event! :dashboard-create))))
 
 
 (api/define-routes)

src/metabase/automagic_dashboards/populate.clj

@@ -4,7 +4,9 @@
             [clojure.tools.logging :as log]
             [metabase.api.common :as api]
             [metabase.automagic-dashboards.filters :as filters]
-            [metabase.models.card :as card]
+            [metabase.models
+             [card :as card]
+             [collection :as collection]]
             [metabase.query-processor.util :as qp.util]
             [metabase.util :as u]
             [puppetlabs.i18n.core :as i18n :refer [trs]]
@@ -24,12 +26,23 @@
 
 (defn create-collection!
   "Create a new collection."
-  [title color description]
-  (when api/*is-superuser?*
-    (db/insert! 'Collection
-      :name        title
+  [title color description parent-collection-id]
+  (db/insert! 'Collection
+    (merge
+     {:name        title
       :color       color
-      :description description)))
+      :description description}
+     (when parent-collection-id
+       {:location (collection/children-location (db/select-one ['Collection :location :id]
+                                                  :id parent-collection-id))}))))
+
+(defn get-or-create-root-container-collection
+  "Get or create container collection for automagic dashboards in the root collection."
+  []
+  (or (db/select-one 'Collection
+        :name     "Automatically Generated Dashboards"
+        :location "/")
+      (create-collection! "Automatically Generated Dashboards" "#509EE3" nil nil)))
 
 (def colors
   "Colors used for coloring charts and collections."

src/metabase/models/dashboard.clj

@@ -12,6 +12,7 @@
             [metabase.automagic-dashboards.populate :as magic.populate]
             [metabase.models
              [card :as card :refer [Card]]
+             [collection :as collection]
              [dashboard-card :as dashboard-card :refer [DashboardCard]]
              [field-values :as field-values]
              [interface :as i]
@@ -243,28 +244,33 @@
            (str "Filtered by: ")))
 
 (defn- ensure-unique-collection-name
-  [collection]
-  (let [c (db/count 'Collection :name [:like (format "%s%%" collection)])]
+  [collection-name parent-collection-id]
+  (let [c (db/count 'Collection
+            :name     [:like (format "%s%%" collection-name)]
+            :location  (collection/children-location  (db/select-one ['Collection :location :id]
+                                                        :id parent-collection-id)))]
     (if (zero? c)
-      collection
-      (format "%s %s" collection (inc c)))))
+      collection-name
+      (format "%s %s" collection-name (inc c)))))
 
 (defn save-transient-dashboard!
   "Save a denormalized description of `dashboard`."
-  [dashboard]
+  [dashboard parent-collection-id]
   (let [dashcards  (:ordered_cards dashboard)
+        collection (magic.populate/create-collection!
+                    (ensure-unique-collection-name (:name dashboard) parent-collection-id)
+                    (rand-nth magic.populate/colors)
+                    "Automatically generated cards."
+                    parent-collection-id)
         dashboard  (db/insert! Dashboard
                      (-> dashboard
                          (dissoc :ordered_cards :rule :related :transient_name
                                  :transient_filters :param_fields :more)
-                         (assoc :description (->> dashboard
-                                                  :transient_filters
-                                                  applied-filters-blurb))))
-        collection (magic.populate/create-collection!
-                    (ensure-unique-collection-name
-                     (format "Questions for the dashboard \"%s\"" (:name dashboard)))
-                    (rand-nth magic.populate/colors)
-                    "Automatically generated cards.")]
+                         (assoc :description         (->> dashboard
+                                                          :transient_filters
+                                                          applied-filters-blurb)
+                                :collection_id       (:id collection)
+                                :collection_position 1)))]
     (doseq [dashcard dashcards]
       (let [card     (some-> dashcard :card (assoc :collection_id (:id collection)) save-card!)
             series   (some->> dashcard :series (map (fn [card]

test/metabase/models/dashboard_test.clj

@@ -235,9 +235,10 @@
                                                      users/user->id
                                                      user/permissions-set
                                                      atom)]
-      (let [dashboard (magic/automagic-analysis (Table (id :venues)) {})]
-        (->> dashboard
-             save-transient-dashboard!
+      (let [dashboard (magic/automagic-analysis (Table (id :venues)) {})
+            rastas-personal-collection (db/select-one-field :id 'Collection
+                                         :personal_owner_id api/*current-user-id*)]
+        (->> (save-transient-dashboard! dashboard rastas-personal-collection)
              :id
              (db/count 'DashboardCard :dashboard_id)
              (= (-> dashboard :ordered_cards count)))))))