diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc index 61c5ecec05446310b7b8a3a7684115d95721cb75..f7316153694c48cd127ebe94e7c7aa1e4865b81a 100644 --- a/src/metabase/lib/core.cljc +++ b/src/metabase/lib/core.cljc @@ -140,6 +140,9 @@ [lib.filter filter filters + filterable-columns + filterable-column-operators + filter-clause and or not diff --git a/src/metabase/lib/filter.cljc b/src/metabase/lib/filter.cljc index d9a60f08995d04b8d88cd3bcc8014d811c8c7462..d5fbd0a4950422d7198c77bfa1734b6dc45c1f67 100644 --- a/src/metabase/lib/filter.cljc +++ b/src/metabase/lib/filter.cljc @@ -1,22 +1,23 @@ (ns metabase.lib.filter (:refer-clojure :exclude - [filter and or not = < <= > ->> >= not-empty case]) + [filter and or not = < <= > >= not-empty case]) (:require [clojure.string :as str] [metabase.lib.common :as lib.common] [metabase.lib.hierarchy :as lib.hierarchy] + [metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata.calculation :as lib.metadata.calculation] - [metabase.lib.schema] - [metabase.lib.schema.common :as schema.common] + [metabase.lib.schema :as lib.schema] + [metabase.lib.schema.filter :as lib.schema.filter] [metabase.lib.temporal-bucket :as lib.temporal-bucket] + [metabase.lib.types.isa :as lib.types.isa] [metabase.lib.util :as lib.util] [metabase.shared.util.i18n :as i18n] + [metabase.util :as u] [metabase.util.malli :as mu]) #?(:cljs (:require-macros [metabase.lib.filter]))) -(comment metabase.lib.schema/keep-me) - (doseq [tag [:and :or]] (lib.hierarchy/derive tag ::compound)) @@ -157,7 +158,7 @@ new-filter (lib.common/->op-arg query stage-number boolean-expression)] (lib.util/update-query-stage query stage-number update :filters (fnil conj []) new-filter)))) -(mu/defn filters :- [:maybe [:sequential ::schema.common/external-op]] +(mu/defn filters :- [:maybe [:ref ::lib.schema/filters]] "Returns the current filters in stage with `stage-number` of `query`. If `stage-number` is omitted, the last stage is used. Logicaly, the filter attached to the query is the conjunction of the expressions @@ -167,5 +168,159 @@ ([query :- :metabase.lib.schema/query] (filters query nil)) ([query :- :metabase.lib.schema/query stage-number :- [:maybe :int]] - (some->> (clojure.core/not-empty (:filters (lib.util/query-stage query (clojure.core/or stage-number -1)))) - (mapv lib.common/external-op)))) + (clojure.core/not-empty (:filters (lib.util/query-stage query (clojure.core/or stage-number -1)))))) + +(defmethod lib.metadata.calculation/display-name-method :mbql.filter/operator + [_query _stage-number {:keys [display-name]} _display-name-style] + display-name) + +(defmethod lib.metadata.calculation/display-info-method :mbql.filter/operator + [_query _stage-number {:keys [display-name] short-name :short}] + {:short-name (u/qualified-name short-name) + :display-name display-name}) + +(defn- filter-operators + "The list of available filter operators. + The order of operators is relevant for the front end. + There are slight differences between names and ordering for the different base types." + [column] + (let [key-operators [{:short := :display-name (i18n/tru "Is")} + {:short :!= :display-name (i18n/tru "Is not")} + {:short :> :display-name (i18n/tru "Greater than")} + {:short :< :display-name (i18n/tru "Less than")} + {:short :between :display-name (i18n/tru "Between")} + {:short :>= :display-name (i18n/tru "Greater than or equal to")} + {:short :<= :display-name (i18n/tru "Less than or equal to")} + {:short :is-null :display-name (i18n/tru "Is empty")} + {:short :not-null :display-name (i18n/tru "Not empty")}]] + ;; The order of these clauses is important since we want to match the most relevant type + (condp #(lib.types.isa/isa? %2 %1) column + :type/PK + key-operators + + :type/FK + key-operators + + :type/Location + [{:short := :display-name (i18n/tru "Is")} + {:short :!= :display-name (i18n/tru "Is not")} + {:short :is-empty :display-name (i18n/tru "Is empty")} + {:short :not-empty :display-name (i18n/tru "Not empty")} + {:short :contains :display-name (i18n/tru "Contains")} + {:short :does-not-contain :display-name (i18n/tru "Does not contain")} + {:short :starts-with :display-name (i18n/tru "Starts with")} + {:short :ends-with :display-name (i18n/tru "Ends with")}] + + :type/Temporal + [{:short :!= :display-name (i18n/tru "Excludes")} + {:short := :display-name (i18n/tru "Is")} + {:short :< :display-name (i18n/tru "Before")} + {:short :> :display-name (i18n/tru "After")} + {:short :between :display-name (i18n/tru "Between")} + {:short :is-null :display-name (i18n/tru "Is empty")} + {:short :not-null :display-name (i18n/tru "Not empty")}] + + :type/Coordinate + [{:short := :display-name (i18n/tru "Is")} + {:short :!= :display-name (i18n/tru "Is not")} + {:short :inside :display-name (i18n/tru "Inside")} + {:short :> :display-name (i18n/tru "Greater than")} + {:short :< :display-name (i18n/tru "Less than")} + {:short :between :display-name (i18n/tru "Between")} + {:short :>= :display-name (i18n/tru "Greater than or equal to")} + {:short :<= :display-name (i18n/tru "Less than or equal to")}] + + :type/Number + [{:short := :display-name (i18n/tru "Equal to")} + {:short :!= :display-name (i18n/tru "Not equal to")} + {:short :> :display-name (i18n/tru "Greater than")} + {:short :< :display-name (i18n/tru "Less than")} + {:short :between :display-name (i18n/tru "Between")} + {:short :>= :display-name (i18n/tru "Greater than or equal to")} + {:short :<= :display-name (i18n/tru "Less than or equal to")} + {:short :is-null :display-name (i18n/tru "Is empty")} + {:short :not-null :display-name (i18n/tru "Not empty")}] + + :type/Text + [{:short := :display-name (i18n/tru "Is")} + {:short :!= :display-name (i18n/tru "Is not")} + {:short :contains :display-name (i18n/tru "Contains")} + {:short :does-not-contain :display-name (i18n/tru "Does not contain")} + {:short :is-null :display-name (i18n/tru "Is null")} + {:short :not-null :display-name (i18n/tru "Not null")} + {:short :is-empty :display-name (i18n/tru "Is empty")} + {:short :not-empty :display-name (i18n/tru "Not empty")} + {:short :starts-with :display-name (i18n/tru "Starts with")} + {:short :ends-with :display-name (i18n/tru "Ends with")}] + + :type/TextLike + [{:short := :display-name (i18n/tru "Is")} + {:short :!= :display-name (i18n/tru "Is not")} + {:short :is-null :display-name (i18n/tru "Is null")} + {:short :not-null :display-name (i18n/tru "Not null")} + {:short :is-empty :display-name (i18n/tru "Is empty")} + {:short :not-empty :display-name (i18n/tru "Not empty")}] + + :type/Boolean + [{:short := :display-name (i18n/tru "Is")} + {:short :is-null :display-name (i18n/tru "Is empty")} + {:short :not-null :display-name (i18n/tru "Not empty")}] + + ;; default + [{:short := :display-name (i18n/tru "Is")} + {:short :!= :display-name (i18n/tru "Is not")} + {:short :is-null :display-name (i18n/tru "Is null")} + {:short :not-null :display-name (i18n/tru "Not null")}]))) + +(def ^:private ColumnWithOperators + [:merge + lib.metadata/ColumnMetadata + [:map + [:operators {:optional true} [:sequential ::lib.schema.filter/operator]]]]) + +(mu/defn filterable-column-operators :- [:maybe [:sequential ::lib.schema.filter/operator]] + "Returns the operators for which `filterable-column` is applicable." + [filterable-column :- ColumnWithOperators] + (:operators filterable-column)) + +(mu/defn filterable-columns :- [:sequential ColumnWithOperators] + "Get column metadata for all the columns that can be filtered in + the stage number `stage-number` of the query `query` + If `stage-number` is omitted, the last stage is used. + The rules for determining which columns can be broken out by are as follows: + + 1. custom `:expressions` in this stage of the query + + 2. Fields 'exported' by the previous stage of the query, if there is one; + otherwise Fields from the current `:source-table` + + 3. Fields exported by explicit joins + + 4. Fields in Tables that are implicitly joinable." + + ([query :- ::lib.schema/query] + (filterable-columns query -1)) + + ([query :- ::lib.schema/query + stage-number :- :int] + (let [stage (lib.util/query-stage query stage-number) + columns (lib.metadata.calculation/visible-columns query stage-number stage) + with-operators (fn [column] + (when-let [operators (->> (filter-operators column) + (mapv #(assoc % :lib/type :mbql.filter/operator)) + clojure.core/not-empty)] + (assoc column :operators operators)))] + (clojure.core/not-empty + (into [] + (keep with-operators) + columns))))) + +(mu/defn filter-clause + "Returns a standalone filter clause for a `filter-operator`, + a `column`, and arguments." + [filter-operator :- ::lib.schema.filter/operator + column :- lib.metadata/ColumnMetadata + & args] + {:lib/type :lib/external-op + :operator (:short filter-operator) + :args (into [column] args)}) diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs index a5759eade3e5ee18a004e25790ad51bf3522a0b5..c8444c307102866d6e7c4dfae9f181cd6f9aeb4b 100644 --- a/src/metabase/lib/js.cljs +++ b/src/metabase/lib/js.cljs @@ -339,6 +339,26 @@ [aggregation-operator] (to-array (lib.core/aggregation-operator-columns aggregation-operator))) +(defn ^:export filterable-columns + "Get the available filterable columns for the stage with `stage-number` of + the query `a-query`. + If `stage-number` is omitted, the last stage is used." + ([a-query] + (filterable-columns a-query -1)) + ([a-query stage-number] + (to-array (lib.core/filterable-columns a-query stage-number)))) + +(defn ^:export filterable-column-operators + "Returns the operators for which `filterable-column` is applicable." + [filterable-column] + (to-array (lib.core/filterable-column-operators filterable-column))) + +(defn ^:export filter-clause + "Returns a standalone filter clause for a `filter-operator`, + a `column`, and arguments." + [filter-operator column & args] + (apply lib.core/filter-clause filter-operator column args)) + (defn ^:export fields "Get the current `:fields` in a query. Unlike the lib core version, this will return an empty sequence if `:fields` is not specified rather than `nil` for JS-friendliness." diff --git a/src/metabase/lib/schema/expression.cljc b/src/metabase/lib/schema/expression.cljc index 59995074439560c1beffcc14a6aeb043b29c3c92..ef150881173f630627ac38d7cd45fd3db5231a96 100644 --- a/src/metabase/lib/schema/expression.cljc +++ b/src/metabase/lib/schema/expression.cljc @@ -138,9 +138,13 @@ (expression-schema orderable-types "an expression that can be compared with :> or :<")) +(def equality-comparable-types + "Set of base types that can be campared with equality." + #{:type/Boolean :type/Text :type/Number :type/Temporal}) + (mr/def ::equality-comparable [:maybe - (expression-schema #{:type/Boolean :type/Text :type/Number :type/Temporal} + (expression-schema equality-comparable-types "an expression that can appear in := or :!=")]) ;;; any type of expression. diff --git a/src/metabase/lib/schema/filter.cljc b/src/metabase/lib/schema/filter.cljc index 52e026ee4adc892bea420a97ef1720742e668e55..3af76c8eb9ee01216364443b78347610dd271f52 100644 --- a/src/metabase/lib/schema/filter.cljc +++ b/src/metabase/lib/schema/filter.cljc @@ -5,7 +5,8 @@ [metabase.lib.schema.common :as common] [metabase.lib.schema.expression :as expression] [metabase.lib.schema.mbql-clause :as mbql-clause] - [metabase.lib.schema.temporal-bucketing :as temporal-bucketing])) + [metabase.lib.schema.temporal-bucketing :as temporal-bucketing] + [metabase.util.malli.registry :as mr])) (doseq [op [:and :or]] (mbql-clause/define-catn-mbql-clause op :- :type/Boolean @@ -97,3 +98,9 @@ [:= :segment] ::common/options [:or ::common/int-greater-than-zero ::common/non-blank-string]]) + +(mr/def ::operator + [:map + [:lib/type [:= :mbql.filter/operator]] + [:short [:enum := :!= :inside :between :< :> :<= :>= :is-null :not-null :is-empty :not-empty :contains :does-not-contain :starts-with :ends-with]] + [:display-name :string]]) diff --git a/test/metabase/lib/filter_test.cljc b/test/metabase/lib/filter_test.cljc index c546fbe9584d84fdab084e9a98ba69899966dfe8..ea76197d42d962d619caab5bf552a8dfd4b8dc13 100644 --- a/test/metabase/lib/filter_test.cljc +++ b/test/metabase/lib/filter_test.cljc @@ -1,11 +1,11 @@ (ns metabase.lib.filter-test (:require + #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal])) [clojure.test :refer [deftest is testing]] [metabase.lib.core :as lib] [metabase.lib.metadata :as lib.metadata] [metabase.lib.test-metadata :as meta] - [metabase.lib.test-util :as lib.tu] - #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal])))) + [metabase.lib.test-util :as lib.tu])) (defn- test-clause [result-filter f & args] (testing "return a function for later resolution" @@ -14,7 +14,7 @@ (is (=? result-filter (f' {:lib/metadata meta/metadata} -1)))))) -(deftest ^:parallel filter-clause-test +(deftest ^:parallel general-filter-clause-test (let [q1 (lib/query-for-table-name meta/metadata-provider "CATEGORIES") q2 (lib/saved-question-query meta/metadata-provider meta/saved-question) q3 (lib/query-for-table-name meta/metadata-provider "CHECKINS") @@ -126,9 +126,7 @@ (testing "setting a simple filter via the helper function" (let [result-query (lib/filter q1 (lib/between venues-category-id-metadata 42 100)) - result-filter {:operator (-> original-filter first name) - :options (second original-filter) - :args (subvec original-filter 2)}] + result-filter original-filter] (is (=? simple-filtered-query (dissoc result-query :lib/metadata))) (testing "and getting the current filter" @@ -152,23 +150,17 @@ (meta/id :venues :category-id)] 42 100] - first-result-filter {:operator (-> first-filter first name) - :options (second first-filter) - :args (subvec first-filter 2)} + first-result-filter first-filter second-filter [:starts-with {:lib/uuid string?} [:field {:base-type :type/Text, :lib/uuid string?} (meta/id :venues :name)] "prefix"] - second-result-filter {:operator (-> second-filter first name) - :options (second second-filter) - :args (subvec second-filter 2)} + second-result-filter second-filter third-filter [:contains {:lib/uuid string?} [:field {:base-type :type/Text, :lib/uuid string?} (meta/id :venues :name)] "part"] - third-result-filter {:operator (-> third-filter first name) - :options (second third-filter) - :args (subvec third-filter 2)} + third-result-filter third-filter first-add (lib/filter simple-query (lib/between (lib/field "VENUES" "CATEGORY_ID") @@ -212,3 +204,71 @@ {:lib/uuid "953597df-a96d-4453-a57b-665e845abc69"} [:field {:lib/uuid "be28f393-538a-406b-90da-bac5f8ef565e"} (meta/id :venues :name)] "t"]))) )) + +(deftest ^:parallel filterable-columns + (let [query (-> (lib/query-for-table-name meta/metadata-provider "USERS") + (lib/join (-> (lib/join-clause (lib/table (meta/id :checkins)) + [(lib/= + (lib/field "CHECKINS" "USER_ID") + (lib/field "USERS" "ID"))]) + (lib/with-join-fields :all))) + (lib/join (-> (lib/join-clause (lib/table (meta/id :venues)) + [(lib/= + (lib/field "CHECKINS" "VENUE_ID") + (lib/field "VENUES" "ID"))]) + (lib/with-join-fields :all)))) + columns (lib/filterable-columns query) + pk-operators [:= :!= :> :< :between :>= :<= :is-null :not-null] + temporal-operators [:!= := :< :> :between :is-null :not-null] + coordinate-operators [:= :!= :is-empty :not-empty :contains :does-not-contain :starts-with :ends-with] + text-operators [:= :!= :contains :does-not-contain :is-null :not-null :is-empty :not-empty :starts-with :ends-with]] + (is (= ["ID" + "NAME" + "LAST_LOGIN" + "Checkins__ID" + "Checkins__DATE" + "Checkins__USER_ID" + "Checkins__VENUE_ID" + "Venues__ID" + "Venues__NAME" + "Venues__CATEGORY_ID" + "Venues__LATITUDE" + "Venues__LONGITUDE" + "Venues__PRICE" + "CATEGORIES__via__CATEGORY_ID__ID" + "CATEGORIES__via__CATEGORY_ID__NAME"] + (map :lib/desired-column-alias columns))) + (testing "Operators are attached to proper columns" + (is (=? {"ID" pk-operators, + "NAME" text-operators, + "Venues__PRICE" pk-operators + "Venues__LATITUDE" coordinate-operators + "LAST_LOGIN" temporal-operators} + (into {} (for [col columns] + [(:lib/desired-column-alias col) (mapv :short (lib/filterable-column-operators col))]))))) + (testing "Type specific display names" + (let [display-info-by-type-and-op (->> columns + (map (juxt :lib/desired-column-alias + (fn [col] + (->> col + lib/filterable-column-operators + (map (comp (juxt :short-name :display-name) #(lib/display-info query %))) + (into {}))))) + (into {}))] + (is (=? {"ID" {"=" "Is" "is-null" "Is empty" ">" "Greater than"} + "NAME" {"=" "Is" "is-null" "Is null" "is-empty" "Is empty"} + "LAST_LOGIN" {"!=" "Excludes" ">" "After"} + "Venues__PRICE" {"=" "Equal to" "is-null" "Is empty"}} + display-info-by-type-and-op)))))) + +(deftest ^:parallel filter-clause-test + (let [query (-> (lib/query-for-table-name meta/metadata-provider "USERS")) + [first-col] (lib/filterable-columns query) + new-filter (lib/filter-clause + (first (lib/filterable-column-operators first-col)) + first-col + 515)] + (is (=? [[:= {} [:field {} (meta/id :users :id)] 515]] + (-> query + (lib/filter new-filter) + lib/filters))))) diff --git a/test/metabase/lib/remove_replace_test.cljc b/test/metabase/lib/remove_replace_test.cljc index a2979cba35a700c38c04b38df63316542851918e..c8100260a953530f8099e48d7748a4ce9f9a43cc 100644 --- a/test/metabase/lib/remove_replace_test.cljc +++ b/test/metabase/lib/remove_replace_test.cljc @@ -241,7 +241,7 @@ (lib/replace-clause (first filters) (lib/= (lib/field (meta/id :venues :id)) 1))) replaced-filters (lib/filters replaced)] (is (not= filters replaced-filters)) - (is (=? {:operator "=" :args [[:field {} (meta/id :venues :id)] 1]} + (is (=? [:= {} [:field {} (meta/id :venues :id)] 1] (first replaced-filters))) (is (= 2 (count replaced-filters))) (is (= (second filters) (second replaced-filters))))))