Skip to content
Snippets Groups Projects
Unverified Commit 096bc8cf authored by metamben's avatar metamben Committed by GitHub
Browse files

Report page dashboard activities with model "page" (#25158) 

Fixes #25143 and #25157.

For recent_views and popular_items hydrate the is_app_page flag for dashboards.
parent 2e9c9395
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/activity.clj
+ 30
− 17
View file @ 096bc8cf
......@@ -52,28 +52,40 @@
(cond-> {}
(seq dataset-ids) (assoc "dataset" (set dataset-ids))
(seq card-ids') (assoc "card" (set card-ids')))))
(into {} (for [[model ids] (dissoc referenced-objects "card")
(when-let [dashboard-ids (get referenced-objects "dashboard")]
(let [id->page? (db/select-id->field :is_app_page Dashboard
:id [:in dashboard-ids])
{page-ids true dashboard-ids' false} (group-by (comp boolean id->page?)
;; only existing ids go back
(keys id->page?))]
(cond-> {}
(seq page-ids) (assoc "page" (set page-ids))
(seq dashboard-ids') (assoc "dashboard" (set dashboard-ids')))))
(into {} (for [[model ids] (dissoc referenced-objects "card" "dashboard")
:when (seq ids)]
{model (case model
"dashboard" (db/select-ids 'Dashboard, :id [:in ids])
[model (case model
"metric" (db/select-ids 'Metric, :id [:in ids], :archived false)
"pulse" (db/select-ids 'Pulse, :id [:in ids])
"segment" (db/select-ids 'Segment, :id [:in ids], :archived false)
nil)})))) ; don't care about other models
nil)])))) ; don't care about other models
(defn- add-model-exists-info
"Add `:model_exists` keys to `activities`, and `:exists` keys to nested dashcards where appropriate."
[activities]
(let [existing-objects (-> activities activities->referenced-objects referenced-objects->existing-objects)
existing-dataset? (fn [card-id]
(contains? (get existing-objects "dataset") card-id))]
model-exists? (fn [model id] (contains? (get existing-objects model) id))
existing-dataset? (partial model-exists? "dataset")
existing-page? (partial model-exists? "page")
existing-card? (partial model-exists? "card")]
(for [{:keys [model_id], :as activity} activities]
(let [model (if (and (= (:model activity) "card")
(existing-dataset? (:model_id activity)))
"dataset"
(:model activity))]
(let [model (cond
(and (= (:model activity) "card")
(existing-dataset? (:model_id activity))) "dataset"
(and (= (:model activity) "dashboard")
(existing-page? (:model_id activity))) "page"
:else (:model activity))]
(cond-> (assoc activity
:model_exists (contains? (get existing-objects model) model_id)
:model_exists (model-exists? model model_id)
:model model)
(dashcard-activity? activity)
(update-in [:details :dashcards]
......@@ -81,8 +93,7 @@
(for [dashcard dashcards]
(assoc dashcard :exists
(or (existing-dataset? (:card_id dashcard))
(contains? (get existing-objects "card")
(:card_id dashcard))))))))))))
(existing-card? (:card_id dashcard))))))))))))
(defendpoint GET "/"
"Get recent activity."
......@@ -101,7 +112,7 @@
(db/qualify Collection :authority_level)]
"dashboard" [Dashboard
:id :name :collection_id :description
:archived
:archived :is_app_page
(db/qualify Collection :authority_level)]
"table" [Table
:id :name :db_id
......@@ -193,7 +204,8 @@
(not (or (:archived model-object)
(= (:visibility_type model-object) :hidden))))]
(cond-> (assoc view-log :model_object model-object)
(:dataset model-object) (assoc :model "dataset")))
(:dataset model-object) (assoc :model "dataset")
(:is_app_page model-object) (assoc :model "page")))
(take 5))))
(defn- official?
......@@ -233,7 +245,7 @@
(* (/ cnt max-count) views-wt)]]
(assoc item :score (double (reduce + scores))))))))
(def ^:private model-precedence ["dashboard" "card" "dataset" "table"])
(def ^:private model-precedence ["dashboard" "page" "card" "dataset" "table"])
(defn- order-items
[items]
......@@ -260,7 +272,8 @@
(not (or (:archived model-object)
(= (:visibility_type model-object) :hidden))))]
(cond-> (assoc view-log :model_object model-object)
(:dataset model-object) (assoc :model "dataset")))
(:dataset model-object) (assoc :model "dataset")
(:is_app_page model-object) (assoc :model "page")))
scored-views (score-items filtered-views)]
(->> scored-views
(sort-by :score)
......
test/metabase/api/activity_test.clj
+ 38
− 17
View file @ 096bc8cf
......@@ -56,7 +56,15 @@
:user_id (mt/user->id :rasta)
:model "user"
:details {}
:timestamp #t "2015-09-10T05:33:43.641Z[UTC]"}]]
:timestamp #t "2015-09-10T05:33:43.641Z[UTC]"}]
Dashboard [page {:is_app_page true}]
Activity [activity4 {:topic "dashboard-create"
:user_id (mt/user->id :crowberto)
:model "dashboard"
:model_id (u/the-id page)
:details {:description "Because I can too!"
:name "Hehehe"}
:timestamp #t "2015-09-10T04:53:01.632Z[UTC]"}]]
(letfn [(fetch-activity [activity]
(merge
activity-defaults
......@@ -69,13 +77,19 @@
(fetch-activity activity3)
{:topic "user-joined"
:user (activity-user-info :rasta)})
(merge
(fetch-activity activity4)
{:topic "dashboard-create"
:user (activity-user-info :crowberto)
:model_exists true
:model "page"})
(merge
(fetch-activity activity1)
{:topic "install"
:user_id nil
:user nil})]
;; remove other activities from the API response just in case -- we're not interested in those
(let [these-activity-ids (set (map u/the-id [activity1 activity2 activity3]))]
(let [these-activity-ids (set (map u/the-id [activity1 activity2 activity3 activity4]))]
(for [activity (mt/user-http-request :crowberto :get 200 "activity")
:when (contains? these-activity-ids (u/the-id activity))]
(dissoc activity :timestamp)))))))))
......@@ -119,9 +133,13 @@
:display "table"
:archived true
:visualization_settings {}}]
Dashboard [dash1 {:name "rand-name"
:description "rand-name"
:creator_id (mt/user->id :crowberto)}]
Dashboard [page {:name "rand-name"
:description "rand-name"
:creator_id (mt/user->id :crowberto)
:is_app_page true}]
Dashboard [dash {:name "rand-name2"
:description "rand-name2"
:creator_id (mt/user->id :crowberto)}]
Table [table1 {:name "rand-name"}]
Table [hidden-table {:name "hidden table"
:visibility_type "hidden"}]
......@@ -132,20 +150,22 @@
:visualization_settings {}}]]
(mt/with-model-cleanup [ViewLog QueryExecution]
(create-views! [[(mt/user->id :crowberto) "card" (:id dataset)]
[(mt/user->id :crowberto) "dashboard" (:id dash1)]
[(mt/user->id :crowberto) "dashboard" (:id page)]
[(mt/user->id :crowberto) "card" (:id card1)]
[(mt/user->id :crowberto) "card" 36478]
[(mt/user->id :crowberto) "dashboard" (:id dash)]
[(mt/user->id :crowberto) "table" (:id table1)]
;; most recent for crowberto are archived card and hidden table
[(mt/user->id :crowberto) "card" (:id archived)]
[(mt/user->id :crowberto) "table" (:id hidden-table)]
[(mt/user->id :rasta) "card" (:id card1)]])
(is (= [["table" (:id table1)]
["card" (:id card1)]
["dashboard" (:id dash1)]
["dataset" (:id dataset)]]
(for [recent-view (mt/user-http-request :crowberto :get 200 "activity/recent_views")]
((juxt :model :model_id) recent-view)))))))
(let [recent-views (mt/user-http-request :crowberto :get 200 "activity/recent_views")]
(is (partial= [{:model "table" :model_id (:id table1)}
{:model "dashboard" :model_id (:id dash) :model_object {:is_app_page false}}
{:model "card" :model_id (:id card1)}
{:model "page" :model_id (:id page) :model_object {:is_app_page true}}
{:model "dataset" :model_id (:id dataset)}]
recent-views))))))
(deftest popular-items-test
(mt/with-temp* [Card [card1 {:name "rand-name"
......@@ -162,7 +182,8 @@
:creator_id (mt/user->id :crowberto)}]
Dashboard [dash2 {:name "other-dashboard"
:description "just another dashboard"
:creator_id (mt/user->id :crowberto)}]
:creator_id (mt/user->id :crowberto)
:is_app_page true}]
Table [table1 {:name "rand-name"}]
Table [_hidden-table {:name "hidden table"
:visibility_type "hidden"}]
......@@ -205,10 +226,10 @@
[(mt/user->id :rasta) "table" (:id table1)]
[(mt/user->id :rasta) "card" (:id card1)]]))
(is (= [["dashboard" (:id dash1)]
["dashboard" (:id dash2)]
["card" (:id card1)]
["dataset" (:id dataset)]
["table" (:id table1)]]
["page" (:id dash2)]
["card" (:id card1)]
["dataset" (:id dataset)]
["table" (:id table1)]]
;; all views are from :rasta, but :crowberto can still see popular items
(for [popular-item (mt/user-http-request :crowberto :get 200 "activity/popular_items")]
((juxt :model :model_id) popular-item))))))))
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment