diff --git a/frontend/src/metabase-lib/segments.ts b/frontend/src/metabase-lib/segments.ts index c7ccaca23c5aad389be6ad138c8da858e66265be..6d425c7aabd1c5a2638ca50e7730ae6f11856075 100644 --- a/frontend/src/metabase-lib/segments.ts +++ b/frontend/src/metabase-lib/segments.ts @@ -2,6 +2,9 @@ import * as ML from "cljs/metabase.lib.js"; import type { SegmentMetadata, Query } from "./types"; -export function availableSegments(query: Query): SegmentMetadata[] { - return ML.available_segments(query); +export function availableSegments( + query: Query, + stageIndex: number, +): SegmentMetadata[] { + return ML.available_segments(query, stageIndex); } diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs index 733635b9df634bdf7c4d41a2d1edc0a592d18a90..460db79479fde7d1a35fc03c12ae70a46abbe3db 100644 --- a/src/metabase/lib/js.cljs +++ b/src/metabase/lib/js.cljs @@ -795,8 +795,8 @@ (defn ^:export available-segments "Get a list of Segments that you may consider using as filters for a query. Returns JS array of opaque Segment metadata objects." - [a-query] - (to-array (lib.core/available-segments a-query))) + [a-query stage-number] + (to-array (lib.core/available-segments a-query stage-number))) (defn ^:export available-metrics "Get a list of Metrics that you may consider using as aggregations for a query. Returns JS array of opaque Metric diff --git a/src/metabase/lib/segment.cljc b/src/metabase/lib/segment.cljc index 8175e35286a57bd9632bcc34081147b0b1a27c40..31f99e5c4e0ac46a5a90e0810a61a3c528ec5e03 100644 --- a/src/metabase/lib/segment.cljc +++ b/src/metabase/lib/segment.cljc @@ -1,13 +1,13 @@ (ns metabase.lib.segment "A Segment is a saved MBQL query stage snippet with `:filter`. Segments are always boolean expressions." (:require + [metabase.lib.filter :as lib.filter] [metabase.lib.metadata :as lib.metadata] [metabase.lib.metadata.calculation :as lib.metadata.calculation] [metabase.lib.metadata.protocols :as lib.metadata.protocols] [metabase.lib.options :as lib.options] [metabase.lib.ref :as lib.ref] [metabase.lib.schema :as lib.schema] - [metabase.lib.schema.id :as lib.schema.id] [metabase.lib.util :as lib.util] [metabase.shared.util.i18n :as i18n] [metabase.util.malli :as mu])) @@ -44,24 +44,13 @@ (lib.metadata.calculation/display-name query stage-number segment-metadata style))) (fallback-display-name))) -(mu/defn ^:private filtering-by-segment? :- :boolean - "Whether a given stage of a query currently includes a `:segment` ref clause in its filters." - [query :- ::lib.schema/query - stage-number :- :int - segment-id :- ::lib.schema.id/segment] - (boolean - (some (fn [[tag _opts id]] - (and (= tag :segment) - (= id segment-id))) - (:filters (lib.util/query-stage query stage-number))))) - (defmethod lib.metadata.calculation/display-info-method :metadata/segment - [query stage-number {:keys [id description], :as segment-metadata}] + [query stage-number {:keys [description filter-positions], :as segment-metadata}] (let [default-display-info-method (get-method lib.metadata.calculation/display-info-method :default) default-display-info (default-display-info-method query stage-number segment-metadata)] (cond-> default-display-info - description (assoc :description description) - (filtering-by-segment? query stage-number id) (assoc :selected true)))) + description (assoc :description description) + filter-positions (assoc :filter-positions filter-positions)))) (defmethod lib.metadata.calculation/display-info-method :segment [query stage-number [_tag _opts segment-id-or-name]] @@ -74,6 +63,25 @@ (mu/defn available-segments :- [:maybe [:sequential {:min 1} lib.metadata/SegmentMetadata]] "Get a list of Segments that you may consider using as filter for a query. Only Segments that have the same `table-id` as the `source-table` for this query will be suggested." - [query :- ::lib.schema/query] - (when-let [source-table-id (lib.util/source-table-id query)] - (not-empty (lib.metadata.protocols/segments (lib.metadata/->metadata-provider query) source-table-id)))) + ([query] + (available-segments query -1)) + ([query :- ::lib.schema/query + stage-number :- :int] + (when-let [source-table-id (lib.util/source-table-id query)] + (let [segments (lib.metadata.protocols/segments (lib.metadata/->metadata-provider query) source-table-id) + segment-filters (into {} + (keep-indexed (fn [index filter-clause] + (when (lib.util/clause-of-type? filter-clause :segment) + [(get filter-clause 2) index]))) + (lib.filter/filters query stage-number))] + (cond + (empty? segments) nil + (empty? segment-filters) (vec segments) + :else (mapv (fn [segment-metadata] + (let [filter-pos (-> segment-metadata :id segment-filters)] + (cond-> segment-metadata + ;; even though at most one filter can reference a given segment + ;; we use plural in order to keep the interface used with + ;; plain filters referencing columns + filter-pos (assoc :filter-positions [filter-pos])))) + segments)))))) diff --git a/test/metabase/lib/segment_test.cljc b/test/metabase/lib/segment_test.cljc index 4a4501d69a03ed0fba7ec968b7b731482552f516..ffaf2cb0188625b5d027b7c9280d40c00657496f 100644 --- a/test/metabase/lib/segment_test.cljc +++ b/test/metabase/lib/segment_test.cljc @@ -48,18 +48,11 @@ :display-name "PriceID-BBQ", :long-display-name "PriceID-BBQ", :effective-type :type/Boolean, - :description "The ID is greater than 11 times the price and the name contains \"BBQ\".", - :selected true} + :description "The ID is greater than 11 times the price and the name contains \"BBQ\"."} (lib.metadata.calculation/display-info query-with-segment segment)) segment-clause segment-metadata)) -(deftest ^:parallel display-info-unselected-segment-test - (testing "Include `:selected false` in display info for Segments not in aggregations" - (are [segment] (not (:selected (lib.metadata.calculation/display-info lib.tu/venues-query segment))) - segment-clause - segment-metadata))) - (deftest ^:parallel unknown-display-info-test (is (=? {:effective-type :type/Boolean :display-name "[Unknown Segment]" @@ -75,6 +68,25 @@ :definition segment-definition :description "The ID is greater than 11 times the price and the name contains \"BBQ\"."}] (lib/available-segments (lib/query metadata-provider (meta/table-metadata :venues)))))) + (testing "Should return filter-positions" + (let [query (-> (lib/query metadata-provider (meta/table-metadata :venues)) + (lib/filter segment-clause)) + available-segments (lib/available-segments query)] + (is (=? [{:lib/type :metadata/segment + :id segment-id + :name "PriceID-BBQ" + :table-id (meta/id :venues) + :definition segment-definition + :description "The ID is greater than 11 times the price and the name contains \"BBQ\"." + :filter-positions [0]}] + available-segments)) + (is (=? [{:name "priceid_bbq", + :display-name "PriceID-BBQ", + :long-display-name "PriceID-BBQ", + :effective-type :type/Boolean, + :description "The ID is greater than 11 times the price and the name contains \"BBQ\".", + :filter-positions [0]}] + (map #(lib/display-info query %) available-segments))))) (testing "query with different Table -- don't return Segments" (is (nil? (lib/available-segments (lib/query metadata-provider (meta/table-metadata :orders))))))) @@ -100,7 +112,6 @@ :display-name "PriceID-BBQ", :long-display-name "PriceID-BBQ", :effective-type :type/Boolean, - :description "The ID is greater than 11 times the price and the name contains \"BBQ\".", - :selected true}] + :description "The ID is greater than 11 times the price and the name contains \"BBQ\"."}] (map (partial lib/display-info query') (lib/filters query'))))))))))