Skip to content
Snippets Groups Projects
Unverified Commit b8d0579b authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

MLv2: add filter operator short display name (#32064) 

* Refactor: move operator stuff into separate namespace

* Filter operator overhaul

* display-name and long-display-name

* Test fix :wrench:

* Sort namespaces
parent 5d79e417
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/lib/filter.cljc
+ 4
− 152
View file @ b8d0579b
......@@ -7,6 +7,7 @@
[medley.core :as m]
[metabase.lib.common :as lib.common]
[metabase.lib.equality :as lib.equality]
[metabase.lib.filter.operator :as lib.filter.operator]
[metabase.lib.hierarchy :as lib.hierarchy]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
......@@ -16,10 +17,8 @@
[metabase.lib.schema.expression :as lib.schema.expression]
[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])))
......@@ -175,153 +174,6 @@
stage-number :- [:maybe :int]]
(clojure.core/not-empty (:filters (lib.util/query-stage query (clojure.core/or stage-number -1))))))
(defmethod lib.metadata.calculation/display-name-method :operator/filter
[_query _stage-number {:keys [display-name]} _display-name-style]
display-name)
(defmethod lib.metadata.calculation/display-info-method :operator/filter
[_query _stage-number {:keys [display-name default] short-name :short}]
(cond-> {:short-name (u/qualified-name short-name)
:display-name display-name}
default (assoc :default true)))
(defn operator-def
"Get a filter operator definition for the MBQL filter with `tag`, e.g. `:=`. In some cases various tags have alternate
display names used for different situations e.g. for numbers vs temporal values; pass in the
`alternate-display-name-style` to choose a non-default display-name."
([tag]
(operator-def tag nil))
([tag alternate-display-name-style]
{:lib/type :operator/filter
:short tag
:display-name
(clojure.core/case tag
:= (clojure.core/case alternate-display-name-style
:equal-to (i18n/tru "Equal to")
(i18n/tru "Is"))
:!= (clojure.core/case alternate-display-name-style
:not-equal-to (i18n/tru "Not equal to")
:excludes (i18n/tru "Excludes")
(i18n/tru "Is not"))
:> (clojure.core/case alternate-display-name-style
:after (i18n/tru "After")
(i18n/tru "Greater than"))
:< (clojure.core/case alternate-display-name-style
:before (i18n/tru "Before")
(i18n/tru "Less than"))
:between (i18n/tru "Between")
:>= (i18n/tru "Greater than or equal to")
:<= (i18n/tru "Less than or equal to")
:is-null (clojure.core/case alternate-display-name-style
:is-empty (i18n/tru "Is empty")
(i18n/tru "Is null"))
:not-null (clojure.core/case alternate-display-name-style
:not-empty (i18n/tru "Not empty")
(i18n/tru "Not null"))
:is-empty (i18n/tru "Is empty")
:not-empty (i18n/tru "Not empty")
:contains (i18n/tru "Contains")
:does-not-contain (i18n/tru "Does not contain")
:starts-with (i18n/tru "Starts with")
:ends-with (i18n/tru "Ends with")
:inside (i18n/tru "Inside"))}))
(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]
;; this is a function so we don't evaluate it unless we actually return it
(letfn [(key-operators []
[(operator-def :=)
(operator-def :!=)
(operator-def :>)
(operator-def :<)
(operator-def :between)
(operator-def :>=)
(operator-def :<=)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)])]
;; The order of these clauses is important since we want to match the most relevant type
(condp lib.types.isa/field-type? column
:metabase.lib.types.constants/primary_key
(key-operators)
:metabase.lib.types.constants/foreign_key
(key-operators)
:metabase.lib.types.constants/location
[(operator-def :=)
(operator-def :!=)
(operator-def :is-empty)
(operator-def :not-empty)
(operator-def :contains)
(operator-def :does-not-contain)
(operator-def :starts-with)
(operator-def :ends-with)]
:metabase.lib.types.constants/temporal
[(operator-def :!= :excludes)
(operator-def :=)
(operator-def :< :before)
(operator-def :> :after)
(operator-def :between)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)]
:metabase.lib.types.constants/coordinate
[(operator-def :=)
(operator-def :!=)
(operator-def :inside nil)
(operator-def :>)
(operator-def :<)
(operator-def :between)
(operator-def :>=)
(operator-def :<=)]
:metabase.lib.types.constants/number
[(operator-def := :equal-to)
(operator-def :!= :not-equal-to)
(operator-def :>)
(operator-def :<)
(operator-def :between)
(operator-def :>=)
(operator-def :<=)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)]
:metabase.lib.types.constants/string
[(operator-def :=)
(operator-def :!=)
(operator-def :contains)
(operator-def :does-not-contain)
(operator-def :is-null)
(operator-def :not-null)
(operator-def :is-empty)
(operator-def :not-empty)
(operator-def :starts-with)
(operator-def :ends-with)]
:metabase.lib.types.constants/string_like
[(operator-def :=)
(operator-def :!=)
(operator-def :is-null)
(operator-def :not-null)
(operator-def :is-empty)
(operator-def :not-empty)]
:metabase.lib.types.constants/boolean
[(operator-def :=)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)]
;; default
[(operator-def :=)
(operator-def :!=)
(operator-def :is-null)
(operator-def :not-null)])))
(def ^:private ColumnWithOperators
[:merge
lib.metadata/ColumnMetadata
......@@ -356,7 +208,7 @@
(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 (clojure.core/not-empty (filter-operators column))]
(when-let [operators (clojure.core/not-empty (lib.filter.operator/filter-operators column))]
(assoc column :operators operators)))]
(clojure.core/not-empty
(into []
......@@ -388,5 +240,5 @@
ref->col (zipmap (map lib.ref/ref columns) columns)
col-ref (lib.equality/find-closest-matching-ref first-arg (keys ref->col))]
(clojure.core/or (m/find-first #(clojure.core/= (:short %) op)
(filter-operators (ref->col col-ref)))
(operator-def op)))))
(lib.filter.operator/filter-operators (ref->col col-ref)))
(lib.filter.operator/operator-def op)))))
src/metabase/lib/filter/operator.cljc 0 → 100644
+ 201
− 0
View file @ b8d0579b
(ns metabase.lib.filter.operator
(:require
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.schema.common :as lib.schema.common]
[metabase.lib.schema.filter :as lib.schema.filter]
[metabase.lib.types.isa :as lib.types.isa]
[metabase.shared.util.i18n :as i18n]
[metabase.util :as u]
[metabase.util.malli :as mu]))
(mu/defn operator-def :- ::lib.schema.filter/operator
"Get a filter operator definition for the MBQL filter with `tag`, e.g. `:=`. In some cases various tags have alternate
display names used for different situations e.g. for numbers vs temporal values; pass in the
`display-name-style` to choose a non-default display-name."
([tag]
(operator-def tag :default))
([tag display-name-style]
{:lib/type :operator/filter
:short tag
:display-name-variant display-name-style}))
(def ^:private key-operators
[(operator-def :=)
(operator-def :!=)
(operator-def :>)
(operator-def :<)
(operator-def :between)
(operator-def :>=)
(operator-def :<=)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)])
(def ^:private location-operators
[(operator-def :=)
(operator-def :!=)
(operator-def :is-empty)
(operator-def :not-empty)
(operator-def :contains)
(operator-def :does-not-contain)
(operator-def :starts-with)
(operator-def :ends-with)])
(def ^:private temporal-operators
[(operator-def :!= :excludes)
(operator-def :=)
(operator-def :< :before)
(operator-def :> :after)
(operator-def :between)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)])
(def ^:private coordinate-operators
[(operator-def :=)
(operator-def :!=)
(operator-def :inside)
(operator-def :>)
(operator-def :<)
(operator-def :between)
(operator-def :>=)
(operator-def :<=)])
(def ^:private number-operators
[(operator-def := :equal-to)
(operator-def :!= :not-equal-to)
(operator-def :>)
(operator-def :<)
(operator-def :between)
(operator-def :>=)
(operator-def :<=)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)])
(def ^:private text-operators
[(operator-def :=)
(operator-def :!=)
(operator-def :contains)
(operator-def :does-not-contain)
(operator-def :is-null)
(operator-def :not-null)
(operator-def :is-empty)
(operator-def :not-empty)
(operator-def :starts-with)
(operator-def :ends-with)])
(def ^:private text-like-operators
[(operator-def :=)
(operator-def :!=)
(operator-def :is-null)
(operator-def :not-null)
(operator-def :is-empty)
(operator-def :not-empty)])
(def ^:private boolean-operators
[(operator-def :=)
(operator-def :is-null :is-empty)
(operator-def :not-null :not-empty)])
(def ^:private default-operators
[(operator-def :=)
(operator-def :!=)
(operator-def :is-null)
(operator-def :not-null)])
(def join-operators
"Operators that should be listed as options in join conditions."
[(assoc (operator-def :=) :default true)
(operator-def :>)
(operator-def :<)
(operator-def :>=)
(operator-def :<=)
(operator-def :!=)])
(mu/defn filter-operators :- [:sequential ::lib.schema.filter/operator]
"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 :- lib.metadata/ColumnMetadata]
;; The order of these clauses is important since we want to match the most relevant type
(condp lib.types.isa/field-type? column
:metabase.lib.types.constants/primary_key key-operators
:metabase.lib.types.constants/foreign_key key-operators
:metabase.lib.types.constants/location location-operators
:metabase.lib.types.constants/temporal temporal-operators
:metabase.lib.types.constants/coordinate coordinate-operators
:metabase.lib.types.constants/number number-operators
:metabase.lib.types.constants/string text-operators
:metabase.lib.types.constants/string_like text-like-operators
:metabase.lib.types.constants/boolean boolean-operators
;; default
default-operators))
(mu/defn ^:private filter-operator-long-display-name :- ::lib.schema.common/non-blank-string
[tag :- :keyword
display-name-variant :- :keyword]
(case tag
:= (case display-name-variant
:equal-to (i18n/tru "Equal to")
:default (i18n/tru "Is"))
:!= (case display-name-variant
:not-equal-to (i18n/tru "Not equal to")
:excludes (i18n/tru "Excludes")
:default (i18n/tru "Is not"))
:> (case display-name-variant
:after (i18n/tru "After")
:default (i18n/tru "Greater than"))
:< (case display-name-variant
:before (i18n/tru "Before")
:default (i18n/tru "Less than"))
:>= (case display-name-variant
:default (i18n/tru "Greater than or equal to"))
:<= (case display-name-variant
:default (i18n/tru "Less than or equal to"))
:between (case display-name-variant
:default (i18n/tru "Between"))
:is-null (case display-name-variant
:is-empty (i18n/tru "Is empty")
:default (i18n/tru "Is null"))
:not-null (case display-name-variant
:not-empty (i18n/tru "Not empty")
:default (i18n/tru "Not null"))
:is-empty (case display-name-variant
:default (i18n/tru "Is empty"))
:not-empty (case display-name-variant
:default (i18n/tru "Not empty"))
:contains (case display-name-variant
:default (i18n/tru "Contains"))
:does-not-contain (case display-name-variant
:default (i18n/tru "Does not contain"))
:starts-with (case display-name-variant
:default (i18n/tru "Starts with"))
:ends-with (case display-name-variant
:default (i18n/tru "Ends with"))
:inside (case display-name-variant
:default (i18n/tru "Inside"))))
(mu/defn ^:private filter-operator-display-name :- ::lib.schema.common/non-blank-string
[tag :- :keyword
display-name-variant :- :keyword]
(case tag
:= "="
:!= "≠"
:> ">"
:< "<"
:>= "≥"
:<= "≤"
(filter-operator-long-display-name tag display-name-variant)))
(defmethod lib.metadata.calculation/display-name-method :operator/filter
[_query _stage-number {short-name :short, :keys [display-name-variant]} display-name-style]
(case display-name-style
:default (filter-operator-display-name short-name display-name-variant)
:long (filter-operator-long-display-name short-name display-name-variant)))
(defmethod lib.metadata.calculation/display-info-method :operator/filter
[_query _stage-number {short-name :short, :keys [display-name-variant default]}]
(cond-> {:short-name (u/qualified-name short-name)
:display-name (filter-operator-display-name short-name display-name-variant)
:long-display-name (filter-operator-long-display-name short-name display-name-variant)}
default (assoc :default true)))
src/metabase/lib/join.cljc
+ 3
− 10
View file @ b8d0579b
......@@ -8,6 +8,7 @@
[metabase.lib.dispatch :as lib.dispatch]
[metabase.lib.equality :as lib.equality]
[metabase.lib.filter :as lib.filter]
[metabase.lib.filter.operator :as lib.filter.operator]
[metabase.lib.hierarchy :as lib.hierarchy]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
......@@ -559,14 +560,6 @@
(sort-join-condition-columns
(lib.metadata.calculation/visible-columns query stage-number joinable {:include-implicitly-joinable? false}))))
(defn- join-condition-operator-definitions []
[(assoc (lib.filter/operator-def := :equal-to) :default true)
(lib.filter/operator-def :>)
(lib.filter/operator-def :<)
(lib.filter/operator-def :>=)
(lib.filter/operator-def :<=)
(lib.filter/operator-def :!= :not-equal-to)])
(mu/defn join-condition-operators :- [:sequential ::lib.schema.filter/operator]
"Return a sequence of valid filter clause operators that can be used to build a join condition. In the Query Builder
UI, this can be chosen at any point before or after choosing the LHS and RHS. Invalid options are not currently
......@@ -580,7 +573,7 @@
_lhs-column-or-nil :- [:maybe lib.metadata/ColumnMetadata]
_rhs-column-or-nil :- [:maybe lib.metadata/ColumnMetadata]]
;; currently hardcoded to these six operators regardless of LHS and RHS.
(join-condition-operator-definitions)))
lib.filter.operator/join-operators))
(mu/defn ^:private pk-column :- [:maybe lib.metadata/ColumnMetadata]
"Given something `x` (e.g. a Table metadata) find the PK column."
......@@ -614,7 +607,7 @@
joinable]
(when-let [pk-col (pk-column query stage-number joinable)]
(when-let [fk-col (fk-column-for query stage-number pk-col)]
(lib.filter/filter-clause (lib.filter/operator-def := :equal-to) fk-col pk-col)))))
(lib.filter/filter-clause (lib.filter.operator/operator-def :=) fk-col pk-col)))))
(def ^:private Joinable
[:or lib.metadata/TableMetadata lib.metadata/CardMetadata])
......
src/metabase/lib/schema/filter.cljc
+ 12
− 1
View file @ b8d0579b
......@@ -103,4 +103,15 @@
[:map
[:lib/type [:= :operator/filter]]
[:short [:enum := :!= :inside :between :< :> :<= :>= :is-null :not-null :is-empty :not-empty :contains :does-not-contain :starts-with :ends-with]]
[:display-name :string]])
;; this is used for display name and it depends on the arguments to the filter clause itself... e.g.
;;
;; number_a < number_b
;;
;; gets a display name of "less than" for the operator, while
;;
;; timestamp_a < timestamp_b
;;
;; gets a display name of "before" for the operator. We don't want to encode the display name in the `::operator`
;; definition itself, because it forces us to do i18n in the definition itself; it's nicer to have static
;; definitions and only add the display name when we call `display-name` or `display-info`.
[:display-name-variant :keyword]])
test/metabase/lib/filter_test.cljc
+ 13
− 5
View file @ b8d0579b
......@@ -256,13 +256,21 @@
(fn [col]
(->> col
lib/filterable-column-operators
(map (comp (juxt :short-name :display-name) #(lib/display-info query %)))
(map (comp (juxt :short-name identity)
#(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"}}
(is (=? {"ID" {"=" {:display-name "=", :long-display-name "Is"}
"is-null" {:display-name "Is empty", :long-display-name "Is empty"}
">" {:display-name ">", :long-display-name "Greater than"}
">=" {:display-name "≥", :long-display-name "Greater than or equal to"},}
"NAME" {"=" {:display-name "=", :long-display-name "Is"}
"is-null" {:display-name "Is null", :long-display-name "Is null"}
"is-empty" {:display-name "Is empty", :long-display-name "Is empty"}}
"LAST_LOGIN" {"!=" {:display-name "≠", :long-display-name "Excludes"}
">" {:display-name ">", :long-display-name "After"}}
"Venues__PRICE" {"=" {:display-name "=", :long-display-name "Equal to"}
"is-null" {:display-name "Is empty", :long-display-name "Is empty"}}}
display-info-by-type-and-op))))))
(deftest ^:parallel filter-clause-test
......
test/metabase/lib/join_test.cljc
+ 8
− 0
View file @ b8d0579b
......@@ -495,6 +495,14 @@
{:short :<=}
{:short :!=}]
(lib/join-condition-operators lib.tu/venues-query lhs rhs)))
(is (=? [{:short-name "=", :display-name "=", :long-display-name "Is"}
{:short-name ">", :display-name ">", :long-display-name "Greater than"}
{:short-name "<", :display-name "<", :long-display-name "Less than"}
{:short-name ">=", :display-name "≥", :long-display-name "Greater than or equal to"}
{:short-name "<=", :display-name "≤", :long-display-name "Less than or equal to"}
{:short-name "!=", :display-name "≠", :long-display-name "Is not"}]
(map (partial lib/display-info query)
(lib/join-condition-operators lib.tu/venues-query lhs rhs))))
(is (= (lib/join-condition-operators lib.tu/venues-query lhs rhs)
(lib/join-condition-operators lib.tu/venues-query -1 lhs rhs))))
(testing `lib/display-info
......
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