Skip to content

Commit

Permalink
Support segment and metric metadata -> legacy reference conversion
Browse files Browse the repository at this point in the history
Fixes #36699.
  • Loading branch information
metamben committed Dec 12, 2023
1 parent 3d1962b commit cc45609
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 18 deletions.
14 changes: 11 additions & 3 deletions frontend/src/metabase-lib/fields.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import * as ML from "cljs/metabase.lib.js";
import type { FieldReference } from "metabase-types/api";
import type { Clause, ColumnMetadata, Query } from "./types";
import type {
Clause,
ColumnMetadata,
MetricMetadata,
Query,
SegmentMetadata,
} from "./types";

export function fields(query: Query, stageIndex: number): Clause[] {
return ML.fields(query, stageIndex);
Expand Down Expand Up @@ -61,6 +67,8 @@ export function findVisibleColumnForLegacyRef(
return ML.find_visible_column_for_legacy_ref(query, stageIndex, fieldRef);
}

export function legacyFieldRef(column: ColumnMetadata): FieldReference {
return ML.legacy_field_ref(column);
export function legacyRef(
column: ColumnMetadata | MetricMetadata | SegmentMetadata,
): FieldReference {
return ML.legacy_ref(column);
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function ColumnValuesWidget<T extends string | number>({
const field = metadata.field(fieldId, tableId);
return field ? [field] : [];
}
const fieldRef = Lib.legacyFieldRef(column);
const fieldRef = Lib.legacyRef(column);
const dimension = LegacyDimension.parseMBQL(fieldRef, metadata);
const field = dimension?.field?.();
return field ? [field] : [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ export const addColumnInSettings = (
if (settingIndex >= 0) {
newSettings[settingIndex] = { ...newSettings[settingIndex], enabled: true };
} else {
const fieldRef = Lib.legacyFieldRef(column);
const fieldRef = Lib.legacyRef(column);
newSettings.push({ name, fieldRef, enabled: true });
}

Expand Down
34 changes: 21 additions & 13 deletions src/metabase/lib/js.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,9 @@

(defn ^:export find-filterable-column-for-legacy-ref
"Given a legacy `:field` reference, return the filterable [[ColumnWithOperators]] that best fits it."
[a-query stage-number legacy-ref]
[a-query stage-number a-legacy-ref]
;; [[lib.convert/legacy-ref->pMBQL]] will handle JS -> Clj conversion as needed
(lib.core/find-filterable-column-for-legacy-ref a-query stage-number legacy-ref))
(lib.core/find-filterable-column-for-legacy-ref a-query stage-number a-legacy-ref))

(defn ^:export fields
"Get the current `:fields` in a query. Unlike the lib core version, this will return an empty sequence if `:fields` is
Expand Down Expand Up @@ -560,15 +560,15 @@
(defn ^:export find-visible-column-for-legacy-ref
"Like [[find-visible-column-for-ref]], but takes a legacy MBQL reference instead of a pMBQL one. This is currently
only meant for use with `:field` clauses."
[a-query stage-number legacy-ref]
[a-query stage-number a-legacy-ref]
;; [[lib.convert/legacy-ref->pMBQL]] will handle JS -> Clj conversion as needed
(lib.core/find-visible-column-for-legacy-ref a-query stage-number legacy-ref))
(lib.core/find-visible-column-for-legacy-ref a-query stage-number a-legacy-ref))

(defn ^:export find-column-for-legacy-ref
"Given a sequence of `columns` (column metadatas), return the one that is the best fit for `legacy-ref`."
[a-query stage-number legacy-ref columns]
[a-query stage-number a-legacy-ref columns]
;; [[lib.convert/legacy-ref->pMBQL]] will handle JS -> Clj conversion as needed
(lib.core/find-column-for-legacy-ref a-query stage-number legacy-ref columns))
(lib.core/find-column-for-legacy-ref a-query stage-number a-legacy-ref columns))

;; TODO: Added as an expedient to fix metabase/metabase#32373. Due to the interaction with viz-settings, this issue
;; was difficult to fix entirely within MLv2. Once viz-settings are ported, this function should not be needed, and the
Expand All @@ -591,19 +591,27 @@
(map #(assoc % :selected? true))
to-array)))

(defn ^:export legacy-field-ref
"Given a column metadata from eg. [[fieldable-columns]], return it as a legacy JSON field ref."
(defn- normalize-legacy-ref
[a-ref]
(if (#{:metric :segment} (first a-ref))
(subvec a-ref 0 2)
(update a-ref 2 update-vals #(if (qualified-keyword? %)
(u/qualified-name %)
%))))

(defn ^:export legacy-ref
"Given a column, metric or segment metadata from eg. [[fieldable-columns]] or [[available-segments]],
return it as a legacy JSON field ref.
For compatibility reasons, segment and metric references are always returned without options."
[column]
(-> column
lib.core/ref
lib.convert/->legacy-MBQL
(update 2 update-vals #(if (qualified-keyword? %)
(u/qualified-name %)
%))
normalize-legacy-ref
clj->js))

(defn- legacy-ref->pMBQL [legacy-ref]
(-> legacy-ref
(defn- legacy-ref->pMBQL [a-legacy-ref]
(-> a-legacy-ref
(js->clj :keywordize-keys true)
(update 0 keyword)
lib.convert/->pMBQL))
Expand Down
50 changes: 50 additions & 0 deletions test/metabase/lib/js_test.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,53 @@
(testing "converts :null keyword used by drill-thrus back to JS null"
(is (=? nil
(.-value (lib.js/filter-drill-details {:value :null})))))))

(deftest ^:parallel legacy-ref-test
(let [segment-id 100

segment-definition
{:source-table (meta/id :venues)
:aggregation [[:count]]
:filter [:and
[:> [:field (meta/id :venues :id) nil] [:* [:field (meta/id :venues :price) nil] 11]]
[:contains [:field (meta/id :venues :name) nil] "BBQ" {:case-sensitive true}]]}

metric-id 101

metric-definition
{:source-table (meta/id :venues)
:aggregation [[:sum [:field (meta/id :venues :price) nil]]]
:filter [:= [:field (meta/id :venues :price) nil] 4]}

metadata-provider
(lib.tu/mock-metadata-provider
meta/metadata-provider
{:segments [{: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\"."}]
:metrics [{:id metric-id
:name "Sum of Cans"
:table-id (meta/id :venues)
:definition metric-definition
:description "Number of toucans plus number of pelicans"}]})

query (lib/query metadata-provider (meta/table-metadata :venues))

array-checker #(when (array? %) (js->clj %))
to-legacy-refs (comp array-checker lib.js/legacy-ref)]
(testing "field refs come with options"
(is (= [["field" (meta/id :venues :id) {"base-type" "type/BigInteger"}]
["field" (meta/id :venues :name) {"base-type" "type/Text"}]
["field" (meta/id :venues :category-id) {"base-type" "type/Integer"}]
["field" (meta/id :venues :latitude) {"base-type" "type/Float"}]
["field" (meta/id :venues :longitude) {"base-type" "type/Float"}]
["field" (meta/id :venues :price) {"base-type" "type/Integer"}]]
(->> query lib/returned-columns (map to-legacy-refs)))))
(testing "segment refs come without options"
(is (= [["segment" segment-id]]
(->> query lib/available-segments (map to-legacy-refs)))))
(testing "metric refs come without options"
(is (= [["metric" metric-id]]
(->> query lib/available-metrics (map to-legacy-refs)))))))

0 comments on commit cc45609

Please sign in to comment.