revisions of dashboards

src/metabase/api/card.clj

@@ -47,12 +47,12 @@
    display      [Required CardDisplayType]}
   ;; TODO - which other params are required?
   (ins Card
-    :creator_id *current-user-id*
-    :dataset_query dataset_query
-    :description description
-    :display display
-    :name name
-    :public_perms public_perms
+    :creator_id             *current-user-id*
+    :dataset_query          dataset_query
+    :description            description
+    :display                display
+    :name                   name
+    :public_perms           public_perms
     :visualization_settings visualization_settings))
 
 (defendpoint GET "/:id"

src/metabase/api/dash.clj

@@ -8,7 +8,8 @@
                              [card :refer [Card]]
                              [common :as common]
                              [dashboard :refer [Dashboard]]
-                             [dashboard-card :refer [DashboardCard]])))
+                             [dashboard-card :refer [DashboardCard]]
+                             [revision :refer [push-revision]])))
 
 (defendpoint GET "/"
   "Get `Dashboards`. With filter option `f` (default `all`), restrict results as follows:
@@ -50,7 +51,7 @@
                                :description description
                                :name name
                                :public_perms public_perms))
-  (Dashboard id))
+  (push-revision :entity Dashboard, :object (Dashboard id)))
 
 (defendpoint DELETE "/:id"
   "Delete a `Dashboard`."
@@ -64,14 +65,18 @@
   {cardId [Required Integer]}
   (write-check Dashboard id)
   (check-400 (exists? Card :id cardId))
-  (ins DashboardCard :card_id cardId :dashboard_id id))
+  (let [result (ins DashboardCard :card_id cardId :dashboard_id id)]
+    (push-revision :entity Dashboard, :object (Dashboard id))
+    result))
 
 (defendpoint DELETE "/:id/cards"
   "Remove a `Card` from a `Dashboard`."
   [id dashcardId]
   {dashcardId [Required String->Integer]}
   (write-check Dashboard id)
-  (del DashboardCard :id dashcardId :dashboard_id id))
+  (let [result (del DashboardCard :id dashcardId :dashboard_id id)]
+    (push-revision :entity Dashboard, :object (Dashboard id))
+    result))
 
 (defendpoint POST "/:id/reposition"
   "Reposition `Cards` on a `Dashboard`. Request body should have the form:
@@ -83,10 +88,10 @@
               :col} ...]}"
   [id :as {{:keys [cards]} :body}]
   (write-check Dashboard id)
-  (dorun (map (fn [{:keys [card_id sizeX sizeY row col]}]
-                (let [{dashcard-id :id} (sel :one [DashboardCard :id] :card_id card_id :dashboard_id id)]
-                  (upd DashboardCard dashcard-id :sizeX sizeX :sizeY sizeY :row row :col col)))
-              cards))
+  (doseq [{:keys [card_id sizeX sizeY row col]} cards]
+    (let [{dashcard-id :id} (sel :one [DashboardCard :id] :card_id card_id :dashboard_id id)]
+      (upd DashboardCard dashcard-id :sizeX sizeX :sizeY sizeY :row row :col col)))
+  (push-revision :entity Dashboard, :object (Dashboard id))
   {:status :ok})
 
 (define-routes)

src/metabase/api/revision.clj

@@ -3,10 +3,12 @@
             [metabase.api.common :refer :all]
             [metabase.db :refer [exists?]]
             (metabase.models [card :refer [Card]]
+                             [dashboard :refer [Dashboard]]
                              [revision :as revision])))
 
 (def ^:private ^:const entity-kw->entity
-  {:card Card})
+  {:card      Card
+   :dashboard Dashboard})
 
 (defannotation Entity
   "Option must be a valid revisionable entity name. Returns corresponding entity."

src/metabase/models/dashboard.clj

 (ns metabase.models.dashboard
-  (:require [korma.core :refer :all, :exclude [defentity update]]
+  (:require (clojure [data :refer [diff]]
+                     [string :as s])
+            [korma.core :refer :all, :exclude [defentity update]]
+            [medley.core :as m]
             [metabase.db :refer :all]
             (metabase.models [common :refer :all]
                              [dashboard-card :refer [DashboardCard]]
                              [interface :refer :all]
+                             [revision :refer [IRevisioned]]
                              [user :refer [User]])
+            [metabase.models.revision.diff :refer [build-sentence]]
             [metabase.util :as u]))
 
 (defrecord DashboardInstance []
@@ -30,3 +35,66 @@
     (cascade-delete DashboardCard :dashboard_id id)))
 
 (extend-ICanReadWrite DashboardEntity :read :public-perms, :write :public-perms)
+
+(defn- serialize-instance [_ id {:keys [ordered_cards], :as dashboard}]
+  (-> dashboard
+      (select-keys [:description :name :public_perms])
+      (assoc :cards (for [card @ordered_cards]
+                      (select-keys card [:sizeX :sizeY :row :col :id :card_id])))))
+
+(defn- revert-to-revision [_ dashboard-id serialized-dashboard]
+  ;; Update the dashboard description / name / permissions
+
+  ;; Now update the cards as needed
+  (let [serialized-cards    (:cards serialized-dashboard)
+        id->serialized-card (zipmap (map :id serialized-cards) serialized-cards)
+        current-cards       (sel :many :fields [DashboardCard :sizeX :sizeY :row :col :id :card_id], :dashboard_id dashboard-id)
+        id->current-card    (zipmap (map :id current-cards) current-cards)
+        all-dashcard-ids    (concat (map :id serialized-cards)
+                                    (map :id current-cards))]
+    (doseq [dashcard-id all-dashcard-ids]
+      (let [serialized-card (id->serialized-card dashcard-id)
+            current-card    (id->current-card dashcard-id)]
+        (println "SERIALIZED CARD -> " serialized-card)
+        (cond
+          ;; If card is in current-cards but not serialized-cards then we need to delete it
+          (not serialized-card) (del DashboardCard :id dashcard-id)
+
+          ;; If card is in serialized-cards but not current-cards we need to add it
+          (not current-card) (m/mapply ins DashboardCard :dashboard_id dashboard-id, serialized-card)
+
+          ;; If card is in both we need to change :sizeX, :sizeY, :row, and :col to match serialized-card as needed
+          :else (let [[_ changes] (diff current-card serialized-card)]
+                  (println "Making these changes:" changes)
+                  (m/mapply upd DashboardCard dashcard-id changes))))))
+
+  serialized-dashboard)
+
+(defn- describe-diff [_ username dashboard₁ dashboard₂]
+  (let [[_ changes] (diff dashboard₁ dashboard₂)]
+    (println "CHANGES -> " changes)
+    (str (->> [(when (:name changes)
+                 (format "renamed it from \"%s\" to \"%s\"" (:name dashboard₁) (:name dashboard₂)))
+               (when (:description changes)
+                 (format "changed the description from \"%s\" to \"%s\"" (:description dashboard₁) (:description dashboard₂)))
+               (when (:public_perms changes)
+                 (if (zero? (:public_perms dashboard₂))
+                   "made it private"
+                   "made it public")) ; TODO - are both 1 and 2 "public" now ?
+               (when (:cards changes)
+                 (let [num-cards₁ (count (:cards dashboard₁))
+                       num-cards₂ (count (:cards dashboard₂))]
+                   (cond
+                     (> num-cards₁ num-cards₂) "removed a card"
+                     (< num-cards₁ num-cards₂) "added a card"
+                     :else                     "repositioned the cards")))]
+              (filter identity)
+              build-sentence
+              (apply str username " ")
+              (#(s/replace-first % "it " "this dashboard "))) \.)))
+
+(extend DashboardEntity
+  IRevisioned
+  {:serialize-instance serialize-instance
+   :revert-to-revision revert-to-revision
+   :describe-diff      describe-diff})

src/metabase/models/revision.clj

@@ -3,8 +3,7 @@
             [medley.core :as m]
             [metabase.db :refer [sel ins upd] :as db]
             [metabase.api.common :refer [*current-user-id* let-404]]
-            (metabase.models [card :refer [Card]]
-                             [hydrate :refer [hydrate]]
+            (metabase.models [hydrate :refer [hydrate]]
                              [interface :refer :all]
                              [user :refer [User]])
             [metabase.models.revision.diff :refer [diff-str]]

src/metabase/models/revision/diff.clj

@@ -26,7 +26,9 @@
     [_ _ _ _]
     (format "changed %s from \"%s\" to \"%s\"" (name k) v1 v2)))
 
-(defn- build-sentence [parts]
+(defn build-sentence
+  "Join parts of a sentence together to build a compound one."
+  [parts]
   (when (seq parts)
     (cond
       (= (count parts) 1) (str (first parts) \.)