From b7180693544069490f37c076adc0b8d8365ed6dd Mon Sep 17 00:00:00 2001
From: dpsutton <dan@dpsutton.com>
Date: Tue, 4 May 2021 11:35:22 -0500
Subject: [PATCH] Collections metadata backend work (#15718)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Provide `:last-edit-info` information on collection items
only provided for cards and dashboards. At the moment, collections can
have:
- cards (☑ï¸)
- dashboards (☑ï¸)
- snippets (âŒ)(not in UI)
- pulses (âŒ)(deprecated)
- other collection (💪 to be done. needs a table for these
changes. they are revisioned)
* Send edit info along with cards
* Last-edit-info on dashboards
* Move last-edit-info functions into a bespoke namespace under revision
Under revision as the source of changes all come from revisions at the
moment. This might be ill-considered once we add collections into the
mix since they are not good candidates for revisions.
* Renames and docstring update on the last-edit namespace
* Add types
* Add `:last-edit-info` on card and dashboard creation
* Expect last-edit-info in creation repsonse for cards/dashboard
* Docstring on last-edit types
* Make the namespace checker happy
* namespace checker
* Remove moved function
* Add edit info to api/card/ and api/dashboard/
* Move event emission outside of transaction
---
src/metabase/api/card.clj | 27 ++++--
src/metabase/api/collection.clj | 36 +++++---
src/metabase/api/dashboard.clj | 37 ++++++---
src/metabase/models/revision/last_edit.clj | 95 ++++++++++++++++++++++
test/metabase/api/card_test.clj | 27 +++++-
test/metabase/api/collection_test.clj | 26 +++++-
test/metabase/api/dashboard_test.clj | 48 ++++++++---
test/metabase/test/util.clj | 2 +-
8 files changed, 248 insertions(+), 50 deletions(-)
create mode 100644 src/metabase/models/revision/last_edit.clj
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index f2525ac1050..02e878cd225 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -19,6 +19,7 @@
[metabase.models.pulse :as pulse :refer [Pulse]]
[metabase.models.query :as query]
[metabase.models.query.permissions :as query-perms]
+ [metabase.models.revision.last-edit :as last-edit]
[metabase.models.table :refer [Table]]
[metabase.models.view-log :refer [ViewLog]]
[metabase.public-settings :as public-settings]
@@ -152,16 +153,22 @@
(case f
:database (api/read-check Database model_id)
:table (api/read-check Database (db/select-one-field :db_id Table, :id model_id))))
- (->> (cards-for-filter-option f model_id)
- ;; filterv because we want make sure all the filtering is done while current user perms set is still bound
- (filterv mi/can-read?))))
+ (let [cards (filter mi/can-read? (cards-for-filter-option f model_id))
+ last-edit-info (:card (last-edit/fetch-last-edited-info {:card-ids (map :id cards)}))]
+ (into []
+ (map (fn [{:keys [id] :as card}]
+ (if-let [edit-info (get last-edit-info id)]
+ (assoc card :last-edit-info edit-info)
+ card)))
+ cards))))
(api/defendpoint GET "/:id"
"Get `Card` with ID."
[id]
(u/prog1 (-> (Card id)
(hydrate :creator :dashboard_count :can_write :collection)
- api/read-check)
+ api/read-check
+ (last-edit/with-last-edit-info :card))
(events/publish-event! :card-read (assoc <> :actor_id api/*current-user-id*))))
;;; -------------------------------------------------- Saving Cards --------------------------------------------------
@@ -211,7 +218,7 @@
(defn- save-new-card-async!
"Save `card-data` as a new Card on a separate thread. Returns a channel to fetch the response; closing this channel
will cancel the save."
- [card-data]
+ [card-data user]
(async.u/cancelable-thread
(let [card (db/transaction
;; Adding a new card at `collection_position` could cause other cards in this
@@ -221,7 +228,9 @@
(events/publish-event! :card-create card)
;; include same information returned by GET /api/card/:id since frontend replaces the Card it
;; currently has with returned one -- See #4283
- (hydrate card :creator :dashboard_count :can_write :collection))))
+ (-> card
+ (hydrate :creator :dashboard_count :can_write :collection)
+ (assoc :last-edit-info (last-edit/edit-information-for-user user))))))
(defn- create-card-async!
"Create a new Card asynchronously. Returns a channel for fetching the newly created Card, or an Exception if one was
@@ -241,7 +250,7 @@
(a/close! result-metadata-chan)
;; now do the actual saving on a separate thread so we don't tie up our precious core.async thread. Pipe the
;; result into `out-chan`.
- (async.u/promise-pipe (save-new-card-async! card-data) out-chan))
+ (async.u/promise-pipe (save-new-card-async! card-data @api/*current-user*) out-chan))
(catch Throwable e
(a/put! out-chan e)
(a/close! out-chan))))
@@ -424,7 +433,9 @@
(publish-card-update! card archived)
;; include same information returned by GET /api/card/:id since frontend replaces the Card it currently
;; has with returned one -- See #4142
- (hydrate card :creator :dashboard_count :can_write :collection))))
+ (-> card
+ (hydrate :creator :dashboard_count :can_write :collection)
+ (assoc :last-edit-info (last-edit/edit-information-for-user @api/*current-user*))))))
(api/defendpoint ^:returns-chan PUT "/:id"
"Update a `Card`."
diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj
index 2a4b940f3b2..b846c92cf45 100644
--- a/src/metabase/api/collection.clj
+++ b/src/metabase/api/collection.clj
@@ -18,6 +18,7 @@
[metabase.models.permissions :as perms]
[metabase.models.pulse :as pulse :refer [Pulse]]
[metabase.models.pulse-card :refer [PulseCard]]
+ [metabase.models.revision.last-edit :as last-edit]
[metabase.util :as u]
[metabase.util.schema :as su]
[schema.core :as s]
@@ -149,17 +150,30 @@
"Fetch a sequence of 'child' objects belonging to a Collection, filtered using `options`."
[{collection-namespace :namespace, :as collection} :- collection/CollectionWithLocationAndIDOrRoot
{:keys [model collections-only?], :as options} :- CollectionChildrenOptions]
- (->> (for [model-kw [:collection :card :dashboard :pulse :snippet]
- ;; only fetch models that are specified by the `model` param; or everything if it's `nil`
- :when (or (not model) (= model model-kw))
- :let [toucan-model (model-name->toucan-model model-kw)
- allowed-namespaces (collection/allowed-namespaces toucan-model)]
- :when (or (= model-kw :collection)
- (contains? allowed-namespaces (keyword collection-namespace)))
- item (fetch-collection-children model-kw collection (assoc options :collection-namespace collection-namespace))]
- (assoc item :model model-kw))
- ;; sorting by name should be fine for now.
- (sort-by (comp str/lower-case :name))))
+ (let [item-groups (into {}
+ (for [model-kw [:collection :card :dashboard :pulse :snippet]
+ ;; only fetch models that are specified by the `model` param; or everything if it's `nil`
+ :when (or (not model) (= model model-kw))
+ :let [toucan-model (model-name->toucan-model model-kw)
+ allowed-namespaces (collection/allowed-namespaces toucan-model)]
+ :when (or (= model-kw :collection)
+ (contains? allowed-namespaces (keyword collection-namespace)))]
+ [model-kw (fetch-collection-children model-kw collection (assoc options :collection-namespace collection-namespace))]))
+ last-edited (last-edit/fetch-last-edited-info
+ {:card-ids (->> item-groups :card (map :id))
+ :dashboard-ids (->> item-groups :dashboard (map :id))})]
+ (sort-by (comp str/lower-case :name) ;; sorting by name should be fine for now.
+ (into []
+ ;; items are grouped by model, needed for last-edit lookup. put model on each one, cat them, then
+ ;; plop edit information on them if present
+ (comp (map (fn [[model items]]
+ (map #(assoc % :model model) items)))
+ cat
+ (map (fn [{:keys [model id] :as item}]
+ (if-let [edit-info (get-in last-edited [model id])]
+ (assoc item :last-edit-info edit-info)
+ item))))
+ item-groups))))
(s/defn ^:private collection-detail
"Add a standard set of details to `collection`, including things like `effective_location`.
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index 3460b98ebe3..d70ef16ce40 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -18,6 +18,7 @@
[metabase.models.params.chain-filter :as chain-filter]
[metabase.models.query :as query :refer [Query]]
[metabase.models.revision :as revision]
+ [metabase.models.revision.last-edit :as last-edit]
[metabase.query-processor.error-type :as qp.error-type]
[metabase.query-processor.middleware.constraints :as constraints]
[metabase.query-processor.util :as qp-util]
@@ -59,7 +60,14 @@
* `archived` - Return Dashboards that have been archived. (By default, these are *excluded*.)"
[f]
{f (s/maybe (s/enum "all" "mine" "archived"))}
- (dashboards-list f))
+ (let [dashboards (dashboards-list f)
+ edit-infos (:dashboard (last-edit/fetch-last-edited-info {:dashboard-ids (map :id dashboards)}))]
+ (into []
+ (map (fn [{:keys [id] :as dashboard}]
+ (if-let [edit-info (get edit-infos id)]
+ (assoc dashboard :last-edit-info edit-info)
+ dashboard)))
+ dashboards)))
(api/defendpoint POST "/"
@@ -78,14 +86,15 @@
:creator_id api/*current-user-id*
:collection_id collection_id
:collection_position collection_position}]
- (db/transaction
- ;; Adding a new dashboard at `collection_position` could cause other dashboards in this collection to change
- ;; position, check that and fix up if needed
- (api/maybe-reconcile-collection-position! dashboard-data)
- ;; Ok, now save the Dashboard
- (->> (db/insert! Dashboard dashboard-data)
- ;; publish an event and return the newly created Dashboard
- (events/publish-event! :dashboard-create)))))
+ (let [dash (db/transaction
+ ;; Adding a new dashboard at `collection_position` could cause other dashboards in this collection to change
+ ;; position, check that and fix up if needed
+ (api/maybe-reconcile-collection-position! dashboard-data)
+ ;; Ok, now save the Dashboard
+ (db/insert! Dashboard dashboard-data))]
+ ;; publish event after the txn so that lookup can succeed
+ (events/publish-event! :dashboard-create dash)
+ (assoc dash :last-edit-info (last-edit/edit-information-for-user @api/*current-user*)))))
;;; -------------------------------------------- Hiding Unreadable Cards ---------------------------------------------
@@ -249,8 +258,9 @@
(api/defendpoint GET "/:id"
"Get Dashboard with ID."
[id]
- (u/prog1 (get-dashboard id)
- (events/publish-event! :dashboard-read (assoc <> :actor_id api/*current-user-id*))))
+ (let [dashboard (get-dashboard id)]
+ (events/publish-event! :dashboard-read (assoc dashboard :actor_id api/*current-user-id*))
+ (last-edit/with-last-edit-info dashboard :dashboard)))
(defn- check-allowed-to-change-embedding
@@ -302,8 +312,9 @@
:non-nil #{:name :parameters :caveats :points_of_interest :show_in_getting_started :enable_embedding
:embedding_params :archived})))))
;; now publish an event and return the updated Dashboard
- (u/prog1 (Dashboard id)
- (events/publish-event! :dashboard-update (assoc <> :actor_id api/*current-user-id*))))
+ (let [dashboard (Dashboard id)]
+ (events/publish-event! :dashboard-update (assoc dashboard :actor_id api/*current-user-id*))
+ (assoc dashboard :last-edit-info (last-edit/edit-information-for-user @api/*current-user*))))
;; TODO - We can probably remove this in the near future since it should no longer be needed now that we're going to
;; be setting `:archived` to `true` via the `PUT` endpoint instead
diff --git a/src/metabase/models/revision/last_edit.clj b/src/metabase/models/revision/last_edit.clj
new file mode 100644
index 00000000000..b283976624a
--- /dev/null
+++ b/src/metabase/models/revision/last_edit.clj
@@ -0,0 +1,95 @@
+(ns metabase.models.revision.last-edit
+ "A namespace to handle getting the last edited information about items that satisfy the revision system. The revision
+ system is a 'reversion' system, built to easily revert to previous states and can compute on demand a changelog. The
+ revision system works through events and so when editing something, you should construct the last-edit-info
+ yourself (using `edit-information-for-user`) rather looking at the revision table which might not be updated yet.
+
+ This constructs `:last-edit-info`, a map with keys `:timestamp`, `:id`, `:first_name`, `:last_name`, and
+ `:email`. It is not a full User object (missing some superuser metadata, last login time, and a common name). This
+ was done to prevent another db call and hooking up timestamps to users but this can be added if preferred."
+ (:require [clj-time.core :as time]
+ [clojure.set :as set]
+ [honeysql.core :as hsql]
+ [medley.core :as m]
+ [metabase.util.schema :as su]
+ [schema.core :as s]
+ [toucan.db :as db]))
+
+(def ^:private model->db-model {:card "Card" :dashboard "Dashboard"})
+
+;; these are all maybes as sometimes revisions don't exist, or users might be missing the names, etc
+(def LastEditInfo
+ "Schema of the `:last-edit-info` map. A subset of a user with a timestamp indicating when the last edit was."
+ {:timestamp (s/maybe s/Any)
+ :id (s/maybe su/IntGreaterThanZero)
+ :first_name (s/maybe s/Str)
+ :last_name (s/maybe s/Str)
+ :email (s/maybe s/Str)})
+
+(def MaybeAnnotated
+ "Spec for an item annotated with last-edit-info. Items are cards or dashboards. Optional because we may not always
+ have revision history for all cards/dashboards."
+ {(s/optional-key :last-edit-info) LastEditInfo s/Any s/Any})
+
+(s/defn with-last-edit-info :- MaybeAnnotated
+ "Add the last edited information to a card. Will add a key `:last-edit-info`. Model should be one of `:dashboard` or
+ `:card`. Gets the last edited information from the revisions table. If you need this information from a put route,
+ use `@api/*current-user*` and a current timestamp since revisions are events and asynchronous."
+ [{:keys [id] :as item} model :- (s/enum :dashboard :card)]
+ (if-let [[updated-info] (seq (db/query {:select [:u.id :u.email :u.first_name :u.last_name :r.timestamp]
+ :from [[:revision :r]]
+ :left-join [[:core_user :u] [:= :u.id :r.user_id]]
+ :where [:and
+ [:= :r.model (model->db-model model)]
+ [:= :r.model_id id]]
+ :order-by [[:u.id :desc]]
+ :limit 1}))]
+ (assoc item :last-edit-info updated-info)
+ item))
+
+(s/defn edit-information-for-user :- LastEditInfo
+ "Construct the `:last-edit-info` map given a user. Useful for editing routes. Most edit info information comes from
+ the revisions table. But this table is populated from events asynchronously so when editing and wanting
+ last-edit-info, you must construct it from `@api/*current-user*` and the current timestamp rather than checking the
+ revisions table as those revisions may not be present yet."
+ [user]
+ (merge {:timestamp (time/now)}
+ (select-keys user [:id :first_name :last_name :email])))
+
+(def CollectionLastEditInfo
+ "Schema for the map of bulk last-item-info. A map of two keys, `:card` and `:dashboard`, each of which is a map from
+ id to a LastEditInfo."
+ {(s/optional-key :card) {s/Int LastEditInfo}
+ (s/optional-key :dashboard) {s/Int LastEditInfo}})
+
+(s/defn fetch-last-edited-info :- (s/maybe CollectionLastEditInfo)
+ "Fetch edited info from the revisions table. Revision information is timestamp, user id, email, first and last
+ name. Takes card-ids and dashboard-ids and returns a map structured like
+
+ {:card {model_id {:id :email :first_name :last_name :timestamp}}
+ :dashboard {model_id {:id :email :first_name :last_name :timestamp}}}"
+ [{:keys [card-ids dashboard-ids]}]
+ (when (seq (concat card-ids dashboard-ids))
+ ;; [:in :model_id []] generates bad sql so need to conditionally add it
+ (let [where-clause (into [:or]
+ (keep (fn [[model-name ids]]
+ (when (seq ids)
+ [:and [:= :model model-name] [:in :model_id ids]])))
+ [["Card" card-ids]
+ ["Dashboard" dashboard-ids]])
+ latest-changes (db/query {:select [:u.id :u.email :u.first_name :u.last_name
+ :r.model :r.model_id :r.timestamp]
+ :from [[:revision :r]]
+ :left-join [[:core_user :u] [:= :u.id :r.user_id]]
+ :where [:in :r.id
+ ;; subselect for the max revision id for each item
+ {:select [[(hsql/call :max :id) :latest-revision-id]]
+ :from [:revision]
+ :where where-clause
+ :group-by [:model :model_id]}]})]
+ (->> latest-changes
+ (group-by :model)
+ (m/map-vals (fn [model-changes]
+ (into {} (map (juxt :model_id #(dissoc % :model :model_id))) model-changes)))
+ ;; keys are "Card" and "Dashboard" (model in revision table) back to keywords
+ (m/map-keys (set/map-invert model->db-model))))))
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index ae1d8e80382..b633e52522d 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -14,6 +14,8 @@
PulseChannelRecipient Table ViewLog]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
+ [metabase.models.revision :as revision :refer [Revision]]
+ [metabase.models.user :refer [User]]
[metabase.query-processor :as qp]
[metabase.query-processor.async :as qp.async]
[metabase.query-processor.middleware.constraints :as constraints]
@@ -293,6 +295,8 @@
:can_write true
:dashboard_count 0
:result_metadata true
+ :last-edit-info {:timestamp true :id true :first_name "Rasta"
+ :last_name "Toucan" :email "rasta@metabase.com"}
:creator (merge
(select-keys (mt/fetch-user :rasta) [:id :date_joined :last_login :locale])
{:common_name "Rasta Toucan"
@@ -308,7 +312,11 @@
(update :dataset_query map?)
(update :collection map?)
(update :result_metadata (partial every? map?))
- (update :creator dissoc :is_qbnewb)))))))))))
+ (update :creator dissoc :is_qbnewb)
+ (update :last-edit-info (fn [edit-info]
+ (-> edit-info
+ (update :id boolean)
+ (update :timestamp boolean))))))))))))))
(deftest save-empty-card-test
(testing "POST /api/card"
@@ -584,7 +592,17 @@
:collection_id (u/the-id collection)
:collection (into {} collection)
:result_metadata (mt/obj->json->obj (:result_metadata card))})
- (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card))))))))))
+ (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card))))))
+ (testing "Card should include last edit info if available"
+ (mt/with-temp* [User [{user-id :id} {:first_name "Test" :last_name "User" :email "user@test.com"}]
+ Revision [_ {:model "Card"
+ :model_id (:id card)
+ :user_id user-id
+ :object (revision/serialize-instance card (:id card) card)}]]
+ (is (= {:id true :email "user@test.com" :first_name "Test" :last_name "User" :timestamp true}
+ (-> (mt/user-http-request :rasta :get 200 (str "card/" (u/the-id card)))
+ mt/boolean-ids-and-timestamps
+ :last-edit-info)))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | UPDATING A CARD |
@@ -600,7 +618,10 @@
(with-cards-in-writeable-collection card
(is (= "Original Name"
(db/select-one-field :name Card, :id (u/the-id card))))
- (mt/user-http-request :rasta :put 202 (str "card/" (u/the-id card)) {:name "Updated Name"})
+ (is (= {:timestamp true, :first_name "Rasta", :last_name "Toucan", :email "rasta@metabase.com", :id true}
+ (-> (mt/user-http-request :rasta :put 202 (str "card/" (u/the-id card)) {:name "Updated Name"})
+ mt/boolean-ids-and-timestamps
+ :last-edit-info)))
(is (= "Updated Name"
(db/select-one-field :name Card, :id (u/the-id card)))))))
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index 1ee886944d3..5d022de1d56 100644
--- a/test/metabase/api/collection_test.clj
+++ b/test/metabase/api/collection_test.clj
@@ -3,13 +3,15 @@
(:require [clojure.string :as str]
[clojure.test :refer :all]
[metabase.models :refer [Card Collection Dashboard DashboardCard NativeQuerySnippet PermissionsGroup
- PermissionsGroupMembership Pulse PulseCard PulseChannel PulseChannelRecipient]]
+ PermissionsGroupMembership Pulse PulseCard PulseChannel PulseChannelRecipient
+ Revision User]]
[metabase.models.collection :as collection]
[metabase.models.collection-test :as collection-test]
[metabase.models.collection.graph :as graph]
[metabase.models.collection.graph-test :as graph.test]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as group]
+ [metabase.models.revision :as revision]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
@@ -333,7 +335,27 @@
(db/update-where! Dashboard {:collection_id (u/the-id collection)} :archived true)
(is (= [(default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"})]
(mt/boolean-ids-and-timestamps
- (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?archived=true"))))))))))
+ (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?archived=true"))))))))
+ (testing "Results include last edited information from the `Revision` table"
+ (mt/with-temp* [Collection [{collection-id :id} {:name "Collection with Items"}]
+ User [{user-id :id} {:first_name "Test" :last_name "User" :email "testuser@example.com"}]
+ Card [{card-id :id :as card}
+ {:name "Card with history" :collection_id collection-id}]
+ Revision [_revision {:model "Card"
+ :model_id card-id
+ :user_id user-id
+ :object (revision/serialize-instance card card-id card)}]]
+ (is (= [{:name "Card with history",
+ :last-edit-info
+ {:id true,
+ :email "testuser@example.com",
+ :first_name "Test",
+ :last_name "User",
+ ;; timestamp collapsed to true, ordinarily a OffsetDateTime
+ :timestamp true}}]
+ (->> (mt/user-http-request :rasta :get 200 (str "collection/" collection-id "/items"))
+ mt/boolean-ids-and-timestamps
+ (map #(select-keys % [:name :last-edit-info])))))))))
(deftest snippet-collection-items-test
(testing "GET /api/collection/:id/items"
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 2c495d4953a..6fc3a9201bf 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -18,8 +18,9 @@
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as group]
[metabase.models.pulse :refer [Pulse]]
- [metabase.models.revision :refer [Revision]]
+ [metabase.models.revision :as revision :refer [Revision]]
[metabase.models.table :refer [Table]]
+ [metabase.models.user :refer [User]]
[metabase.server.middleware.util :as middleware.u]
[metabase.test :as mt]
[metabase.util :as u]
@@ -60,12 +61,18 @@
(update :collection_id boolean)))))
(defn- dashboard-response [{:keys [creator ordered_cards created_at updated_at] :as dashboard}]
+ ;; todo: should be udpated to use mt/boolean-ids-and-timestamps
(let [dash (-> (into {} dashboard)
(dissoc :id)
(assoc :created_at (boolean created_at)
:updated_at (boolean updated_at))
(update :collection_id boolean))]
(cond-> dash
+ (contains? dash :last-edit-info)
+ (update :last-edit-info (fn [info]
+ (-> info
+ (update :id boolean)
+ (update :timestamp boolean))))
creator (update :creator #(into {} %))
ordered_cards (update :ordered_cards #(mapv dashcard-response %)))))
@@ -136,12 +143,14 @@
(try
(is (= (merge
dashboard-defaults
- {:name test-dashboard-name
- :creator_id (mt/user->id :rasta)
- :parameters [{:id "abc123", :name "test", :type "date"}]
- :updated_at true
- :created_at true
- :collection_id true})
+ {:name test-dashboard-name
+ :creator_id (mt/user->id :rasta)
+ :parameters [{:id "abc123", :name "test", :type "date"}]
+ :updated_at true
+ :created_at true
+ :collection_id true
+ :last-edit-info {:timestamp true :id true :first_name "Rasta"
+ :last_name "Toucan" :email "rasta@metabase.com"}})
(-> (mt/user-http-request :rasta :post 200 "dashboard" {:name test-dashboard-name
:parameters [{:id "abc123", :name "test", :type "date"}]
:collection_id (u/get-id collection)})
@@ -184,9 +193,18 @@
(deftest fetch-dashboard-test
(testing "GET /api/dashboard/:id"
(testing "fetch a dashboard WITH a dashboard card on it"
- (mt/with-temp* [Dashboard [{dashboard-id :id} {:name "Test Dashboard"}]
+ (mt/with-temp* [Dashboard [{dashboard-id :id
+ :as dashboard} {:name "Test Dashboard"}]
Card [{card-id :id} {:name "Dashboard Test Card"}]
- DashboardCard [_ {:dashboard_id dashboard-id, :card_id card-id}]]
+ DashboardCard [_ {:dashboard_id dashboard-id, :card_id card-id}]
+ User [{user-id :id} {:first_name "Test" :last_name "User"
+ :email "test@example.com"}]
+ Revision [_ {:user_id user-id
+ :model "Dashboard"
+ :model_id dashboard-id
+ :object (revision/serialize-instance dashboard
+ dashboard-id
+ dashboard)}]]
(with-dashboards-in-readable-collection [dashboard-id]
(card-api-test/with-cards-in-readable-collection [card-id]
(is (= (merge
@@ -197,6 +215,7 @@
:can_write false
:param_values nil
:param_fields nil
+ :last-edit-info {:timestamp true :id true :first_name "Test" :last_name "User" :email "test@example.com"}
:ordered_cards [{:sizeX 2
:sizeY 2
:col 0
@@ -296,9 +315,11 @@
(dashboard-response (Dashboard dashboard-id)))))
(testing "PUT response"
- (is (= (merge dashboard-defaults {:name "My Cool Dashboard"
- :description "Some awesome description"
- :creator_id (mt/user->id :rasta)
+ (is (= (merge dashboard-defaults {:name "My Cool Dashboard"
+ :description "Some awesome description"
+ :creator_id (mt/user->id :rasta)
+ :last-edit-info {:timestamp true :id true :first_name "Rasta"
+ :last_name "Toucan" :email "rasta@metabase.com"}
:collection_id true})
(dashboard-response
(mt/user-http-request :rasta :put 200 (str "dashboard/" dashboard-id)
@@ -324,6 +345,9 @@
:collection_id true
:caveats ""
:points_of_interest ""
+ :last-edit-info
+ {:timestamp true, :id true, :first_name "Rasta",
+ :last_name "Toucan", :email "rasta@metabase.com"}
:show_in_getting_started true})
(dashboard-response (mt/user-http-request :rasta :put 200 (str "dashboard/" dashboard-id)
{:caveats ""
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index d585d124f71..6fff0264cd7 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -89,7 +89,7 @@
(boolean-ids-and-timestamps
(every-pred (some-fn keyword? string?)
(some-fn #{:id :created_at :updated_at :last_analyzed :created-at :updated-at :field-value-id :field-id
- :date_joined :date-joined :last_login :dimension-id :human-readable-field-id}
+ :date_joined :date-joined :last_login :dimension-id :human-readable-field-id :timestamp}
#(str/ends-with? % "_id")
#(str/ends-with? % "_at")))
data))
--
GitLab