Skip to content
Snippets Groups Projects
Unverified Commit 977841b0 authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Ignore hidden/archived tables cards and dashboards in recent list (#20238) 

*** Code:
- ignore hidden tables and archived cards/dashboards
- correct docstring to indicate we are only getting 5 not 10 as
previously claimed
- in query, set limit to 8 in the case we get archived or non-existing
things and then take 5 in memory
- select extra properties from models that we need for this
determination

*** Tests:
- for mysql we were inserting views with a thread/sleep of one second
between each. Just compute timestamps seconds apart and do them all at
once. Should save 7 seconds off mysql test runs. verified temporal
spacing works with

```
 MB_DB_CONNECTION_URI="mysql://username:password@localhost:3306/cli" \
 clj -X:dev:test :only metabase.api.activity-test
```
- cleanup these activities we were creating with `mt/with-model-cleanup`
which introduces a big looking diff. But checking diff with whitespace
shows its just an indentation and then add the extra keys needed for
identifying which are archived (:archived and :visibility_type for cards
and tables respectively)
parent f2ce41a0
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/activity.clj
+ 17
− 10
View file @ 977841b0
......@@ -94,9 +94,10 @@
(db/select
(case model
"card" [Card :id :name :collection_id :description :display
:dataset_query :dataset]
"dashboard" [Dashboard :id :name :collection_id :description]
"table" [Table :id :name :db_id :display_name :initial_sync_status])
:dataset_query :dataset :archived]
"dashboard" [Dashboard :id :name :collection_id :description :archived]
"table" [Table :id :name :db_id :display_name :initial_sync_status
:visibility_type])
{:where [:in :id ids]})))
(by-id [models] (m/index-by :id models))]
(into {} (map (fn [[model models]]
......@@ -107,7 +108,8 @@
(group-by :model views))))
(defendpoint GET "/recent_views"
"Get the list of 10 things the current user has been viewing most recently."
"Get the list of 5 things the current user has been viewing most recently. Query takes 8 and limits to 5 so that if it
finds anything archived, deleted, etc it can hopefully still get 5."
[]
;; expected output of the query is a single row per unique model viewed by the current user
;; including a `:max_ts` which has the most recent view timestamp of the item and `:cnt` which has total views
......@@ -119,12 +121,17 @@
[:= :user_id *current-user-id*]
[:in :model #{"card" "dashboard" "table"}]]
:order-by [[:max_ts :desc]]
:limit 5})
:limit 8})
model->id->items (models-for-views views)]
(for [{:keys [model model_id] :as view-log} views
:let [model-object (get-in model->id->items [model model_id])]
:when (and model-object (mi/can-read? model-object))]
(cond-> (assoc view-log :model_object (dissoc model-object :dataset_query))
(:dataset model-object) (assoc :model "dataset")))))
(->> (for [{:keys [model model_id] :as view-log} views
:let [model-object (get-in model->id->items [model model_id])]
:when (and model-object
(mi/can-read? model-object)
;; hidden tables, archived cards/dashboards
(not (or (:archived model-object)
(= (:visibility_type model-object) :hidden))))]
(cond-> (assoc view-log :model_object (dissoc model-object :dataset_query))
(:dataset model-object) (assoc :model "dataset")))
(take 5))))
(define-routes)
test/metabase/api/activity_test.clj
+ 70
− 58
View file @ 977841b0
(ns metabase.api.activity-test
"Tests for /api/activity endpoints."
(:require [clojure.test :refer :all]
[java-time :as t]
[metabase.api.activity :as activity-api]
[metabase.db :as mdb]
[metabase.models.activity :refer [Activity]]
[metabase.models.card :refer [Card]]
[metabase.models.dashboard :refer [Dashboard]]
......@@ -86,78 +86,90 @@
;; 3. `:model_object` is hydrated in each result
;; 4. we filter out entries where `:model_object` is nil (object doesn't exist)
(defn- create-view! [user model model-id]
(db/insert! ViewLog
:user_id user
:model model
:model_id model-id
:timestamp :%now)
;; we sleep a bit to ensure no events have the same timestamp
;; sadly, MySQL doesn't support milliseconds so we have to wait a second
;; otherwise our records are out of order and this test fails :(
(Thread/sleep (if (= (mdb/db-type) :mysql)
1000
10)))
(defn- create-views!
"Insert views [user-id model model-id]. Reviews are entered a second apart with last review as most recent."
[views]
(let [views (map (fn [[user model model-id] hours-ago]
{:user_id user, :model model, :model_id model-id
:timestamp (t/plus (t/local-date-time) (t/seconds (- hours-ago)))})
(reverse views)
(range))]
(db/insert-many! ViewLog views)))
(deftest recent-views-test
(mt/with-temp* [Card [card1 {:name "rand-name"
:creator_id (mt/user->id :crowberto)
:display "table"
:visualization_settings {}}]
Card [archived {:name "archived-card"
:creator_id (mt/user->id :crowberto)
:display "table"
:archived true
:visualization_settings {}}]
Dashboard [dash1 {:name "rand-name"
:description "rand-name"
:creator_id (mt/user->id :crowberto)}]
Table [table1 {:name "rand-name"}]
Table [hidden-table {:name "hidden table"
:visibility_type "hidden"}]
Card [dataset {:name "rand-name"
:dataset true
:creator_id (mt/user->id :crowberto)
:display "table"
:visualization_settings {}}]]
(create-view! (mt/user->id :crowberto) "card" (:id dataset))
(create-view! (mt/user->id :crowberto) "dashboard" (:id dash1))
(create-view! (mt/user->id :crowberto) "card" (:id card1))
(create-view! (mt/user->id :crowberto) "card" 36478)
(create-view! (mt/user->id :crowberto) "table" (:id table1))
(create-view! (mt/user->id :rasta) "card" (:id card1))
(is (= [{:cnt 1,
:model "table",
:model_id (:id table1),
:model_object {:db_id (:db_id table1),
:id (:id table1),
:name (:name table1)
:display_name (:display_name table1)
:initial_sync_status "incomplete"},
:user_id (mt/user->id :crowberto)}
{:cnt 1
:user_id (mt/user->id :crowberto)
:model "card"
:model_id (:id card1)
:model_object {:id (:id card1)
:name (:name card1)
:collection_id nil
:dataset false
:description (:description card1)
:display (name (:display card1))}}
{:cnt 1
:user_id (mt/user->id :crowberto)
:model "dashboard"
:model_id (:id dash1)
:model_object {:id (:id dash1)
:name (:name dash1)
:collection_id nil
:description (:description dash1)}}
{:cnt 1
:user_id (mt/user->id :crowberto)
:model "dataset"
:model_id (:id dataset)
:model_object {:id (:id dataset)
:name (:name dataset)
:dataset true
:collection_id nil
:description (:description dataset)
:display (name (:display dataset))}}]
(for [recent-view (mt/user-http-request :crowberto :get 200 "activity/recent_views")]
(dissoc recent-view :max_ts))))))
(mt/with-model-cleanup [ViewLog]
(create-views! [[(mt/user->id :crowberto) "card" (:id dataset)]
[(mt/user->id :crowberto) "dashboard" (:id dash1)]
[(mt/user->id :crowberto) "card" (:id card1)]
[(mt/user->id :crowberto) "card" 36478]
[(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 (= [{:cnt 1,
:model "table",
:model_id (:id table1),
:model_object {:db_id (:db_id table1),
:id (:id table1),
:visibility_type nil
:name (:name table1)
:display_name (:display_name table1)
:initial_sync_status "incomplete"},
:user_id (mt/user->id :crowberto)}
{:cnt 1
:user_id (mt/user->id :crowberto)
:model "card"
:model_id (:id card1)
:model_object {:id (:id card1)
:name (:name card1)
:archived false
:collection_id nil
:dataset false
:description (:description card1)
:display (name (:display card1))}}
{:cnt 1
:user_id (mt/user->id :crowberto)
:model "dashboard"
:model_id (:id dash1)
:model_object {:id (:id dash1)
:name (:name dash1)
:archived false
:collection_id nil
:description (:description dash1)}}
{:cnt 1
:user_id (mt/user->id :crowberto)
:model "dataset"
:model_id (:id dataset)
:model_object {:id (:id dataset)
:name (:name dataset)
:archived false
:dataset true
:collection_id nil
:description (:description dataset)
:display (name (:display dataset))}}]
(for [recent-view (mt/user-http-request :crowberto :get 200 "activity/recent_views")]
(dissoc recent-view :max_ts)))))))
;;; activities->referenced-objects, referenced-objects->existing-objects, add-model-exists-info
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment