Skip to content
Snippets Groups Projects
Commit 680293aa authored by Ryan Senior's avatar Ryan Senior
Browse files

Split `/collection/:id` and `/collection/:id/items` tests 

Recently the `/collection/:id` endpoint was updated to only return
collection metadata and separately (via the `/collection/:id/items`
endpoint) return the children of the collection. This updates the test
to follow that same split.
parent 8b8c7e57
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
test/metabase/api/collection_test.clj
+ 92
− 131
View file @ 680293aa
......@@ -104,33 +104,18 @@
;;; ----------------------------------------- Cards, Dashboards, and Pulses ------------------------------------------
;; check that cards are returned with the collections detail endpoint
;; check that cards are returned with the collection/items endpoint
(tt/expect-with-temp [Collection [collection]
Card [card {:collection_id (u/get-id collection)}]]
(tu/obj->json->obj
(assoc collection
:items [{:id (u/get-id card)
:name (:name card)
:description nil
:collection_position nil
:favorite false
:model "card"}]
:effective_ancestors []
:effective_location "/"
:can_write true))
[{:id (u/get-id card)
:name (:name card)
:description nil
:collection_position nil
:favorite false
:model "card"}])
(tu/obj->json->obj
(-> ((user->client :crowberto) :get 200 (str "collection/" (u/get-id collection)))
(assoc :items ((user->client :crowberto) :get 200 (str "collection/" (u/get-id collection) "/items"))))))
(defn- remove-ids-from-collection-detail [results & {:keys [keep-collection-id?]
:or {keep-collection-id? false}}]
(into {} (for [[k items] (select-keys results (cond->> [:name :items :can_write]
keep-collection-id? (cons :id)))]
[k (if-not (sequential? items)
items
(for [item items]
(dissoc item :id)))])))
((user->client :crowberto) :get 200 (str "collection/" (u/get-id collection) "/items"))))
(defn- do-with-some-children-of-collection [collection-or-id-or-nil f]
(collection-test/force-create-personal-collections!)
......@@ -144,44 +129,43 @@
(defmacro ^:private with-some-children-of-collection {:style/indent 1} [collection-or-id-or-nil & body]
`(do-with-some-children-of-collection ~collection-or-id-or-nil (fn [] ~@body)))
(defn- default-item [item-map]
(merge {:id true, :collection_position nil} item-map))
(defn- collection-item [collection-name & {:as extra-keypairs}]
(merge {:id true, :description nil,
:model "collection", :name collection-name}
extra-keypairs))
;; check that you get to see the children as appropriate
(expect
{:name "Debt Collection"
:items [{:name "Birthday Card", :description nil, :collection_position nil, :favorite false, :model "card"}
{:name "Dine & Dashboard", :description nil, :collection_position nil, :model "dashboard"}
{:name "Electro-Magnetic Pulse", :collection_position nil, :model "pulse"}]
:can_write false}
(map default-item [{:name "Birthday Card", :description nil, :favorite false, :model "card"}
{:name "Dine & Dashboard", :description nil, :model "dashboard"}
{:name "Electro-Magnetic Pulse", :model "pulse"}])
(tt/with-temp Collection [collection {:name "Debt Collection"}]
(perms/grant-collection-read-permissions! (group/all-users) collection)
(with-some-children-of-collection collection
(-> ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection)))
(assoc :items ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection) "/items")))
remove-ids-from-collection-detail))))
(tu/boolean-ids-and-timestamps
((user->client :rasta) :get 200 (str "collection/" (u/get-id collection) "/items"))))))
;; ...and that you can also filter so that you only see the children you want to see
(expect
{:name "Art Collection"
:items [{:name "Dine & Dashboard", :description nil, :collection_position nil, :model "dashboard"}]
:can_write false}
[(default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"})]
(tt/with-temp Collection [collection {:name "Art Collection"}]
(perms/grant-collection-read-permissions! (group/all-users) collection)
(with-some-children-of-collection collection
(-> ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection)))
(assoc :items ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection) "/items?model=dashboard")))
remove-ids-from-collection-detail))))
(tu/boolean-ids-and-timestamps
((user->client :rasta) :get 200 (str "collection/" (u/get-id collection) "/items?model=dashboard"))))))
;; Let's make sure the `archived` option works.
(expect
{:name "Art Collection"
:items [{:name "Dine & Dashboard", :description nil, :collection_position nil, :model "dashboard"}]
:can_write false}
[(default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"})]
(tt/with-temp Collection [collection {:name "Art Collection"}]
(perms/grant-collection-read-permissions! (group/all-users) collection)
(with-some-children-of-collection collection
(db/update-where! Dashboard {:collection_id (u/get-id collection)} :archived true)
(-> ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection)))
(assoc :items ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection) "/items?archived=true")))
remove-ids-from-collection-detail))))
(tu/boolean-ids-and-timestamps
((user->client :rasta) :get 200 (str "collection/" (u/get-id collection) "/items?archived=true"))))))
;;; --------------------------------- Fetching Personal Collections (Ours & Others') ---------------------------------
......@@ -223,27 +207,23 @@
"You don't have permissions to do that."
(api-get-lucky-personal-collection :rasta, :expected-status-code 403))
(defn- lucky-personal-collection-with-subcollection []
(assoc (lucky-personal-collection)
:items [{:name "Lucky's Personal Sub-Collection", :description nil, :model "collection"}]))
(def ^:private lucky-personal-subcollection-item
[(collection-item "Lucky's Personal Sub-Collection")])
(defn- api-get-lucky-personal-collection-with-subcollection [user-kw]
(tt/with-temp Collection [_ {:name "Lucky's Personal Sub-Collection"
:location (collection/children-location
(collection/user->personal-collection (user->id :lucky)))}]
(-> (api-get-lucky-personal-collection user-kw)
(assoc :items (api-get-lucky-personal-collection-items user-kw))
(update :items (partial map #(dissoc % :id))))))
(tu/boolean-ids-and-timestamps (api-get-lucky-personal-collection-items user-kw))))
;; If we have a sub-Collection of our Personal Collection, that should show up
(expect
(lucky-personal-collection-with-subcollection)
lucky-personal-subcollection-item
(api-get-lucky-personal-collection-with-subcollection :lucky))
;; sub-Collections of other's Personal Collections should show up for admins as well
(expect
(lucky-personal-collection-with-subcollection)
lucky-personal-subcollection-item
(api-get-lucky-personal-collection-with-subcollection :crowberto))
......@@ -265,8 +245,7 @@
"Nicely format the `:effective_` results from an API call."
[results]
(-> results
(select-keys [:items :effective_ancestors :effective_location])
(update :items (partial map #(update % :id integer?)))
(select-keys [:effective_ancestors :effective_location])
(update :effective_ancestors (partial map #(update % :id integer?)))
(update :effective_location collection-test/location-path-ids->names)))
......@@ -274,79 +253,73 @@
"Call the API with Rasta to fetch `collection-or-id` and put the `:effective_` results in a nice format for the tests
below."
[collection-or-id & additional-get-params]
(-> ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection-or-id)))
(assoc :items (apply (user->client :rasta) :get 200 (str "collection/" (u/get-id collection-or-id) "/items")
additional-get-params))
format-ancestors-and-children))
[(format-ancestors-and-children ((user->client :rasta) :get 200 (str "collection/" (u/get-id collection-or-id))))
(tu/boolean-ids-and-timestamps (apply (user->client :rasta) :get 200 (str "collection/" (u/get-id collection-or-id) "/items")
additional-get-params))])
;; does a top-level Collection like A have the correct Children?
(expect
{:items [{:name "B", :id true, :description nil, :model "collection"}
{:name "C", :id true, :description nil, :model "collection"}]
:effective_ancestors []
:effective_location "/"}
[{:effective_ancestors []
:effective_location "/"}
(map collection-item ["B" "C"])]
(with-collection-hierarchy [a b c d g]
(api-get-collection-ancestors-and-children a)))
;; ok, does a second-level Collection have its parent and its children?
(expect
{:items [{:name "D", :id true, :description nil, :model "collection"}
{:name "G", :id true, :description nil, :model "collection"}]
:effective_ancestors [{:name "A", :id true}]
:effective_location "/A/"}
[{:effective_ancestors [{:name "A", :id true}]
:effective_location "/A/"}
(map collection-item ["D" "G"])]
(with-collection-hierarchy [a b c d g]
(api-get-collection-ancestors-and-children c)))
;; what about a third-level Collection?
(expect
{:items []
:effective_ancestors [{:name "A", :id true}
{:name "C", :id true}]
:effective_location "/A/C/"}
[{:effective_ancestors [{:name "A", :id true}
{:name "C", :id true}]
:effective_location "/A/C/"}
[]]
(with-collection-hierarchy [a b c d g]
(api-get-collection-ancestors-and-children d)))
;; for D: if we remove perms for C we should only have A as an ancestor; effective_location should lie and say we are
;; a child of A
(expect
{:items []
:effective_ancestors [{:name "A", :id true}]
:effective_location "/A/"}
[{:effective_ancestors [{:name "A", :id true}]
:effective_location "/A/"}
[]]
(with-collection-hierarchy [a b d g]
(api-get-collection-ancestors-and-children d)))
;; for D: If, on the other hand, we remove A, we should see C as the only ancestor and as a root-level Collection.
(expect
{:items [],
:effective_ancestors [{:name "C", :id true}]
:effective_location "/C/"}
[{:effective_ancestors [{:name "C", :id true}]
:effective_location "/C/"}
[]]
(with-collection-hierarchy [b c d g]
(api-get-collection-ancestors-and-children d)))
;; for C: if we remove D we should get E and F as effective children
(expect
{:items [{:name "E", :id true, :description nil, :model "collection"}
{:name "F", :id true, :description nil, :model "collection"}]
:effective_ancestors [{:name "A", :id true}]
:effective_location "/A/"}
[{:effective_ancestors [{:name "A", :id true}]
:effective_location "/A/"}
(map collection-item ["E" "F"])]
(with-collection-hierarchy [a b c e f g]
(api-get-collection-ancestors-and-children c)))
;; Make sure we can collapse multiple generations. For A: removing C and D should move up E and F
(expect
{:items [{:name "B", :id true, :description nil, :model "collection"}
{:name "E", :id true, :description nil, :model "collection"}
{:name "F", :id true, :description nil, :model "collection"}]
:effective_ancestors []
:effective_location "/"}
[{:effective_ancestors []
:effective_location "/"}
(map collection-item ["B" "E" "F"])]
(with-collection-hierarchy [a b e f g]
(api-get-collection-ancestors-and-children a)))
;; Let's make sure the 'archived` option works on Collections, nested or not
(expect
{:items [{:name "B", :id true, :description nil, :model "collection"}]
:effective_ancestors []
:effective_location "/"}
[{:effective_ancestors []
:effective_location "/"}
[(collection-item "B")]]
(with-collection-hierarchy [a b c]
(db/update! Collection (u/get-id b) :archived true)
(api-get-collection-ancestors-and-children a :archived true)))
......@@ -357,49 +330,42 @@
;;; +----------------------------------------------------------------------------------------------------------------+
;; Check that we can see stuff that isn't in any Collection -- meaning they're in the so-called "Root" Collection
(expect
{:name "Saved items"
:id "root"
:can_write true
:effective_location nil
:effective_ancestors []}
(with-some-children-of-collection nil
((user->client :crowberto) :get 200 "collection/root")))
;; Make sure you can see everything for Users that can see everything
(expect
{:name "Saved items"
:id "root"
:items [{:name "Birthday Card", :description nil, :collection_position nil, :favorite false, :model "card"}
{:name "Crowberto Corv's Personal Collection", :description nil, :model "collection"}
{:name "Dine & Dashboard", :description nil, :collection_position nil, :model "dashboard"}
{:name "Electro-Magnetic Pulse", :collection_position nil, :model "pulse"}]
:can_write true}
[(default-item {:name "Birthday Card", :description nil, :favorite false, :model "card"})
(collection-item "Crowberto Corv's Personal Collection")
(default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"})
(default-item {:name "Electro-Magnetic Pulse", :model "pulse"})]
(with-some-children-of-collection nil
(-> ((user->client :crowberto) :get 200 "collection/root")
(assoc :items ((user->client :crowberto) :get 200 "collection/root/items"))
(remove-ids-from-collection-detail :keep-collection-id? true))))
(tu/boolean-ids-and-timestamps ((user->client :crowberto) :get 200 "collection/root/items"))))
;; ...but we don't let you see stuff you wouldn't otherwise be allowed to see
(expect
{:name "Saved items"
:id "root"
:items [{:name "Rasta Toucan's Personal Collection", :description nil, :model "collection"}]
:can_write false}
[(collection-item "Rasta Toucan's Personal Collection")]
;; if a User doesn't have perms for the Root Collection then they don't get to see things with no collection_id
(with-some-children-of-collection nil
(-> ((user->client :rasta) :get 200 "collection/root")
(assoc :items ((user->client :rasta) :get 200 "collection/root/items"))
(remove-ids-from-collection-detail :keep-collection-id? true))))
(tu/boolean-ids-and-timestamps ((user->client :rasta) :get 200 "collection/root/items"))))
;; ...but if they have read perms for the Root Collection they should get to see them
(expect
{:name "Saved items"
:id "root"
:items [{:name "Birthday Card", :collection_position nil, :description nil, :favorite false, :model "card"}
{:name "Dine & Dashboard", :collection_position nil, :description nil, :model "dashboard"}
{:name "Electro-Magnetic Pulse", :collection_position nil, :model "pulse"}
{:name "Rasta Toucan's Personal Collection", :description nil, :model "collection"}]
:can_write false}
[(default-item {:name "Birthday Card", :description nil, :favorite false, :model "card"})
(default-item {:name "Dine & Dashboard", :description nil, :model "dashboard"})
(default-item {:name "Electro-Magnetic Pulse", :model "pulse"})
(collection-item "Rasta Toucan's Personal Collection")]
(with-some-children-of-collection nil
(tt/with-temp* [PermissionsGroup [group]
PermissionsGroupMembership [_ {:user_id (user->id :rasta), :group_id (u/get-id group)}]]
(perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection/is-root? true}))
(-> ((user->client :rasta) :get 200 "collection/root")
(assoc :items ((user->client :rasta) :get 200 "collection/root/items"))
(remove-ids-from-collection-detail :keep-collection-id? true)))))
(tu/boolean-ids-and-timestamps ((user->client :rasta) :get 200 "collection/root/items")))))
;; So I suppose my Personal Collection should show up when I fetch the Root Collection, shouldn't it...
(expect
......@@ -454,35 +420,30 @@
tests below."
[& additional-get-params]
(collection-test/force-create-personal-collections!)
(-> ((user->client :rasta) :get 200 "collection/root")
(assoc :items (apply (user->client :rasta) :get 200 "collection/root/items" additional-get-params))
format-ancestors-and-children))
[(format-ancestors-and-children ((user->client :rasta) :get 200 "collection/root"))
(tu/boolean-ids-and-timestamps (apply (user->client :rasta) :get 200 "collection/root/items" additional-get-params))])
;; Do top-level collections show up as children of the Root Collection?
(expect
{:items [{:name "A", :id true, :description nil, :model "collection"}
{:name "Rasta Toucan's Personal Collection", :id true, :description nil, :model "collection"}]
:effective_ancestors []
:effective_location nil}
[{:effective_ancestors []
:effective_location nil}
(map collection-item ["A" "Rasta Toucan's Personal Collection"])]
(with-collection-hierarchy [a b c d e f g]
(api-get-root-collection-ancestors-and-children)))
;; ...and collapsing children should work for the Root Collection as well
(expect
{:items [{:name "B", :id true, :description nil, :model "collection"}
{:name "D", :id true, :description nil, :model "collection"}
{:name "F", :id true, :description nil, :model "collection"}
{:name "Rasta Toucan's Personal Collection", :id true, :description nil, :model "collection"}]
:effective_ancestors []
:effective_location nil}
[{:effective_ancestors []
:effective_location nil}
(map collection-item ["B" "D" "F" "Rasta Toucan's Personal Collection"])]
(with-collection-hierarchy [b d e f g]
(api-get-root-collection-ancestors-and-children)))
;; does `archived` work on Collections as well?
(expect
{:items [{:name "A", :id true, :description nil, :model "collection"}]
:effective_ancestors []
:effective_location nil}
[{:effective_ancestors []
:effective_location nil}
[(collection-item "A")]]
(with-collection-hierarchy [a b d e f g]
(db/update! Collection (u/get-id a) :archived true)
(api-get-root-collection-ancestors-and-children :archived true)))
......
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