diff --git a/src/metabase/api/activity.clj b/src/metabase/api/activity.clj
index a6806f51a71cde0f611c90265fe338cd042cdb45..ea334dfb090a85eb13e494aeaac0c21e3cbf4d83 100644
--- a/src/metabase/api/activity.clj
+++ b/src/metabase/api/activity.clj
@@ -138,23 +138,23 @@
from the query_execution table. The query context is always a `:question`. The results are normalized and concatenated to the
query results for dashboard and table views."
[views-limit card-runs-limit all-users?]
- (let [dashboard-and-table-views (db/select [ViewLog :user_id :model :model_id
+ (let [dashboard-and-table-views (db/select [ViewLog :%min.view_log.user_id :model :model_id
[:%count.* :cnt] [:%max.timestamp :max_ts]]
- {:group-by [(db/qualify ViewLog :user_id) :model :model_id]
+ {:group-by [:model :model_id]
:where [:and
(when-not all-users? [:= (db/qualify ViewLog :user_id) *current-user-id*])
[:in :model #{"dashboard" "table"}]
[:= :bm.id nil]]
- :order-by [[:max_ts :desc]]
+ :order-by [[:model :desc] [:max_ts :desc]]
:limit views-limit
:left-join [[DashboardBookmark :bm]
[:and
[:not [:= :model "table"]]
[:= :bm.user_id *current-user-id*]
[:= :model_id :bm.dashboard_id]]]})
- card-runs (->> (db/select [QueryExecution [:executor_id :user_id] [(db/qualify QueryExecution :card_id) :model_id]
+ card-runs (->> (db/select [QueryExecution [:%min.executor_id :user_id] [(db/qualify QueryExecution :card_id) :model_id]
[:%count.* :cnt] [:%max.started_at :max_ts]]
- {:group-by [:executor_id (db/qualify QueryExecution :card_id) :context]
+ {:group-by [(db/qualify QueryExecution :card_id) :context]
:where [:and
(when-not all-users? [:= :executor_id *current-user-id*])
[:= :context (hx/literal :question)]
@@ -232,6 +232,14 @@
(* (/ cnt max-count) views-wt)]]
(assoc item :score (double (reduce + scores))))))))
+(def ^:private model-precedence ["dashboard" "card" "dataset" "table"])
+
+(defn- order-items
+ [items]
+ (when (seq items)
+ (let [groups (group-by :model items)]
+ (mapcat #(get groups %) model-precedence))))
+
(defendpoint GET "/popular_items"
"Get the list of 5 popular things for the current user. Query takes 8 and limits to 5 so that if it
finds anything archived, deleted, etc it can hopefully still get 5."
@@ -256,6 +264,7 @@
(->> scored-views
(sort-by :score)
reverse
+ order-items
(take 5)
(map #(dissoc % :score)))))
diff --git a/test/metabase/api/activity_test.clj b/test/metabase/api/activity_test.clj
index 09081a6f0b7e467b327e5594b8748ea549f5c6a7..32ffe343b92a9a04d9c6508d23d53d38491b4c03 100644
--- a/test/metabase/api/activity_test.clj
+++ b/test/metabase/api/activity_test.clj
@@ -160,6 +160,9 @@
Dashboard [dash1 {:name "rand-name"
:description "rand-name"
:creator_id (mt/user->id :crowberto)}]
+ Dashboard [dash2 {:name "other-dashboard"
+ :description "just another dashboard"
+ :creator_id (mt/user->id :crowberto)}]
Table [table1 {:name "rand-name"}]
Table [hidden-table {:name "hidden table"
:visibility_type "hidden"}]
@@ -168,21 +171,47 @@
:creator_id (mt/user->id :crowberto)
:display "table"
:visualization_settings {}}]]
- (mt/with-model-cleanup [ViewLog QueryExecution]
- (create-views! (concat
- ;; one item with many views is considered more popular
- (repeat 10 [(mt/user->id :rasta) "card" (:id dataset)])
- [[(mt/user->id :rasta) "dashboard" (:id dash1)]
- [(mt/user->id :rasta) "card" (:id card1)]
- [(mt/user->id :rasta) "table" (:id table1)]
- [(mt/user->id :rasta) "card" (:id card1)]]))
- (is (= [["dataset" (:id dataset)]
- ["card" (:id card1)]
- ["table" (:id table1)]
- ["dashboard" (:id dash1)]]
- ;; 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)))))))
+ (testing "Items viewed by multiple users are not duplicated in the popular items list."
+ (mt/with-model-cleanup [ViewLog QueryExecution]
+ (create-views! [[(mt/user->id :rasta) "dashboard" (:id dash1)]
+ [(mt/user->id :crowberto) "dashboard" (:id dash1)]
+ [(mt/user->id :rasta) "card" (:id card1)]
+ [(mt/user->id :crowberto) "card" (:id card1)]])
+ (is (= [["dashboard" (:id dash1)]
+ ["card" (:id card1)]]
+ ;; 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))))))
+ (testing "Items viewed by other users can still show up in popular items."
+ (mt/with-model-cleanup [ViewLog QueryExecution]
+ (create-views! [[(mt/user->id :rasta) "dashboard" (:id dash1)]
+ [(mt/user->id :rasta) "card" (:id card1)]
+ [(mt/user->id :rasta) "table" (:id table1)]
+ [(mt/user->id :rasta) "card" (:id dataset)]])
+ (is (= [["dashboard" (:id dash1)]
+ ["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))))))
+ (testing "Items with more views show up sooner in popular items."
+ (mt/with-model-cleanup [ViewLog QueryExecution]
+ (create-views! (concat
+ ;; one item with many views is considered more popular
+ (repeat 10 [(mt/user->id :rasta) "dashboard" (:id dash1)])
+ [[(mt/user->id :rasta) "dashboard" (:id dash2)]
+ [(mt/user->id :rasta) "card" (:id dataset)]
+ [(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)]]
+ ;; 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))))))))
;;; activities->referenced-objects, referenced-objects->existing-objects, add-model-exists-info