Add analytics event for combining/extracting columns #41767
Conversation
romeovs
force-pushed
the
41766-combine-shortcut-analytics-event
branch
from
7982cc0 to
83aa02b
Compare
romeovs
changed the title
|
romeovs
force-pushed
the
41766-combine-shortcut-analytics-event
branch
from
952869d to
a83a2be
Compare
romeovs
changed the title
romeovs
force-pushed
the
notebook-expression-shortcuts
branch
from
8acacd3 to
2f331b4
Compare
romeovs
force-pushed
the
41766-combine-shortcut-analytics-event
branch
from
6945e02 to
9414309
Compare
romeovs
force-pushed
the
41766-combine-shortcut-analytics-event
branch
from
9414309 to
62153f7
Compare
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff dd66a69...ddeb3a8. No notifications. |
romeovs
force-pushed
the
41766-combine-shortcut-analytics-event
branch
from
62153f7 to
a8860f1
Compare
| trackSchemaEvent("question", "1-0-4", { | ||
| event: "column_combine_via_shortcut", | ||
| custom_expressions_used: ["concat"], | ||
| }); |
There was a problem hiding this comment.
suggestion/question (not-blocking):
Does it make sense to populate other properties of the event as well here?
It seems like the following could make sense (if we have the info available):
visualization_typedatabase_id- ... I guess we don't have information about the
question_idor something like that, do we?
There was a problem hiding this comment.
I think database_id is the only one that is trivial to find and pass. Is that fine?
There was a problem hiding this comment.
Yeah sure - let's start with that one 🙂 .
Just to check: I had the assumption that the column_combine_via_shortcut would only happen in table vizualizations but that seems not to hold, does it?
There was a problem hiding this comment.
column_combine_via_shortcut happens on the notebook editor, column_combine_via_header happens in the table vizualizations.
romeovs
force-pushed
the
41766-combine-shortcut-analytics-event
branch
from
8ba222a to
4779153
Compare
| import { trackSchemaEvent } from "metabase/lib/analytics"; | ||
| import * as Lib from "metabase-lib"; | ||
|
|
||
| export const trackColumnCombineViaShortcut = (query: Lib.Query) => { |
There was a problem hiding this comment.
This should probably go to metabase/query_builder folder
| }, | ||
| "custom_expressions_used": { | ||
| "description": "A list of names of expression functions and aggregations used", | ||
| "type": "array", |
There was a problem hiding this comment.
It should allow null as well for events that haven't this property:
"type": [
"array",
"null"
]
| import * as Lib from "metabase-lib"; | ||
|
|
||
| export const trackColumnCombineViaShortcut = (query: Lib.Query) => { | ||
| trackSchemaEvent("question", "1-0-4", { |
There was a problem hiding this comment.
question_id is required and cannot be omitted unfortunately. You would also have to default to 0 for unsaved questions.
| import { trackSchemaEvent } from "metabase/lib/analytics"; | ||
| import * as Lib from "metabase-lib"; | ||
|
|
||
| export const trackColumnCombineViaShortcut = (query: Lib.Query) => { |
There was a problem hiding this comment.
Make sure to add e2e tests for this event. Search for describeWithSnowplow for examples.
| @@ -31,3 +32,12 @@ export const trackNotebookNativePreviewShown = (question, isShown) => { | |||
| question_id: question.id() ?? 0, | |||
| }); | |||
| }; | |||
|
|
|||
| export const trackColumnCombineViaShortcut = (query, question) => { | |||
There was a problem hiding this comment.
If you pass question you can use question.query() to know to pass query
Closes #41766
Diffs for the schema change
Description
Adds an analytics events that fires when the custom expression for combining or extracting columns is inserted via the notebook shortcut, or via the chill mode column header.
column_combine_via_shortcutcolumn_combine_via_column_headercolumn_extract_via_shortcutcolumn_extract_via_column_headerAdds an new property
custom_expressions_usedto track the expressions used.I chose to add all of these at once here to avoid having to create new schema versions in 4 PR's.