Skip to content

Commit

Permalink
Merge pull request metabase#7906 from metabase/collection-api-items-t…
Browse files Browse the repository at this point in the history 
…est-split

Split `/collection/:id` and `/collection/:id/items` tests
  • Loading branch information
@senior
senior authored Jun 20, 2018
2 parents 9ea31ab + 680293a commit 7d1d6da
Showing 1 changed file with 92 additions and 131 deletions.
223 changes: 92 additions & 131 deletions test/metabase/api/collection_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand All @@ -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') ---------------------------------

Expand Down Expand Up @@ -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))


Expand All @@ -265,88 +245,81 @@
"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)))

(defn- api-get-collection-ancestors-and-children
"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)))
Expand All @@ -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
Expand Down Expand Up @@ -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)))
Expand Down

0 comments on commit 7d1d6da

Please sign in to comment.