diff --git a/frontend/src/metabase/entities/timeline-events/forms.js b/frontend/src/metabase/entities/timeline-events/forms.js index 02286bcc51639edd5856b7163d4410fc4b1a57cf..bbb7c3b54dfedeb5a8fc00b74b383bb2a66efbfd 100644 --- a/frontend/src/metabase/entities/timeline-events/forms.js +++ b/frontend/src/metabase/entities/timeline-events/forms.js @@ -33,18 +33,26 @@ const createForm = ({ timelines }) => { validate: validate.required(), }, { - name: "timezone", + name: "timeline_id", + title: t`Timeline`, + type: timelines.length > 1 ? "select" : "hidden", + options: timelines.map(t => ({ name: t.name, value: t.id })), + }, + { + name: "source", type: "hidden", }, { - name: "time_matters", + name: "question_id", type: "hidden", }, { - name: "timeline_id", - title: t`Timeline`, - type: timelines.length > 1 ? "select" : "hidden", - options: timelines.map(t => ({ name: t.name, value: t.id })), + name: "timezone", + type: "hidden", + }, + { + name: "time_matters", + type: "hidden", }, ]; }; diff --git a/frontend/src/metabase/timelines/collections/components/NewEventModal/NewEventModal.tsx b/frontend/src/metabase/timelines/collections/components/NewEventModal/NewEventModal.tsx index 2f15a5c5117e565872b90a1080d5c8f623541240..5c711819e5613519b45d1bc45929a84f5597ceac 100644 --- a/frontend/src/metabase/timelines/collections/components/NewEventModal/NewEventModal.tsx +++ b/frontend/src/metabase/timelines/collections/components/NewEventModal/NewEventModal.tsx @@ -34,6 +34,7 @@ const NewEventModal = ({ timeline_id: timeline?.id, icon: timeline ? timeline.icon : getDefaultTimelineIcon(), timezone: getDefaultTimezone(), + source: "collections", }), [timeline], ); diff --git a/frontend/src/metabase/timelines/questions/components/NewEventModal/NewEventModal.tsx b/frontend/src/metabase/timelines/questions/components/NewEventModal/NewEventModal.tsx index 5816a32853d77bcb84a228852a0c077da4938f11..1bc0bb95b7937c882ef35b2610720b75341500f9 100644 --- a/frontend/src/metabase/timelines/questions/components/NewEventModal/NewEventModal.tsx +++ b/frontend/src/metabase/timelines/questions/components/NewEventModal/NewEventModal.tsx @@ -9,6 +9,7 @@ import { Collection, Timeline, TimelineEvent } from "metabase-types/api"; import { ModalBody } from "./NewEventModal.styled"; export interface NewEventModalProps { + cardId?: number; timelines?: Timeline[]; collection: Collection; onSubmit: (values: Partial<TimelineEvent>, collection: Collection) => void; @@ -16,6 +17,7 @@ export interface NewEventModalProps { } const NewEventModal = ({ + cardId, timelines = [], collection, onSubmit, @@ -31,8 +33,10 @@ const NewEventModal = ({ timeline_id: hasTimelines ? defaultTimeline.id : null, icon: hasOneTimeline ? defaultTimeline.icon : getDefaultTimelineIcon(), timezone: getDefaultTimezone(), + source: "question", + question_id: cardId, }), - [defaultTimeline, hasTimelines, hasOneTimeline], + [defaultTimeline, hasTimelines, hasOneTimeline, cardId], ); const handleSubmit = useCallback( diff --git a/frontend/test/__support__/e2e/helpers/e2e-snowplow-helpers.js b/frontend/test/__support__/e2e/helpers/e2e-snowplow-helpers.js index 3db2edb6bf649e28b58d1aeb5df2b5a774be2228..16b30c9d869b4a715d56b08537f3437b9a312b9a 100644 --- a/frontend/test/__support__/e2e/helpers/e2e-snowplow-helpers.js +++ b/frontend/test/__support__/e2e/helpers/e2e-snowplow-helpers.js @@ -5,6 +5,10 @@ const SNOWPLOW_TIMEOUT = 1000; export const describeWithSnowplow = HAS_SNOWPLOW ? describe : describe.skip; +export const enableTracking = () => { + cy.request("PUT", "/api/setting/anon-tracking-enabled", { value: true }); +}; + export const resetSnowplow = () => { sendSnowplowRequest("micro/reset"); }; diff --git a/frontend/test/metabase/scenarios/collections/timelines.cy.spec.js b/frontend/test/metabase/scenarios/collections/timelines.cy.spec.js index c9b38b664c2e55dac9b7ee59591ffb1e0ddf6128..fac56b2d6acebc63b1af15d017311effedfff223 100644 --- a/frontend/test/metabase/scenarios/collections/timelines.cy.spec.js +++ b/frontend/test/metabase/scenarios/collections/timelines.cy.spec.js @@ -1,4 +1,11 @@ -import { restore } from "__support__/e2e/cypress"; +import { + describeWithSnowplow, + enableTracking, + expectGoodSnowplowEvents, + expectNoBadSnowplowEvents, + resetSnowplow, + restore, +} from "__support__/e2e/cypress"; describe("scenarios > collections > timelines", () => { beforeEach(() => { @@ -257,6 +264,38 @@ describe("scenarios > collections > timelines", () => { }); }); +describeWithSnowplow("scenarios > collections > timelines", () => { + beforeEach(() => { + restore(); + resetSnowplow(); + cy.signInAsAdmin(); + enableTracking(); + }); + + afterEach(() => { + expectNoBadSnowplowEvents(); + }); + + it("should send snowplow events when creating a timeline event", () => { + // 1 - new_instance_created + // 2 - pageview + cy.visit("/collection/root"); + + // 3 - pageview + cy.findByLabelText("calendar icon").click(); + + cy.findByText("Add an event").click(); + cy.findByLabelText("Event name").type("Event"); + cy.findByLabelText("Date").type("10/20/2020"); + + // 4 - new_event_created + // 5 - pageview + cy.button("Create").click(); + + expectGoodSnowplowEvents(5); + }); +}); + const openMenu = name => { return cy .findByText(name) diff --git a/snowplow/iglu-client-embedded/schemas/com.metabase/timeline/jsonschema/1-0-0 b/snowplow/iglu-client-embedded/schemas/com.metabase/timeline/jsonschema/1-0-0 new file mode 100644 index 0000000000000000000000000000000000000000..6dba0744e60b7aa183768d7c63d5cffa2dcb6723 --- /dev/null +++ b/snowplow/iglu-client-embedded/schemas/com.metabase/timeline/jsonschema/1-0-0 @@ -0,0 +1,63 @@ +{ + "$schema": "http://iglucentral.com/schemas/com.snowplowanalytics.self-desc/schema/jsonschema/1-0-0#", + "description": "Event timelines", + "self": { + "vendor": "com.metabase", + "name": "timeline", + "format": "jsonschema", + "version": "1-0-0" + }, + "type": "object", + "properties": { + "event": { + "description": "Event name", + "type": "string", + "enum": [ + "new_event_created" + ], + "maxLength": 1024 + }, + "source": { + "description": "String identifying the product location where the event happened", + "type": [ + "string", + "null" + ], + "enum": [ + "question", + "collections", + "api" + ], + "maxLength": 1024 + }, + "question_id": { + "description": "Unique identifier for question, only applicable if created from a question", + "type": [ + "integer", + "null" + ], + "minimum": 0, + "maximum": 2147483647 + }, + "collection_id": { + "description": "Unique identifier for collection, only applicable if created from a collection", + "type": [ + "integer", + "null" + ], + "minimum": 0, + "maximum": 2147483647 + }, + "time_matters": { + "description": "Whether the user has added time resolution to the event", + "type": [ + "boolean", + "null" + ] + } + }, + "required": [ + "event" + ], + "additionalProperties": true +} diff --git a/src/metabase/analytics/snowplow.clj b/src/metabase/analytics/snowplow.clj index 769c567b6ed882465513eeccdfb372979fe8213a..3b5c524fa6339e7db567812b7400bd6a5b7a02bd 100644 --- a/src/metabase/analytics/snowplow.clj +++ b/src/metabase/analytics/snowplow.clj @@ -117,7 +117,8 @@ ::invite "1-0-1" ::dashboard "1-0-0" ::database "1-0-0" - ::instance "1-1-0"}) + ::instance "1-1-0" + ::timeline "1-0-0"}) (defn- context "Common context included in every analytics event" @@ -158,7 +159,8 @@ ::dashboard-created ::dashboard ::question-added-to-dashboard ::dashboard ::database-connection-successful ::database - ::database-connection-failed ::database}) + ::database-connection-failed ::database + ::new-event-created ::timeline}) (defn track-event! "Send a single analytics event to the Snowplow collector, if tracking is enabled for this MB instance and a collector diff --git a/src/metabase/api/timeline_event.clj b/src/metabase/api/timeline_event.clj index 1e85e51acec4243221283f814d7afadb19c660df..89491f84778d4017a2994031bb5019460e0729ee 100644 --- a/src/metabase/api/timeline_event.clj +++ b/src/metabase/api/timeline_event.clj @@ -1,6 +1,7 @@ (ns metabase.api.timeline-event "/api/timeline-event endpoints." (:require [compojure.core :refer [DELETE GET POST PUT]] + [metabase.analytics.snowplow :as snowplow] [metabase.api.common :as api] [metabase.models.collection :as collection] [metabase.models.timeline :refer [Timeline]] @@ -14,7 +15,7 @@ (api/defendpoint POST "/" "Create a new [[TimelineEvent]]." - [:as {{:keys [name description timestamp time_matters timezone icon timeline_id archived] :as body} :body}] + [:as {{:keys [name description timestamp time_matters timezone icon timeline_id source question_id archived] :as body} :body}] {name su/NonBlankString description (s/maybe s/Str) timestamp su/TemporalString @@ -22,21 +23,29 @@ timezone s/Str icon (s/maybe timeline-event/Icons) timeline_id su/IntGreaterThanZero + source (s/maybe timeline-event/Sources) + question_id (s/maybe su/IntGreaterThanZero) archived (s/maybe s/Bool)} ;; deliberately not using api/check-404 so we can have a useful error message. (let [timeline (Timeline timeline_id)] (when-not timeline (throw (ex-info (tru "Timeline with id {0} not found" timeline_id) {:status-code 404}))) - (collection/check-write-perms-for-collection (:collection_id timeline))) - ;; todo: revision system - (let [parsed-timestamp (if (nil? timestamp) - (throw (ex-info (tru "Timestamp cannot be null") {:status-code 400})) - (u.date/parse timestamp)) - evt (merge body {:creator_id api/*current-user-id* - :timestamp parsed-timestamp})] - (db/insert! TimelineEvent (cond-> evt - (nil? icon) (assoc :icon "star"))))) + (collection/check-write-perms-for-collection (:collection_id timeline)) + ;; todo: revision system + (let [parsed (if (nil? timestamp) + (throw (ex-info (tru "Timestamp cannot be null") {:status-code 400})) + (u.date/parse timestamp)) + tl-event (merge (dissoc body :source :question_id) + {:creator_id api/*current-user-id* + :timestamp parsed})] + (snowplow/track-event! ::snowplow/new-event-created + api/*current-user-id* + (cond-> {:time_matters time_matters + :collection_id (:collection_id timeline)} + (boolean source) (assoc :source source) + (boolean question_id) (assoc :question_id question_id))) + (db/insert! TimelineEvent tl-event)))) (api/defendpoint GET "/:id" "Fetch the [[TimelineEvent]] with `id`." diff --git a/src/metabase/models/timeline_event.clj b/src/metabase/models/timeline_event.clj index f905982f681b49ac5169545f34cd5fd38d4a4806..b8e60a23a339e5eb2903c211fc3a792178d6c5ff 100644 --- a/src/metabase/models/timeline_event.clj +++ b/src/metabase/models/timeline_event.clj @@ -15,6 +15,11 @@ "Timeline and TimelineEvent icon string Schema" (s/enum "star" "balloons" "mail" "warning" "bell" "cloud")) +(def Sources + "Timeline Event Source Schema. For Snowplow Events, where the Event is created from is important. + Events are added from one of three sources: `collections`, `questions` (cards in backend code), or directly with an API call. An API call is indicated by having no source key in the `timeline-event` request." + (s/enum "collections" "question")) + ;;;; permissions (defn- perms-objects-set diff --git a/test/metabase/analytics/snowplow_test.clj b/test/metabase/analytics/snowplow_test.clj index 7cb490a4a7e33150ce763ffa653946c6cc904640..04ebdfe8461def1680b651642193cdc216c9b57b 100644 --- a/test/metabase/analytics/snowplow_test.clj +++ b/test/metabase/analytics/snowplow_test.clj @@ -135,6 +135,11 @@ :user-id "1"}] (pop-event-data-and-user-id!))) + (snowplow/track-event! ::snowplow/new-event-created 1 {:source "question", :question_id 1}) + (is (= [{:data {"event" "new_event_created", "source" "question", "question_id" 1} + :user-id "1"}] + (pop-event-data-and-user-id!))) + (testing "Snowplow events are not sent when tracking is disabled" (mt/with-temporary-setting-values [anon-tracking-enabled false] (snowplow/track-event! ::snowplow/new-instance-created)