From 2627bdf60570e3fcb06fd6589cebac232be33664 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Thu, 1 Sep 2022 00:03:57 +0300
Subject: [PATCH] Hydrate app ID on app collection bookmarks (#25024)
---
src/metabase/models/bookmark.clj | 10 +++++++---
test/metabase/api/bookmark_test.clj | 16 ++++++++++------
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/src/metabase/models/bookmark.clj b/src/metabase/models/bookmark.clj
index 599abdc04e8..c31b405d5ba 100644
--- a/src/metabase/models/bookmark.clj
+++ b/src/metabase/models/bookmark.clj
@@ -1,6 +1,7 @@
(ns metabase.models.bookmark
(:require [clojure.string :as str]
[metabase.db.connection :as mdb.connection]
+ [metabase.models.app :refer [App]]
[metabase.models.card :refer [Card]]
[metabase.models.collection :refer [Collection]]
[metabase.models.dashboard :refer [Dashboard]]
@@ -30,7 +31,8 @@
(s/optional-key :dataset) (s/maybe s/Bool)
(s/optional-key :display) (s/maybe s/Str)
(s/optional-key :authority_level) (s/maybe s/Str)
- (s/optional-key :description) (s/maybe s/Str)})
+ (s/optional-key :description) (s/maybe s/Str)
+ (s/optional-key :app_id) (s/maybe su/IntGreaterThanOrEqualToZero)})
(s/defn ^:private normalize-bookmark-result :- BookmarkResult
"Normalizes bookmark results. Bookmarks are left joined against the card, collection, and dashboard tables, but only
@@ -41,7 +43,7 @@
normalized-result (zipmap (map unqualify-key (keys result)) (vals result))
id-str (str (:type normalized-result) "-" (:item_id normalized-result))]
(-> normalized-result
- (select-keys [:item_id :type :name :dataset :description :display :authority_level])
+ (select-keys [:item_id :type :name :dataset :description :display :authority_level :app_id])
(assoc :id id-str))))
(defn- bookmarks-union-query
@@ -91,11 +93,13 @@
[:collection.name (db/qualify 'Collection :name)]
[:collection.authority_level (db/qualify 'Collection :authority_level)]
[:collection.description (db/qualify 'Collection :description)]
- [:collection.archived (db/qualify 'Collection :archived)]]
+ [:collection.archived (db/qualify 'Collection :archived)]
+ [:app.id (db/qualify 'Collection :app_id)]]
:from [[(bookmarks-union-query user-id) :bookmark]]
:left-join [[Card :card] [:= :bookmark.card_id :card.id]
[Dashboard :dashboard] [:= :bookmark.dashboard_id :dashboard.id]
[Collection :collection] [:= :bookmark.collection_id :collection.id]
+ [App :app] [:= :app.collection_id :collection.id]
[BookmarkOrdering :bookmark_ordering] [:and
[:= :bookmark_ordering.user_id user-id]
[:= :bookmark_ordering.type :bookmark.type]
diff --git a/test/metabase/api/bookmark_test.clj b/test/metabase/api/bookmark_test.clj
index 431442b5f16..850e5d78825 100644
--- a/test/metabase/api/bookmark_test.clj
+++ b/test/metabase/api/bookmark_test.clj
@@ -1,6 +1,7 @@
(ns metabase.api.bookmark-test
"Tests for /api/bookmark endpoints."
(:require [clojure.test :refer :all]
+ [metabase.models.app :refer [App]]
[metabase.models.bookmark :refer [BookmarkOrdering CardBookmark
CollectionBookmark
DashboardBookmark]]
@@ -16,7 +17,8 @@
(testing "POST /api/bookmark/:model/:model-id"
(mt/with-temp* [Collection [collection {:name "Test Collection"}]
Card [card {:name "Test Card" :display "area"}]
- Dashboard [dashboard {:name "Test Dashboard"}]]
+ Dashboard [dashboard {:name "Test Dashboard"}]
+ App [app {:collection_id (:id collection), :dashboard_id (:id dashboard)}]]
(testing "check that we can bookmark a Collection"
(is (= (u/the-id collection)
(->> (mt/user-http-request :rasta :post 200 (str "bookmark/collection/" (u/the-id collection)))
@@ -26,7 +28,7 @@
(->> (mt/user-http-request :rasta :post 200 (str "bookmark/card/" (u/the-id card)))
:card_id))))
(let [card-result (->> (mt/user-http-request :rasta :get 200 "bookmark")
- (filter #(= (:type % ) "card"))
+ (filter #(= (:type %) "card"))
first)]
(testing "check a card bookmark has `:display` key"
(is (contains? card-result :display)))
@@ -37,10 +39,12 @@
(->> (mt/user-http-request :rasta :post 200 (str "bookmark/dashboard/" (u/the-id dashboard)))
:dashboard_id))))
(testing "check that we can retreive the user's bookmarks"
- (is (= #{"card" "collection" "dashboard"}
- (->> (mt/user-http-request :rasta :get 200 "bookmark")
- (map :type)
- set))))
+ (let [result (mt/user-http-request :rasta :get 200 "bookmark")]
+ (is (= #{"card" "collection" "dashboard"}
+ (into #{} (map :type) result)))
+ (testing "that app_id is hydrated on app collections"
+ (is (partial= [{:app_id (:id app)}]
+ (filter #(= (:type %) "collection") result))))))
(testing "check that we can delete bookmarks"
(mt/user-http-request :rasta :delete 204 (str "bookmark/card/" (u/the-id card)))
(is (= #{"collection" "dashboard"}
--
GitLab