diff --git a/dev/src/dev.clj b/dev/src/dev.clj
index 97f2d7dd6490814324eed78ea7e99868f33ca874..12db644614c447cdd7579db0c0191765a033ec5f 100644
--- a/dev/src/dev.clj
+++ b/dev/src/dev.clj
@@ -39,6 +39,7 @@
[clojure.string :as str]
[dev.debug-qp :as debug-qp]
[dev.model-tracking :as model-tracking]
+ [dev.explain :as dev.explain]
[honeysql.core :as hsql]
[malli.dev :as malli-dev]
[metabase.api.common :as api]
@@ -79,6 +80,8 @@
[debug-qp
process-query-debug
pprint-sql]
+ [dev.explain
+ explain-query]
[model-tracking
track!
untrack!
diff --git a/dev/src/dev/explain.clj b/dev/src/dev/explain.clj
new file mode 100644
index 0000000000000000000000000000000000000000..b66668f5b2c410e90f0125ec0434c500c4662c63
--- /dev/null
+++ b/dev/src/dev/explain.clj
@@ -0,0 +1,18 @@
+(ns dev.explain
+ (:require
+ [clojure.string :as str]
+ [honey.sql :as sql]
+ [toucan2.core :as t2]))
+
+(defn explain-query
+ "Explain a sql query or a honeysql query with option to analyze the query."
+ ([queryable]
+ (explain-query queryable false))
+ ([queryable analyze?]
+ (->> (t2/query
+ (str/join
+ " "
+ (remove nil? ["EXPLAIN"
+ (when analyze? "ANALYZE")
+ "(" (if (map? queryable) (first (sql/format queryable {:inline true})) queryable) ")"])))
+ (map #(get % (keyword "query plan"))))))
diff --git a/src/metabase/api/search.clj b/src/metabase/api/search.clj
index 99d20416afc8f0cd097a81ee0f467d8cb292bcbd..167074cc64f26faeee0dff290ff70582451a0db7 100644
--- a/src/metabase/api/search.clj
+++ b/src/metabase/api/search.clj
@@ -139,7 +139,7 @@
query))
(mu/defn ^:private replace-select :- :map
- "Replace a select from query that has alias is `target-alias` with the `with` column, throw an error if
+ "Replace a select from query that has alias is `target-alias` with [`with` `target-alias`] column, throw an error if
can't find the target select.
This works with the assumption that `query` contains a list of select from [[select-clause-for-model]],
@@ -148,15 +148,16 @@
This function then will replace the dummy column with alias is `target-alias` with the `with` column."
[query :- :map
target-alias :- :keyword
- with :- [:or :keyword [:sequential :any]]]
- (let [selects (:select query)
- idx (first (keep-indexed (fn [index item]
- (when (and (coll? item)
- (= (last item) target-alias))
- index))
- selects))]
+ with :- :keyword]
+ (let [selects (:select query)
+ idx (first (keep-indexed (fn [index item]
+ (when (and (coll? item)
+ (= (last item) target-alias))
+ index))
+ selects))
+ with-select [with target-alias]]
(if (some? idx)
- (assoc query :select (m/replace-nth idx with selects))
+ (assoc query :select (m/replace-nth idx with-select selects))
(throw (ex-info "Failed to replace selector" {:status-code 400
:target-alias target-alias
:with with})))))
@@ -165,13 +166,24 @@
[query :- :map
model :- [:enum "card" "dataset" "dashboard" "metric"]]
(-> query
- (replace-select :last_editor_id [:r.user_id :last_editor_id])
- (replace-select :last_edited_at [:r.timestamp :last_edited_at])
+ (replace-select :last_editor_id :r.user_id)
+ (replace-select :last_edited_at :r.timestamp)
(sql.helpers/left-join [:revision :r]
[:and [:= :r.model_id (search.config/column-with-model-alias model :id)]
[:= :r.most_recent true]
[:= :r.model (search.config/search-model->revision-model model)]])))
+(mu/defn ^:private with-moderated-status :- :map
+ [query :- :map
+ model :- [:enum "card" "dataset"]]
+ (-> query
+ (replace-select :moderated_status :mr.status)
+ (sql.helpers/left-join [:moderation_review :mr]
+ [:and
+ [:= :mr.moderated_item_type "card"]
+ [:= :mr.moderated_item_id (search.config/column-with-model-alias model :id)]
+ [:= :mr.most_recent true]])))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Search Queries for each Toucan Model |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -191,7 +203,8 @@
[:= :bookmark.user_id api/*current-user-id*]])
(add-collection-join-and-where-clauses :card.collection_id search-ctx)
(add-card-db-id-clause (:table-db-id search-ctx))
- (with-last-editing-info model)))
+ (with-last-editing-info model)
+ (with-moderated-status model)))
(defmethod search-query-for-model "action"
[model search-ctx]
diff --git a/src/metabase/search/config.clj b/src/metabase/search/config.clj
index 41dbfdfafc3a8062a1f8cc583c395368fc04b40a..ce6ba9cc5780196cbd6f3ebff1695123046c807d 100644
--- a/src/metabase/search/config.clj
+++ b/src/metabase/search/config.clj
@@ -245,7 +245,8 @@
[:table.description :table_description]])
(defmulti columns-for-model
- "The columns that will be returned by the query for `model`, excluding `:model`, which is added automatically."
+ "The columns that will be returned by the query for `model`, excluding `:model`, which is added automatically.
+ This is not guaranteed to be the final list of columns, new columns can be added by calling [[api.search/replace-select]]"
{:arglists '([model])}
(fn [model] model))
@@ -264,17 +265,6 @@
(conj default-columns :collection_id :collection_position :dataset_query :creator_id
[:collection.name :collection_name]
[:collection.authority_level :collection_authority_level]
- [{:select [:status]
- :from [:moderation_review]
- :where [:and
- [:= :moderated_item_type "card"]
- [:= :moderated_item_id :card.id]
- [:= :most_recent true]]
- ;; order by and limit just in case a bug violates the invariant of only one most_recent. We don't want to
- ;; error in this query
- :order-by [[:id :desc]]
- :limit 1}
- :moderated_status]
bookmark-col dashboardcard-count-col))
(defmethod columns-for-model "indexed-entity" [_]
diff --git a/src/metabase/search/filter.clj b/src/metabase/search/filter.clj
index 1fb2301656ae49f31314b12961a980a3b3e2ed39..cb3d2902c66f66577f29f856276fbe64df0186eb 100644
--- a/src/metabase/search/filter.clj
+++ b/src/metabase/search/filter.clj
@@ -189,6 +189,13 @@
[query join-type table]
(->> (get query join-type) (partition 2) (map first) (some #(= % table)) boolean))
+(defn search-model->revision-model
+ "Return the apporpriate revision model given a search model."
+ [model]
+ (case model
+ "dataset" (recur "card")
+ (str/capitalize model)))
+
(doseq [model ["dashboard" "card" "dataset" "metric"]]
(defmethod build-optional-filter-query [:last-edited-by model]
[_filter model query editor-ids]
diff --git a/test/metabase/api/search_test.clj b/test/metabase/api/search_test.clj
index a8136ce24995f039e05802cd3a969444122db8b8..47886a1cb7c82be400d65d9809ce194669ac9ea2 100644
--- a/test/metabase/api/search_test.clj
+++ b/test/metabase/api/search_test.clj
@@ -13,6 +13,7 @@
PermissionsGroupMembership Pulse PulseCard QueryAction Segment Table]]
[metabase.models.collection :as collection]
[metabase.models.model-index :as model-index]
+ [metabase.models.moderation-review :as moderation-review]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.models.revision :as revision]
@@ -371,6 +372,22 @@
(is (= dashboard-count-results
(set (unsorted-search-request-data :rasta :q "dashboard-count")))))))
+(deftest moderated-status-test
+ (let [search-term "moderated-status-test"]
+ (mt/with-temp [:model/Card {card-id :id} {:name "moderated-status-test"}]
+ ;; an item could have multiple moderation-review, and it's current status is defined as
+ ;; moderation-review.most_recent, so we creates multiple moderation review here to make sure
+ ;; test result return the most recent status and don't duplicate the result
+ (doseq [status ["verified" nil "verified"]]
+ (moderation-review/create-review! {:moderated_item_id card-id
+ :moderated_item_type "card"
+ :moderator_id (mt/user->id :crowberto)
+ :status status}))
+ (is (=? [{:id card-id
+ :model "card"
+ :moderated_status "verified"}]
+ (:data (mt/user-http-request :crowberto :get 200 "search" :q search-term)))))))
+
(deftest permissions-test
(testing (str "Ensure that users without perms for the root collection don't get results NOTE: Metrics and segments "
"don't have collections, so they'll be returned")