From 50ef456400d941e24a26f2aaf46a8310b6f8dfe4 Mon Sep 17 00:00:00 2001 From: Romeo Van Snick <romeo@romeovansnick.be> Date: Thu, 2 May 2024 21:14:09 +0200 Subject: [PATCH] Add analytics event for extract column in chill mode (#41774) * Add trackColumnExtractViaHeader helper * Add Lib.extract wrapper * Track column extractions via header * Fix analytics event name * Add tests for extract action analytics * Set up extractions metadata helper * Use functionsUseByExtraction in analytics * Add custom type for ExtractionTag * Use ExtractionTag in analytics * [MBQL lib] Add `Lib.columnExtractDrillExtractions` This gets the extractions from a column-extract drill, which is needed in the UI. * Add extractionExpression wrapper * Add extractionsForDrill helper * Fix type signature of extractionExpression * Use expressionParts for getting expression function * Walk expression to get functions used * Simplify ColumnExtractionTag * Fix expected expression * Be more explicit with what the guard does --------- Co-authored-by: Braden Shepherdson <braden@metabase.com> --- .../column_extract_drill.cy.spec.js | 37 +++++++++- frontend/src/metabase-lib/extractions.ts | 68 +++++++++++++++++-- frontend/src/metabase-lib/types.ts | 3 +- frontend/src/metabase/querying/analytics.ts | 21 ++++++ .../column-extract-drill.tsx | 26 +++++-- src/metabase/lib/core.cljc | 3 + .../lib/drill_thru/column_extract.cljc | 6 ++ src/metabase/lib/js.cljs | 9 +++ .../lib/drill_thru/column_extract_test.cljc | 14 +++- 9 files changed, 176 insertions(+), 11 deletions(-) create mode 100644 frontend/src/metabase/querying/analytics.ts diff --git a/e2e/test/scenarios/visualizations-tabular/drillthroughs/column_extract_drill.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/drillthroughs/column_extract_drill.cy.spec.js index 10da5566eb0..448def481d6 100644 --- a/e2e/test/scenarios/visualizations-tabular/drillthroughs/column_extract_drill.cy.spec.js +++ b/e2e/test/scenarios/visualizations-tabular/drillthroughs/column_extract_drill.cy.spec.js @@ -1,13 +1,18 @@ import _ from "underscore"; +import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; import { ORDERS_QUESTION_ID } from "e2e/support/cypress_sample_instance_data"; import { + describeWithSnowplow, enterCustomColumnDetails, + expectGoodSnowplowEvent, + expectNoBadSnowplowEvents, getNotebookStep, openNotebook, openOrdersTable, popover, + resetSnowplow, restore, visitQuestion, visualize, @@ -65,7 +70,7 @@ const DATE_QUESTION = { }, }; -describe("extract action", () => { +describeWithSnowplow("extract action", () => { beforeEach(() => { restore(); cy.signInAsAdmin(); @@ -266,3 +271,33 @@ function extractColumnAndCheck({ function checkColumnIndex({ column, columnIndex }) { cy.findAllByRole("columnheader").eq(columnIndex).should("have.text", column); } + +describeWithSnowplow("extract action", () => { + beforeEach(() => { + restore(); + resetSnowplow(); + cy.signInAsAdmin(); + }); + + afterEach(() => { + expectNoBadSnowplowEvents(); + }); + + it("should create a snowplow event for the column extraction action", () => { + openOrdersTable({ limit: 1 }); + + cy.wait(1); + + extractColumnAndCheck({ + column: "Created At", + option: "Year", + value: "2,025", + }); + + expectGoodSnowplowEvent({ + event: "column_extract_via_column_header", + custom_expressions_used: ["get-year"], + database_id: SAMPLE_DB_ID, + }); + }); +}); diff --git a/frontend/src/metabase-lib/extractions.ts b/frontend/src/metabase-lib/extractions.ts index 5c1500b9126..493cd518dfd 100644 --- a/frontend/src/metabase-lib/extractions.ts +++ b/frontend/src/metabase-lib/extractions.ts @@ -1,6 +1,34 @@ import * as ML from "cljs/metabase.lib.js"; -import type { ColumnExtraction, ColumnMetadata, Query } from "./types"; +import { expressionParts } from "./expression"; +import type { + ColumnExtraction, + ColumnMetadata, + Query, + DrillThru, + ExpressionParts, + ExpressionArg, +} from "./types"; + +export function extract( + query: Query, + stageIndex: number, + extraction: ColumnExtraction, +): Query { + return ML.extract(query, stageIndex, extraction); +} + +export function extractionExpression( + query: Query, + stageIndex: number, + extraction: ColumnExtraction, +) { + return ML.extraction_expression(query, stageIndex, extraction); +} + +export function extractionsForDrill(drill: DrillThru): ColumnExtraction[] { + return ML.column_extract_drill_extractions(drill); +} export function columnExtractions( query: Query, @@ -9,10 +37,42 @@ export function columnExtractions( return ML.column_extractions(query, column); } -export function extract( +export type ColumnExtractionTag = + | "hour-of-day" + | "day-of-month" + | "day-of-week" + | "month-of-year" + | "quarter-of-year" + | "year" + | "domain" + | "host" + | "subdomain"; + +/** + * Return the functions used by a specific column extraction. + */ +export function functionsUsedByExtraction( query: Query, stageIndex: number, extraction: ColumnExtraction, -): Query { - return ML.extract(query, stageIndex, extraction); +): string[] { + const expression = extractionExpression(query, stageIndex, extraction); + const parts = expressionParts(query, stageIndex, expression); + return walk(parts); +} + +function walk(parts: ExpressionParts): string[] { + const res: string[] = [parts.operator]; + parts.args.forEach(arg => { + if (isExpressionParts(arg)) { + res.push(...walk(arg)); + } + }); + return res; +} + +export function isExpressionParts( + arg: ExpressionParts | ExpressionArg, +): arg is ExpressionParts { + return arg != null && typeof arg === "object" && "operator" in arg; } diff --git a/frontend/src/metabase-lib/types.ts b/frontend/src/metabase-lib/types.ts index a6e03353532..c9fceec95d2 100644 --- a/frontend/src/metabase-lib/types.ts +++ b/frontend/src/metabase-lib/types.ts @@ -19,6 +19,7 @@ import type { RELATIVE_DATE_BUCKETS, TIME_FILTER_OPERATORS, } from "./constants"; +import type { ColumnExtractionTag } from "./extractions"; /** * An "opaque type": this technique gives us a way to pass around opaque CLJS values that TS will track for us, @@ -454,7 +455,7 @@ export type ColumnExtraction = unknown & { }; export type ColumnExtractionInfo = { - tag: string; + tag: ColumnExtractionTag; displayName: string; }; diff --git a/frontend/src/metabase/querying/analytics.ts b/frontend/src/metabase/querying/analytics.ts new file mode 100644 index 00000000000..3acde7209f4 --- /dev/null +++ b/frontend/src/metabase/querying/analytics.ts @@ -0,0 +1,21 @@ +import { trackSchemaEvent } from "metabase/lib/analytics"; +import * as Lib from "metabase-lib"; +import type Question from "metabase-lib/v1/Question"; + +export const trackColumnExtractViaHeader = ( + query: Lib.Query, + stageIndex: number, + extraction: Lib.ColumnExtraction, + question?: Question, +) => { + trackSchemaEvent("question", "1-0-4", { + event: "column_extract_via_column_header", + custom_expressions_used: Lib.functionsUsedByExtraction( + query, + stageIndex, + extraction, + ), + database_id: Lib.databaseID(query), + question_id: question?.id() ?? 0, + }); +}; diff --git a/frontend/src/metabase/querying/utils/drills/column-extract-drill/column-extract-drill.tsx b/frontend/src/metabase/querying/utils/drills/column-extract-drill/column-extract-drill.tsx index a73649077c5..3bbb19477a4 100644 --- a/frontend/src/metabase/querying/utils/drills/column-extract-drill/column-extract-drill.tsx +++ b/frontend/src/metabase/querying/utils/drills/column-extract-drill/column-extract-drill.tsx @@ -1,35 +1,53 @@ +import { trackColumnExtractViaHeader } from "metabase/querying/analytics"; import { ClickActionsView } from "metabase/visualizations/components/ClickActions"; import type { ClickActionPopoverProps, Drill, RegularClickAction, } from "metabase/visualizations/types/click-actions"; -import type * as Lib from "metabase-lib"; +import * as Lib from "metabase-lib"; export const columnExtractDrill: Drill<Lib.ColumnExtractDrillThruInfo> = ({ + query, + stageIndex, + question, drill, drillInfo, clicked, applyDrill, }) => { const DrillPopover = ({ onClose, onClick }: ClickActionPopoverProps) => { + const extractions = Lib.extractionsForDrill(drill); + const actions: RegularClickAction[] = drillInfo.extractions.map( - extraction => ({ + (extraction, index) => ({ name: `extract.${extraction.displayName}`, title: extraction.displayName, subTitle: getExample(extraction), section: "extract-popover", buttonType: "horizontal", question: () => applyDrill(drill, extraction.tag), - extra: () => ({ settingsSyncOptions: { column: clicked.column } }), + extra: () => ({ + extraction: extractions[index], + settingsSyncOptions: { column: clicked.column }, + }), }), ); + function handleClick(action: RegularClickAction) { + const { extraction } = action.extra?.() as { + extraction: Lib.ColumnExtraction; + }; + + trackColumnExtractViaHeader(query, stageIndex, extraction, question); + onClick(action); + } + return ( <ClickActionsView clickActions={actions} close={onClose} - onClick={onClick} + onClick={handleClick} /> ); }; diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc index 8d134a34e9e..d0c8566802b 100644 --- a/src/metabase/lib/core.cljc +++ b/src/metabase/lib/core.cljc @@ -13,6 +13,7 @@ [metabase.lib.convert :as lib.convert] [metabase.lib.database :as lib.database] [metabase.lib.drill-thru :as lib.drill-thru] + [metabase.lib.drill-thru.column-extract :as lib.drill-thru.column-extract] [metabase.lib.drill-thru.pivot :as lib.drill-thru.pivot] [metabase.lib.equality :as lib.equality] [metabase.lib.expression :as lib.expression] @@ -120,6 +121,8 @@ [lib.drill-thru available-drill-thrus drill-thru] + [lib.drill-thru.column-extract + extractions-for-drill] [lib.drill-thru.pivot pivot-columns-for-type pivot-types] diff --git a/src/metabase/lib/drill_thru/column_extract.cljc b/src/metabase/lib/drill_thru/column_extract.cljc index ad073ee1d00..f39918360f2 100644 --- a/src/metabase/lib/drill_thru/column_extract.cljc +++ b/src/metabase/lib/drill_thru/column_extract.cljc @@ -20,6 +20,7 @@ [metabase.lib.metadata.calculation :as lib.metadata.calculation] [metabase.lib.schema :as lib.schema] [metabase.lib.schema.drill-thru :as lib.schema.drill-thru] + [metabase.lib.schema.extraction :as lib.schema.extraction] [metabase.lib.types.isa :as lib.types.isa] [metabase.shared.util.i18n :as i18n] [metabase.util.malli :as mu])) @@ -47,6 +48,11 @@ (lib.drill-thru.column-filter/prepare-query-for-drill-addition query stage-number column column-ref :expression))))) +(mu/defn extractions-for-drill :- [:sequential ::lib.schema.extraction/extraction] + "Returns the extractions from a given drill." + [drill :- ::lib.schema.drill-thru/drill-thru.column-extract] + (:extractions drill)) + (defmethod lib.drill-thru.common/drill-thru-info-method :drill-thru/column-extract [query stage-number drill] (-> drill diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs index f186eb9fb70..0487dd5591b 100644 --- a/src/metabase/lib/js.cljs +++ b/src/metabase/lib/js.cljs @@ -1918,6 +1918,15 @@ "stageIndex" stage-number "value" (if (= value :null) nil value)}) +(defn ^:export column-extract-drill-extractions + "Returns a JS array of the possible column *extractions* offered by `column-extract-drill`. + + The extractions are opaque values of the same type as are returned by [[column-extractions]]. + + > **Code health:** Single use. This is only here to support UI for column extract drills, and should not be reused." + [column-extract-drill] + (to-array (lib.core/extractions-for-drill column-extract-drill))) + (defn ^:export pivot-types "Returns a JS array of pivot types that are available in `a-drill-thru`, which must be a `pivot` drill-thru. diff --git a/test/metabase/lib/drill_thru/column_extract_test.cljc b/test/metabase/lib/drill_thru/column_extract_test.cljc index c8b32f13822..9db7f9c8dc0 100644 --- a/test/metabase/lib/drill_thru/column_extract_test.cljc +++ b/test/metabase/lib/drill_thru/column_extract_test.cljc @@ -1,8 +1,9 @@ (ns metabase.lib.drill-thru.column-extract-test "See also [[metabase.query-processor-test.drill-thru-e2e-test/quick-filter-on-bucketed-date-test]]" (:require - [clojure.test :refer [deftest testing]] + [clojure.test :refer [deftest is testing]] [metabase.lib.core :as lib] + [metabase.lib.drill-thru.column-extract :as lib.drill-thru.column-extract] [metabase.lib.drill-thru.test-util :as lib.drill-thru.tu] [metabase.lib.drill-thru.test-util.canned :as canned] [metabase.lib.metadata :as lib.metadata] @@ -338,3 +339,14 @@ :query-type :unaggregated :column-name "EMAIL" :custom-query query-no-regex})))) + +(deftest ^:parallel extractions-for-drill-test + (let [drill (lib.drill-thru.tu/test-returns-drill + {:click-type :header + :query-type :unaggregated + :column-name "CREATED_AT" + :drill-type :drill-thru/column-extract + :expected {:type :drill-thru/column-extract + :extractions datetime-extraction-units}})] + (is (=? datetime-extraction-units + (lib.drill-thru.column-extract/extractions-for-drill drill))))) -- GitLab