From 8e3b4ad8fb07634082dcbd1e613a8886f7a3168e Mon Sep 17 00:00:00 2001
From: dpsutton <dan@dpsutton.com>
Date: Thu, 3 Jun 2021 12:24:39 -0500
Subject: [PATCH] Paginated collection metadata (#16275)
* Add the last-edit-info to the child query
just for cards right now, need to extend to dashboards and then make
sure we clean it up into the proper map.
Have a set of all columns that are required and ensure those are in
each query. Nulls need to be cast to the correct type if they aren't a
text column (at least on postgres, need to investigate h2 and mysql).
* Correct find last-edit-info for cards and dashboards
* Move from group-by max id to self join
* Paginate and search on collection items
- include the metadata information in the query
- to accomplish this, make a backstop to add all columns into each
query
- need types to union the null. in postgres it assumes all nulls are
text. Haven't checked what mysql and h2 do here yet
- sort_column: name, last_edited, model
- sort_direction: asc, desc
* Add default sort column and direction
* handle mysql
mysql doesn't need types on nulls to unify them
* Correct batch post processing for cards and dashboards
* Correct order of expectations
they were alphabetical inside of models
previous order:
- acme products | pulse
- electro-magnetic pulse | pulse
- birthday card | card
- dine & Dashboard | dashboard
and is now ordered solely by name
* Tests and sort on lower name after timestamp
When sorting by last edited, lots of things don't have this, so don't
want arbitrary sorting, but sort by name afterwards
* Ensure nulls go to the end of the list
* Tests
* trying to make h2, mysql, and pg happy
sorting timestamps and always having nulls last is quite difficult
* Make diff bigger
this was going in as a string and losing the timezone information, so
was actually advancing time.
Now: (java.time.OffsetDateTime/now)
"2021-06-01T13:59:33.165483Z"
Minus 2 hours: (.minusHours (java.time.OffsetDateTime/now) 2)
"2021-06-01T11:59:27.528500Z"
As seen in the code:
;; unaltered "now"
"2021-06-01T08:55:28"
;; altered "two hours ago"
"2021-06-01T11:55:29" <- UTC shenanigans. so just bump back 24 hours
* Sort by model ranking
* Remove clj-time and use ZonedDateTime instead of OffsetDateTime
---
src/metabase/api/collection.clj | 260 +++++++++++++++++++-------
test/metabase/api/collection_test.clj | 108 ++++++++---
2 files changed, 277 insertions(+), 91 deletions(-)
diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj
index c1261ec76c4..1140b01094b 100644
--- a/src/metabase/api/collection.clj
+++ b/src/metabase/api/collection.clj
@@ -4,10 +4,14 @@
`:snippet` namespace, (called 'Snippet folders' in the UI). These namespaces are completely independent hierarchies.
To use these endpoints for other Collections namespaces, you can pass the `?namespace=` parameter (e.g.
`?namespace=snippet`)."
- (:require [compojure.core :refer [GET POST PUT]]
+ (:require [clojure.string :as str]
+ [compojure.core :refer [GET POST PUT]]
+ [honeysql.core :as hsql]
[honeysql.helpers :as h]
+ [medley.core :as m]
[metabase.api.card :as card-api]
[metabase.api.common :as api]
+ [metabase.db.env :as mdb.env]
[metabase.models.card :refer [Card]]
[metabase.models.collection :as collection :refer [Collection]]
[metabase.models.collection.graph :as collection.graph]
@@ -91,12 +95,18 @@
"Valid values for the `?pinned_state` param accepted by endpoints in this namespace."
#{"all" "is_pinned" "is_not_pinned"})
+(def ^:private valid-sort-columns #{"name" "last_edited" "model"})
+(def ^:private valid-sort-directions #{"asc" "desc"})
+(defn- normalize-sort-choice [w] (when w (keyword (str/replace w #"_" "-"))))
+
(def ^:private CollectionChildrenOptions
- {:archived? s/Bool
- :pinned-state (s/maybe (apply s/enum (map keyword valid-pinned-state-values)))
+ {:archived? s/Bool
+ :pinned-state (s/maybe (apply s/enum (map keyword valid-pinned-state-values)))
;; when specified, only return results of this type.
- :models (s/maybe #{(apply s/enum (map keyword valid-model-param-values))})})
+ :models (s/maybe #{(apply s/enum (map keyword valid-model-param-values))})
+ :sort-info (s/maybe [(s/one (apply s/enum (map normalize-sort-choice valid-sort-columns)) "sort-columns")
+ (s/one (apply s/enum (map normalize-sort-choice valid-sort-directions)) "sort-direction")])})
(defmulti ^:private collection-children-query
"Query that will fetch the 'children' of a `collection`, for different types of objects. Possible options are listed
@@ -105,7 +115,11 @@
NOTES:
* `collection` will be either a CollectionInstance, or the Root Collection special placeholder object, so do not use
- `u/the-id` on it! Use `:id`, which will return `nil` for the Root Collection, which is exactly what we want."
+ `u/the-id` on it! Use `:id`, which will return `nil` for the Root Collection, which is exactly what we want.
+
+ * These queries will be combined into a union-all query. You do not need to put all of the columns into the query,
+ any you don't select will be added in the correct position so the union will work (see `all-select-columns` for more
+ details)."
{:arglists '([model collection options])}
(fn [model _ _] (keyword model)))
@@ -132,9 +146,7 @@
[_ collection {:keys [archived? pinned-state]}]
(-> {:select [:p.id
:p.name
- [nil :p.description]
:p.collection_position
- [nil :p.display]
[(hx/literal "pulse") :model]]
:modifiers [:distinct]
:from [[Pulse :p]]
@@ -155,7 +167,7 @@
(defmethod collection-children-query :snippet
[_ collection {:keys [archived? pinned-state]}]
- {:select [:id :name [nil :description] [nil :collection_position] [nil :display] [(hx/literal "snippet") :model]]
+ {:select [:id :name [(hx/literal "snippet") :model]]
:from [[NativeQuerySnippet :nqs]]
:where [:and
[:= :collection_id (:id collection)]
@@ -168,38 +180,58 @@
(defmethod collection-children-query :card
[_ collection {:keys [archived? pinned-state]}]
- (-> {:select [:id :name :description :collection_position :display [(hx/literal "card") :model]]
- :from [[Card :c]]
- :where [:and
- [:= :collection_id (:id collection)]
- [:= :archived (boolean archived?)]]}
+ (-> {:select [:c.id :c.name :c.description :c.collection_position :c.display [(hx/literal "card") :model]
+ [:u.id :last_edit_user] [:u.email :last_edit_email]
+ [:u.first_name :last_edit_first_name] [:u.last_name :last_edit_last_name]
+ [:r.timestamp :last_edit_timestamp]]
+ :from [[Card :c]]
+ ;; todo: should there be a flag, or a realized view?
+ :left-join [[{:select [:r1.*]
+ :from [[:revision :r1]]
+ :left-join [[:revision :r2] [:and
+ [:= :r1.model_id :r2.model_id]
+ [:= :r1.model :r2.model]
+ [:> :r1.id :r2.id]]]
+ :where [:and
+ [:= :r2.id nil]
+ [:= :r1.model (hx/literal "Card")]]} :r]
+ [:= :r.model_id :c.id]
+ [:core_user :u] [:= :u.id :r.user_id]]
+ :where [:and
+ [:= :collection_id (:id collection)]
+ [:= :archived (boolean archived?)]]}
(h/merge-where (pinned-state->clause pinned-state))))
(defmethod post-process-collection-children :card
[_ rows]
- (let [last-edits (last-edit/fetch-last-edited-info {:card-ids (->> rows (map :id))})]
- (for [row (hydrate rows :favorite)]
- (if-let [edit-info (get-in last-edits [:card (:id row)])]
- (assoc row :last-edit-info edit-info)
- row))))
+ (hydrate rows :favorite))
(defmethod collection-children-query :dashboard
[_ collection {:keys [archived? pinned-state]}]
- (-> {:select [:id :name :description :collection_position [nil :display] [(hx/literal "dashboard") :model]]
- :from [[Dashboard :d]]
- :where [:and
- [:= :collection_id (:id collection)]
- [:= :archived (boolean archived?)]]}
+ (-> {:select [:d.id :d.name :d.description :d.collection_position [(hx/literal "dashboard") :model]
+ [:u.id :last_edit_user] [:u.email :last_edit_email]
+ [:u.first_name :last_edit_first_name] [:u.last_name :last_edit_last_name]
+ [:r.timestamp :last_edit_timestamp]]
+ :from [[Dashboard :d]]
+ :left-join [[{:select [:r1.*]
+ :from [[:revision :r1]]
+ :left-join [[:revision :r2] [:and
+ [:= :r1.model_id :r2.model_id]
+ [:= :r1.model :r2.model]
+ [:> :r1.id :r2.id]]]
+ :where [:and
+ [:= :r2.id nil]
+ [:= :r1.model (hx/literal "Dashboard")]]} :r]
+ [:= :r.model_id :d.id]
+ [:core_user :u] [:= :u.id :r.user_id]]
+ :where [:and
+ [:= :collection_id (:id collection)]
+ [:= :archived (boolean archived?)]]}
(h/merge-where (pinned-state->clause pinned-state))))
(defmethod post-process-collection-children :dashboard
[_ rows]
- (let [last-edits (last-edit/fetch-last-edited-info {:dashboard-ids (->> rows (map :id))})]
- (for [row (hydrate rows :favorite)]
- (let [res (dissoc row :display)]
- (if-let [edit-info (get-in last-edits [:dashboard (:id res)])]
- (assoc res :last-edit-info edit-info)
- res)))))
+ (hydrate (map #(dissoc % :display) rows) :favorite))
(defmethod collection-children-query :collection
[_ collection {:keys [archived? collection-namespace pinned-state]}]
@@ -212,8 +244,6 @@
:select [:id
:name
:description
- [nil :collection_position]
- [nil :display]
[(hx/literal "collection") :model]])
;; the nil indicates that collections are never pinned.
(h/merge-where (pinned-state->clause pinned-state nil))))
@@ -229,11 +259,35 @@
:can_write
(mi/can-write? Collection (:id row)))))
-(defn- post-process-rows [rows]
- (mapcat
- (fn [[model rows]]
- (post-process-collection-children (keyword model) rows))
- (group-by :model rows)))
+(s/defn ^:private coalesce-edit-info :- last-edit/MaybeAnnotated
+ "Hoist all of the last edit information into a map under the key :last-edit-info. Considers this information present
+ if `:last_edit_user` is not nil."
+ [row]
+ (letfn [(select-as [original k->k']
+ (reduce (fn [m [k k']] (assoc m k' (get original k)))
+ {}
+ k->k'))]
+ (let [mapping {:last_edit_user :id
+ :last_edit_last_name :last_name
+ :last_edit_first_name :first_name
+ :last_edit_email :email
+ :last_edit_timestamp :timestamp}]
+ (cond-> (apply dissoc row :model_ranking (keys mapping))
+ ;; don't use contains as they all have the key, we care about a value present
+ (:last_edit_user row) (assoc :last-edit-info (select-as row mapping))))))
+
+(defn- post-process-rows
+ "Post process any data. Have a chance to process all of the same type at once using
+ `post-process-collection-children`. Must respect the order passed in."
+ [rows]
+ (->> (map-indexed (fn [i row] (vary-meta row assoc ::index i)) rows) ;; keep db sort order
+ (group-by :model)
+ (into []
+ (comp (map (fn [[model rows]]
+ (post-process-collection-children (keyword model) rows)))
+ cat
+ (map coalesce-edit-info)))
+ (sort-by (comp ::index meta))))
(defn- model-name->toucan-model [model-name]
(case (keyword model-name)
@@ -243,20 +297,92 @@
:pulse Pulse
:snippet NativeQuerySnippet))
+(defn- select-name
+ "Takes a honeysql select column and returns a keyword of which column it is.
+
+ eg:
+ (select-name :id) -> :id
+ (select-name [(literal \"card\") :model]) -> :model
+ (select-name :p.id) -> :id"
+ [x]
+ (if (vector? x)
+ (recur (second x))
+ (-> x name (str/split #"\.") peek keyword)))
+
+(def ^:private all-select-columns
+ "All columns that need to be present for the union-all. Generated with the comment form below. Non-text columns that
+ are optional (not id, but last_edit_user for example) must have a type so that the union-all can unify the nil with
+ the correct column type."
+ [:id :name :description :display :model :collection_position
+ :last_edit_email :last_edit_first_name :last_edit_last_name
+ [:last_edit_user :integer] [:last_edit_timestamp :timestamp]])
+
+(defn- add-missing-columns
+ "Ensures that all necessary columns are in the select-columns collection, adding `[nil :column]` as necessary."
+ [select-columns necessary-columns]
+ (let [columns (m/index-by select-name select-columns)]
+ (map (fn [col]
+ (let [[col-name typpe] (u/one-or-many col)]
+ (get columns col-name (if (and typpe (= @mdb.env/db-type :postgres))
+ [(hx/cast typpe nil) col-name]
+ [nil col-name]))))
+ necessary-columns)))
+
+(defn- add-model-ranking
+ [select-clause model]
+ (let [rankings {:dashboard 1
+ :pulse 2
+ :card 3
+ :snippet 4
+ :collection 5}]
+ (conj select-clause [(get rankings model 100)
+ :model_ranking])))
+
+(comment
+ ;; generate the set of columns across all child queries. Remember to add type info if not a text column
+ (into []
+ (comp cat (map select-name) (distinct))
+ (for [model [:card :dashboard :snippet :pulse :collection]]
+ (:select (collection-children-query model {:id 1 :location "/"} nil))))
+ )
+
(defn- collection-children*
- [collection models options]
- (let [models (sort (map keyword models))
- queries (for [model models]
- (collection-children-query model collection options))
- total-query {:select [[:%count.* :count]]
- :from [[{:union-all queries} :dummy_alias]]}
- rows-query {:select [:*]
- :from [[{:union-all queries} :dummy_alias]]
- :order-by [[:%lower.name :asc]]
- :limit offset-paging/*limit*
- :offset offset-paging/*offset*}]
- {:total (-> (db/query total-query) first :count)
- :data (-> (db/query rows-query) post-process-rows)
+ [collection models {:keys [sort-info] :as options}]
+ (let [sql-order (case sort-info
+ nil [[:%lower.name :asc]]
+ [:name :asc] [[:%lower.name :asc]]
+ [:name :desc] [[:%lower.name :desc]]
+ [:last-edited :asc] [(if (= @mdb.env/db-type :mysql)
+ [(hsql/call :ISNULL :last_edit_timestamp)]
+ [:last_edit_timestamp :nulls-last])
+ [:last_edit_timestamp :asc]
+ [:%lower.name :asc]]
+ [:last-edited :desc] (into
+ (case @mdb.env/db-type
+ :mysql
+ [[(hsql/call :ISNULL :last_edit_timestamp)]]
+ :postgres
+ [[(hsql/raw "last_edit_timestamp DESC NULLS LAST")]]
+ :h2
+ [])
+ [[:last_edit_timestamp :desc]
+ [:%lower.name :asc]])
+ [:model :asc] [[:model_ranking :asc] [:%lower.name :asc]]
+ [:model :desc] [[:model_ranking :desc] [:%lower.name :asc]])
+ models (sort (map keyword models))
+ queries (for [model models]
+ (-> (collection-children-query model collection options)
+ (update :select add-missing-columns all-select-columns)
+ (update :select add-model-ranking model)))
+ total-query {:select [[:%count.* :count]]
+ :from [[{:union-all queries} :dummy_alias]]}
+ rows-query {:select [:*]
+ :from [[{:union-all queries} :dummy_alias]]
+ :order-by sql-order
+ :limit offset-paging/*limit*
+ :offset offset-paging/*offset*}]
+ {:total (->> (db/query total-query) first :count)
+ :data (->> (db/query rows-query) post-process-rows)
:limit offset-paging/*limit*
:offset offset-paging/*offset*
:models models}))
@@ -295,15 +421,19 @@
* `pinned_state` - when `is_pinned`, return pinned objects only.
when `is_not_pinned`, return non pinned objects only.
when `all`, return everything. By default returns everything"
- [id models archived pinned_state]
- {models (s/maybe models-schema)
- archived (s/maybe su/BooleanString)
- pinned_state (s/maybe (apply s/enum valid-pinned-state-values))}
- (let [model-kwds (set (map keyword (u/one-or-many models)))]
+ [id models archived pinned_state sort_column sort_direction]
+ {models (s/maybe models-schema)
+ archived (s/maybe su/BooleanString)
+ pinned_state (s/maybe (apply s/enum valid-pinned-state-values))
+ sort_column (s/maybe (apply s/enum valid-sort-columns))
+ sort_direction (s/maybe (apply s/enum valid-sort-directions))}
+ (let [model-kwds (set (map keyword (u/one-or-many models)))]
(collection-children (api/read-check Collection id)
{:models model-kwds
:archived? (Boolean/parseBoolean archived)
- :pinned-state (keyword pinned_state)})))
+ :pinned-state (keyword pinned_state)
+ :sort-info [(or (some-> sort_column normalize-sort-choice) :name)
+ (or (some-> sort_direction normalize-sort-choice) :asc)]})))
;;; -------------------------------------------- GET /api/collection/root --------------------------------------------
@@ -331,11 +461,13 @@
By default, this will show the 'normal' Collections namespace; to view a different Collections namespace, such as
`snippets`, you can pass the `?namespace=` parameter."
- [models archived namespace pinned_state]
- {models (s/maybe models-schema)
- archived (s/maybe su/BooleanString)
- namespace (s/maybe su/NonBlankString)
- pinned_state (s/maybe (apply s/enum valid-pinned-state-values))}
+ [models archived namespace pinned_state sort_column sort_direction]
+ {models (s/maybe models-schema)
+ archived (s/maybe su/BooleanString)
+ namespace (s/maybe su/NonBlankString)
+ pinned_state (s/maybe (apply s/enum valid-pinned-state-values))
+ sort_column (s/maybe (apply s/enum valid-sort-columns))
+ sort_direction (s/maybe (apply s/enum valid-sort-directions))}
;; Return collection contents, including Collections that have an effective location of being in the Root
;; Collection for the Current User.
(let [root-collection (assoc collection/root-collection :namespace namespace)
@@ -344,9 +476,11 @@
#{:collection})]
(collection-children
root-collection
- {:models model-kwds
- :archived? (Boolean/parseBoolean archived)
- :pinned-state (keyword pinned_state)})))
+ {:models model-kwds
+ :archived? (Boolean/parseBoolean archived)
+ :pinned-state (keyword pinned_state)
+ :sort-info [(or (some-> sort_column normalize-sort-choice) :name)
+ (or (some-> sort_direction normalize-sort-choice) :asc)]})))
;;; ----------------------------------------- Creating/Editing a Collection ------------------------------------------
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index 23a2b1cff13..47bd6ddde70 100644
--- a/test/metabase/api/collection_test.clj
+++ b/test/metabase/api/collection_test.clj
@@ -16,7 +16,8 @@
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
[schema.core :as s]
- [toucan.db :as db]))
+ [toucan.db :as db])
+ (:import [java.time ZonedDateTime ZoneId]))
(use-fixtures :once (fixtures/initialize :test-users-personal-collections))
@@ -301,15 +302,15 @@
(mt/with-temp* [Collection [collection]
Card [card {:collection_id (u/the-id collection)}]]
(is (= (mt/obj->json->obj
- [{:id (u/the-id card)
- :name (:name card)
- :collection_position nil
- :display "table"
- :description nil
- :favorite false
- :model "card"}])
+ [{:id (u/the-id card)
+ :name (:name card)
+ :collection_position nil
+ :display "table"
+ :description nil
+ :favorite false
+ :model "card"}])
(mt/obj->json->obj
- (:data (mt/user-http-request :crowberto :get 200 (str "collection/" (u/the-id collection) "/items"))))))))
+ (:data (mt/user-http-request :crowberto :get 200 (str "collection/" (u/the-id collection) "/items"))))))))
(testing "check that limit and offset work and total comes back"
(mt/with-temp* [Collection [collection]
Card [card3 {:collection_id (u/the-id collection)}]
@@ -332,9 +333,9 @@
(perms/grant-collection-read-permissions! (group/all-users) collection)
(with-some-children-of-collection collection
(is (= (map default-item [{:name "Acme Products", :model "pulse"}
- {:name "Electro-Magnetic Pulse", :model "pulse"}
{:name "Birthday Card", :description nil, :favorite false, :model "card", :display "table"}
- {:name "Dine & Dashboard", :description nil, :favorite false, :model "dashboard"}])
+ {:name "Dine & Dashboard", :description nil, :favorite false, :model "dashboard"}
+ {:name "Electro-Magnetic Pulse", :model "pulse"}])
(mt/boolean-ids-and-timestamps
(:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items"))))))))
@@ -359,26 +360,77 @@
(is (= [(default-item {:name "Dine & Dashboard", :description nil, :favorite false, :model "dashboard"})]
(mt/boolean-ids-and-timestamps
(:data (mt/user-http-request :rasta :get 200 (str "collection/" (u/the-id collection) "/items?archived=true")))))))))
- (testing "Results include last edited information from the `Revision` table"
- (mt/with-temp* [Collection [{collection-id :id} {:name "Collection with Items"}]
- User [{user-id :id} {:first_name "Test" :last_name "User" :email "testuser@example.com"}]
- Card [{card-id :id :as card}
- {:name "Card with history" :collection_id collection-id}]
- Revision [_revision {:model "Card"
- :model_id card-id
- :user_id user-id
- :object (revision/serialize-instance card card-id card)}]]
- (is (= [{:name "Card with history",
+ (mt/with-temp* [Collection [{collection-id :id} {:name "Collection with Items"}]
+ User [{user-id :id} {:first_name "Test" :last_name "User" :email "testuser@example.com"}]
+ Card [{card1-id :id :as card1}
+ {:name "Card with history 1" :collection_id collection-id}]
+ Card [{card2-id :id :as card2}
+ {:name "Card with history 2" :collection_id collection-id}]
+ Card [_ {:name "ZZ" :collection_id collection-id}]
+ Card [_ {:name "AA" :collection_id collection-id}]
+ Revision [_revision1 {:model "Card"
+ :model_id card1-id
+ :user_id user-id
+ :object (revision/serialize-instance card1 card1-id card1)}]
+ Revision [_revision2 {:model "Card"
+ :model_id card2-id
+ :user_id user-id
+ :object (revision/serialize-instance card2 card2-id card2)}]]
+ ;; need different timestamps and Revision has a pre-update to throw as they aren't editable
+ (db/execute! {:update :revision
+ ;; in the past
+ :set {:timestamp (.minusHours (ZonedDateTime/now (ZoneId/of "UTC")) 2)}
+ :where [:= :id (:id _revision1)]})
+ (testing "Results include last edited information from the `Revision` table"
+ (is (= [{:name "AA"}
+ {:name "Card with history 1",
:last-edit-info
{:id true,
:email "testuser@example.com",
:first_name "Test",
:last_name "User",
;; timestamp collapsed to true, ordinarily a OffsetDateTime
- :timestamp true}}]
+ :timestamp true}}
+ {:name "Card with history 2",
+ :last-edit-info
+ {:id true,
+ :email "testuser@example.com",
+ :first_name "Test",
+ :last_name "User",
+ :timestamp true}}
+ {:name "ZZ"}]
(->> (:data (mt/user-http-request :rasta :get 200 (str "collection/" collection-id "/items")))
mt/boolean-ids-and-timestamps
- (map #(select-keys % [:name :last-edit-info])))))))))
+ (map #(select-keys % [:name :last-edit-info]))))))
+ (testing "Results can be ordered by last-edited"
+ (testing "ascending"
+ (is (= ["Card with history 1" "Card with history 2" "AA" "ZZ"]
+ (->> (mt/user-http-request :rasta :get 200 (str "collection/" collection-id "/items?sort_column=last_edited&sort_direction=asc"))
+ :data
+ (map :name)))))
+ (testing "descending"
+ (is (= ["Card with history 2" "Card with history 1" "AA" "ZZ"]
+ (->> (mt/user-http-request :rasta :get 200 (str "collection/" collection-id "/items?sort_column=last_edited&sort_direction=desc"))
+ :data
+ (map :name))))))
+ (testing "Results can be ordered by model"
+ (mt/with-temp* [Collection [{collection-id :id} {:name "Collection with Items"}]
+ Card [_ {:name "ZZ" :collection_id collection-id}]
+ Card [_ {:name "AA" :collection_id collection-id}]
+ Dashboard [_ {:name "ZZ" :collection_id collection-id}]
+ Dashboard [_ {:name "AA" :collection_id collection-id}]
+ Pulse [_ {:name "ZZ" :collection_id collection-id}]
+ Pulse [_ {:name "AA" :collection_id collection-id}]]
+ (testing "sort direction asc"
+ (is (= [["dashboard" "AA"] ["dashboard" "ZZ"] ["pulse" "AA"] ["pulse" "ZZ"] ["card" "AA"] ["card" "ZZ"]]
+ (->> (mt/user-http-request :rasta :get 200 (str "collection/" collection-id "/items?sort_column=model&sort_direction=asc"))
+ :data
+ (map (juxt :model :name))))))
+ (testing "sort direction desc"
+ (is (= [["card" "AA"] ["card" "ZZ"] ["pulse" "AA"] ["pulse" "ZZ"] ["dashboard" "AA"] ["dashboard" "ZZ"]]
+ (->> (mt/user-http-request :rasta :get 200 (str "collection/" collection-id "/items?sort_column=model&sort_direction=desc"))
+ :data
+ (map (juxt :model :name)))))))))))
(deftest snippet-collection-items-test
(testing "GET /api/collection/:id/items"
@@ -602,11 +654,11 @@
(with-some-children-of-collection nil
(mt/user-http-request :crowberto :get 200 "collection/root")))))
(testing "Make sure you can see everything for Users that can see everything"
- (is (= [(default-item {:name "Electro-Magnetic Pulse", :model "pulse"})
- (default-item {:name "Birthday Card", :description nil, :favorite false, :model "card", :display "table"})
+ (is (= [(default-item {:name "Birthday Card", :description nil, :favorite false, :model "card", :display "table"})
(collection-item "Crowberto Corv's Personal Collection")
(default-item {:name "Dine & Dashboard",
- :favorite false, :description nil, :model "dashboard"}) ]
+ :favorite false, :description nil, :model "dashboard"})
+ (default-item {:name "Electro-Magnetic Pulse", :model "pulse"}) ]
(with-some-children-of-collection nil
(-> (:data (mt/user-http-request :crowberto :get 200 "collection/root/items"))
(remove-non-test-items &ids)
@@ -636,9 +688,9 @@
(mt/with-temp* [PermissionsGroup [group]
PermissionsGroupMembership [_ {:user_id (mt/user->id :rasta), :group_id (u/the-id group)}]]
(perms/grant-permissions! group (perms/collection-read-path {:metabase.models.collection.root/is-root? true}))
- (is (= [(default-item {:name "Electro-Magnetic Pulse", :model "pulse"})
- (default-item {:name "Birthday Card", :description nil, :favorite false, :model "card", :display "table"})
+ (is (= [(default-item {:name "Birthday Card", :description nil, :favorite false, :model "card", :display "table"})
(default-item {:name "Dine & Dashboard", :description nil, :favorite false, :model "dashboard"})
+ (default-item {:name "Electro-Magnetic Pulse", :model "pulse"})
(collection-item "Rasta Toucan's Personal Collection")]
(-> (:data (mt/user-http-request :rasta :get 200 "collection/root/items"))
(remove-non-test-items &ids)
--
GitLab