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

Extract largish sort-order function (#16577) 

* Extract largish sort-order function

also, running tests locally could run into timezone issues. only
interested in comparisons (<) so just make sure that its 24 hours in
the past so no issues with time zones

* tests are pretty literal but just satisfying the test checker

* bump coverage from 71 to hopefully 75%
parent 0e5230ea
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/collection.clj
+ 45
− 48
View file @ 0574766e
...@@ -349,56 +349,53 @@ ...@@ -349,56 +349,53 @@
(:select (collection-children-query model {:id 1 :location "/"} nil))))) (:select (collection-children-query model {:id 1 :location "/"} nil)))))
(defn children-sort-clause
"Given the client side sort-info, return sort clause to effect this. `db-type` is necessary due to complications from
treatment of nulls in the different app db types."
[sort-info db-type]
(case sort-info
nil [[:%lower.name :asc]]
[:name :asc] [[:%lower.name :asc]]
[:name :desc] [[:%lower.name :desc]]
[:last-edited-at :asc] [(if (= db-type :mysql)
[(hsql/call :ISNULL :last_edit_timestamp)]
[:last_edit_timestamp :nulls-last])
[:last_edit_timestamp :asc]
[:%lower.name :asc]]
[:last-edited-at :desc] (remove nil?
[(case db-type
:mysql [(hsql/call :ISNULL :last_edit_timestamp)]
:postgres [:last_edit_timestamp :desc :nulls-last]
:h2 nil)
[:last_edit_timestamp :desc]
[:%lower.name :asc]])
[:last-edited-by :asc] [(if (= db-type :mysql)
[(hsql/call :ISNULL :last_edit_last_name)]
[:last_edit_last_name :nulls-last])
[:last_edit_last_name :asc]
(if (= db-type :mysql)
[(hsql/call :ISNULL :last_edit_first_name)]
[:last_edit_first_name :nulls-last])
[:last_edit_first_name :asc]
[:%lower.name :asc]]
[:last-edited-by :desc] (remove nil?
[(case db-type
:mysql [(hsql/call :ISNULL :last_edit_last_name)]
:postgres [:last_edit_last_name :desc :nulls-last]
:h2 nil)
[:last_edit_last_name :desc]
(case db-type
:mysql [(hsql/call :ISNULL :last_edit_first_name)]
:postgres [:last_edit_last_name :desc :nulls-last]
:h2 nil)
[:last_edit_first_name :desc]
[:%lower.name :asc]])
[:model :asc] [[:model_ranking :asc] [:%lower.name :asc]]
[:model :desc] [[:model_ranking :desc] [:%lower.name :asc]]))
(defn- collection-children* (defn- collection-children*
[collection models {:keys [sort-info] :as options}] [collection models {:keys [sort-info] :as options}]
(let [sql-order (case sort-info (let [sql-order (children-sort-clause sort-info @mdb.env/db-type)
nil [[:%lower.name :asc]]
[:name :asc] [[:%lower.name :asc]]
[:name :desc] [[:%lower.name :desc]]
[:last-edited-at :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-at :desc] (keep identity
[(case @mdb.env/db-type
:mysql
[(hsql/call :ISNULL :last_edit_timestamp)]
:postgres
[(hsql/raw "last_edit_timestamp DESC NULLS LAST")]
:h2
nil)
[:last_edit_timestamp :desc]
[:%lower.name :asc]])
[:last-edited-by :asc] [(if (= @mdb.env/db-type :mysql)
[(hsql/call :ISNULL :last_edit_last_name)]
[:last_edit_last_name :nulls-last])
[:last_edit_last_name :asc]
(if (= @mdb.env/db-type :mysql)
[(hsql/call :ISNULL :last_edit_first_name)]
[:last_edit_first_name :nulls-last])
[:last_edit_first_name :asc]
[:%lower.name :asc]]
[:last-edited-by :desc] (keep identity
[(case @mdb.env/db-type
:mysql
[(hsql/call :ISNULL :last_edit_last_name)]
:postgres
[(hsql/raw "last_edit_last_name DESC NULLS LAST")]
:h2
nil)
[:last_edit_last_name :desc]
(case @mdb.env/db-type
:mysql
[(hsql/call :ISNULL :last_edit_first_name)]
:postgres
[(hsql/raw "last_edit_first_name DESC NULLS LAST")]
:h2
nil)
[:last_edit_first_name :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)) models (sort (map keyword models))
queries (for [model models] queries (for [model models]
(-> (collection-children-query model collection options) (-> (collection-children-query model collection options)
......
test/metabase/api/collection_test.clj
+ 38
− 1
View file @ 0574766e
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
"Tests for /api/collection endpoints." "Tests for /api/collection endpoints."
(:require [clojure.string :as str] (:require [clojure.string :as str]
[clojure.test :refer :all] [clojure.test :refer :all]
[honeysql.core :as hsql]
[metabase.api.collection :as api-coll]
[metabase.models :refer [Card Collection Dashboard DashboardCard NativeQuerySnippet PermissionsGroup [metabase.models :refer [Card Collection Dashboard DashboardCard NativeQuerySnippet PermissionsGroup
PermissionsGroupMembership Pulse PulseCard PulseChannel PulseChannelRecipient PermissionsGroupMembership Pulse PulseCard PulseChannel PulseChannelRecipient
Revision User]] Revision User]]
...@@ -398,7 +400,7 @@ ...@@ -398,7 +400,7 @@
;; need different timestamps and Revision has a pre-update to throw as they aren't editable ;; need different timestamps and Revision has a pre-update to throw as they aren't editable
(db/execute! {:update :revision (db/execute! {:update :revision
;; in the past ;; in the past
:set {:timestamp (.minusHours (ZonedDateTime/now (ZoneId/of "UTC")) 2)} :set {:timestamp (.minusHours (ZonedDateTime/now (ZoneId/of "UTC")) 24)}
:where [:= :id (:id _revision1)]}) :where [:= :id (:id _revision1)]})
(testing "Results include last edited information from the `Revision` table" (testing "Results include last edited information from the `Revision` table"
(is (= [{:name "AA"} (is (= [{:name "AA"}
...@@ -505,6 +507,41 @@ ...@@ -505,6 +507,41 @@
:data :data
(map (comp :last_name :last-edit-info))))))))) (map (comp :last_name :last-edit-info)))))))))
(deftest children-sort-clause-test
(testing "Default sort"
(doseq [app-db [:mysql :h2 :postgres]]
(is (= [[:%lower.name :asc]]
(api-coll/children-sort-clause nil app-db)))))
(testing "Sorting by last-edited-at"
(is (= [[(hsql/call :ISNULL :last_edit_timestamp)]
[:last_edit_timestamp :asc]
[:%lower.name :asc]]
(api-coll/children-sort-clause [:last-edited-at :asc] :mysql)))
(is (= [[:last_edit_timestamp :nulls-last]
[:last_edit_timestamp :asc]
[:%lower.name :asc]]
(api-coll/children-sort-clause [:last-edited-at :asc] :postgres))))
(testing "Sorting by last-edited-by"
(is (= [[:last_edit_last_name :nulls-last]
[:last_edit_last_name :asc]
[:last_edit_first_name :nulls-last]
[:last_edit_first_name :asc]
[:%lower.name :asc]]
(api-coll/children-sort-clause [:last-edited-by :asc] :postgres)))
(is (= [[(hsql/call :ISNULL :last_edit_last_name)]
[:last_edit_last_name :asc]
[(hsql/call :ISNULL :last_edit_first_name)]
[:last_edit_first_name :asc]
[:%lower.name :asc]]
(api-coll/children-sort-clause [:last-edited-by :asc] :mysql))))
(testing "Sortinb by model"
(is (= [[:model_ranking :asc]
[:%lower.name :asc]]
(api-coll/children-sort-clause [:model :asc] :postgres)))
(is (= [[:model_ranking :desc]
[:%lower.name :asc]]
(api-coll/children-sort-clause [:model :desc] :mysql)))))
(deftest snippet-collection-items-test (deftest snippet-collection-items-test
(testing "GET /api/collection/:id/items" (testing "GET /api/collection/:id/items"
(testing "Native query snippets should come back when fetching the items in a Collection in the `:snippets` namespace" (testing "Native query snippets should come back when fetching the items in a Collection in the `:snippets` namespace"
......
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