diff --git a/snowplow/iglu-client-embedded/schemas/com.metabase/invite/jsonschema/1-0-1 b/snowplow/iglu-client-embedded/schemas/com.metabase/invite/jsonschema/1-0-1 new file mode 100644 index 0000000000000000000000000000000000000000..8e90774f047591e6233128149820a016da572b8f --- /dev/null +++ b/snowplow/iglu-client-embedded/schemas/com.metabase/invite/jsonschema/1-0-1 @@ -0,0 +1,44 @@ +{ + "$schema": "http://iglucentral.com/schemas/com.snowplowanalytics.self-desc/schema/jsonschema/1-0-0#", + "description": "New user invite", + "self": { + "vendor": "com.metabase", + "name": "invite", + "format": "jsonschema", + "version": "1-0-1" + }, + "type": "object", + "properties": { + "event": { + "description": "Event name", + "type": "string", + "enum": [ + "invite_sent" + ], + "maxLength": 1024 + }, + "invited_user_id": { + "description": "User ID of the user who was invited", + "type": "integer", + "minimum": 0, + "maximum": 2147483647 + }, + "source": { + "description": "String identifying the product location where the event took place", + "type": [ + "string", + "null" + ], + "enum": [ + "setup", + "admin" + ], + "maxLength": 1024 + } + }, + "required": [ + "event", + "invited_user_id" + ], + "additionalProperties": true +} diff --git a/src/metabase/analytics/snowplow.clj b/src/metabase/analytics/snowplow.clj index f8624d8817218bbaf62792aa7dc7b1c9e34e1290..c84a9ff57d10b574d1ac1def4a31b80159d5d5e2 100644 --- a/src/metabase/analytics/snowplow.clj +++ b/src/metabase/analytics/snowplow.clj @@ -85,7 +85,7 @@ "The most recent version for each event schema. This should be updated whenever a new version of a schema is added to SnowcatCloud, at the same time that the data sent to the collector is updated." {::account "1-0-0" - ::invite "1-0-0" + ::invite "1-0-1" ::dashboard "1-0-0" ::database "1-0-0" ::instance "1-1-0"}) diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj index 11f351c69e87ab56f3610960ccb8090d2361449e..adf4c8012ff8ddd0512acf1acaa38adb91d24fd6 100644 --- a/src/metabase/api/setup.clj +++ b/src/metabase/api/setup.clj @@ -63,7 +63,9 @@ (if-not (email/email-configured?) (log/error (trs "Could not invite user because email is not configured.")) (u/prog1 (user/create-and-invite-user! user invitor true) - (user/set-permissions-groups! <> [(group/all-users) (group/admin)]))))) + (user/set-permissions-groups! <> [(group/all-users) (group/admin)]) + (snowplow/track-event! ::snowplow/invite-sent api/*current-user-id* {:invited-user-id (u/the-id <>) + :source "setup"}))))) (defn- setup-create-database! "Create a new Database. Returns newly created Database." diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj index 462e06a75f3bc675265e5cddcb011f82298adcbf..c256024af68eefc2e362dbf83e040a87a373bb93 100644 --- a/src/metabase/api/user.clj +++ b/src/metabase/api/user.clj @@ -184,7 +184,8 @@ @api/*current-user* false))] (maybe-set-user-permissions-groups! new-user-id group_ids) - (snowplow/track-event! ::snowplow/invite-sent api/*current-user-id* {:invited-user-id new-user-id}) + (snowplow/track-event! ::snowplow/invite-sent api/*current-user-id* {:invited-user-id new-user-id + :source "admin"}) (-> (fetch-user :id new-user-id) (hydrate :group_ids))))) diff --git a/test/metabase/analytics/snowplow_test.clj b/test/metabase/analytics/snowplow_test.clj index 4a531c28507b46c8b63117e8a535002b9c044e77..220f3541e97884df0a73daf5cb1e616062a17b23 100644 --- a/test/metabase/analytics/snowplow_test.clj +++ b/test/metabase/analytics/snowplow_test.clj @@ -13,7 +13,7 @@ (use-fixtures :once (fixtures/initialize :db)) -(def ^:dynamic ^:private *snowplow-collector* +(def ^:dynamic *snowplow-collector* "Fake Snowplow collector" (atom [])) @@ -25,7 +25,7 @@ (defn- fake-track-event-impl! "A function that can be used in place of track-event-impl! which pulls and decodes the payload, context and subject ID from an event and adds it to the in-memory [[*snowplow-collector*]] queue." - [_ event] + [collector _ event] (let [payload (-> event .getPayload .getMap normalize-map) ;; Don't normalize keys in [[properties]] so that we can assert that they are snake-case strings in the test cases properties (-> (or (:ue_pr payload) @@ -34,15 +34,17 @@ subject (when-let [subject (.getSubject event)] (-> subject .getSubject normalize-map)) context (->> event .getContext first .getMap normalize-map)] - (swap! *snowplow-collector* conj {:properties properties, :subject subject, :context context}))) + (swap! collector conj {:properties properties, :subject subject, :context context}))) -(defn- do-with-fake-snowplow-collector +(defn do-with-fake-snowplow-collector "Impl for `with-fake-snowplow-collector` macro; prefer using that rather than calling this directly." [f] - (mt/with-temporary-setting-values [snowplow-available true] + (mt/with-temporary-setting-values [snowplow-available true + anon-tracking-enabled true] (binding [*snowplow-collector* (atom [])] - (with-redefs [snowplow/track-event-impl! fake-track-event-impl!] - (f))))) + (let [collector *snowplow-collector*] ;; get a reference to the atom + (with-redefs [snowplow/track-event-impl! (partial fake-track-event-impl! collector)] + (f)))))) (defmacro with-fake-snowplow-collector "Creates a new fake snowplow collector in a dynamic scope, and redefines the track-event! function so that analytics @@ -57,7 +59,7 @@ [] (reset! *snowplow-collector* [])) -(defn- pop-event-data-and-user-id! +(defn pop-event-data-and-user-id! "Returns a vector containing the event data from each tracked event in the Snowplow collector as well as the user ID of the profile associated with each event, and clears the collector." [] @@ -80,47 +82,46 @@ (deftest track-event-test (with-fake-snowplow-collector (testing "Data sent into [[snowplow/track-event!]] for each event type is propagated to the Snowplow collector, - with keys converted into snake-case strings, and the subject's user ID being converted to a string." - (mt/with-temporary-setting-values [anon-tracking-enabled true] - (snowplow/track-event! ::snowplow/new-instance-created) - (is (= [{:data {"event" "new_instance_created"} - :user-id nil}] - (pop-event-data-and-user-id!))) - - (snowplow/track-event! ::snowplow/new-user-created 1) - (is (= [{:data {"event" "new_user_created"} - :user-id "1"}] - (pop-event-data-and-user-id!))) - - (snowplow/track-event! ::snowplow/invite-sent 1 {:invited-user-id 2}) - (is (= [{:data {"invited_user_id" 2, "event" "invite_sent"} - :user-id "1"}] - (pop-event-data-and-user-id!))) - - (snowplow/track-event! ::snowplow/dashboard-created 1 {:dashboard-id 1}) - (is (= [{:data {"dashboard_id" 1, "event" "dashboard_created"} - :user-id "1"}] - (pop-event-data-and-user-id!))) - - (snowplow/track-event! ::snowplow/question-added-to-dashboard 1 {:dashboard-id 1, :question-id 2}) - (is (= [{:data {"dashboard_id" 1, "event" "question_added_to_dashboard", "question_id" 2} - :user-id "1"}] - (pop-event-data-and-user-id!))) - - (snowplow/track-event! ::snowplow/database-connection-successful - 1 - {:database :postgres, :database-id 1, :source :admin}) - (is (= [{:data {"database" "postgres" - "database_id" 1 - "event" "database_connection_successful" - "source" "admin"} - :user-id "1"}] - (pop-event-data-and-user-id!))) - - (snowplow/track-event! ::snowplow/database-connection-failed 1 {:database :postgres, :source :admin}) - (is (= [{:data {"database" "postgres", "event" "database_connection_failed", "source" "admin"} - :user-id "1"}] - (pop-event-data-and-user-id!)))) + with keys converted into snake-case strings, and the subject's user ID being converted to a string." + (snowplow/track-event! ::snowplow/new-instance-created) + (is (= [{:data {"event" "new_instance_created"} + :user-id nil}] + (pop-event-data-and-user-id!))) + + (snowplow/track-event! ::snowplow/new-user-created 1) + (is (= [{:data {"event" "new_user_created"} + :user-id "1"}] + (pop-event-data-and-user-id!))) + + (snowplow/track-event! ::snowplow/invite-sent 1 {:invited-user-id 2, :source "admin"}) + (is (= [{:data {"invited_user_id" 2, "event" "invite_sent", "source" "admin"} + :user-id "1"}] + (pop-event-data-and-user-id!))) + + (snowplow/track-event! ::snowplow/dashboard-created 1 {:dashboard-id 1}) + (is (= [{:data {"dashboard_id" 1, "event" "dashboard_created"} + :user-id "1"}] + (pop-event-data-and-user-id!))) + + (snowplow/track-event! ::snowplow/question-added-to-dashboard 1 {:dashboard-id 1, :question-id 2}) + (is (= [{:data {"dashboard_id" 1, "event" "question_added_to_dashboard", "question_id" 2} + :user-id "1"}] + (pop-event-data-and-user-id!))) + + (snowplow/track-event! ::snowplow/database-connection-successful + 1 + {:database :postgres, :database-id 1, :source :admin}) + (is (= [{:data {"database" "postgres" + "database_id" 1 + "event" "database_connection_successful" + "source" "admin"} + :user-id "1"}] + (pop-event-data-and-user-id!))) + + (snowplow/track-event! ::snowplow/database-connection-failed 1 {:database :postgres, :source :admin}) + (is (= [{:data {"database" "postgres", "event" "database_connection_failed", "source" "admin"} + :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] @@ -129,7 +130,6 @@ (deftest instance-creation-test (let [original-value (db/select-one-field :value Setting :key "instance-creation")] - (def my-original-value original-value) (try (testing "Instance creation timestamp is set only once when setting is first fetched" (db/delete! Setting {:key "instance-creation"}) diff --git a/test/metabase/api/setup_test.clj b/test/metabase/api/setup_test.clj index 87819c75d8c0565ccff2994347adcd559a166a73..2ddca858c08566afbe5f2c17b46439447db39500 100644 --- a/test/metabase/api/setup_test.clj +++ b/test/metabase/api/setup_test.clj @@ -3,6 +3,7 @@ (:require [clojure.core.async :as a] [clojure.test :refer :all] [medley.core :as m] + [metabase.analytics.snowplow-test :as snowplow-test] [metabase.api.setup :as setup-api] [metabase.email :as email] [metabase.events :as events] @@ -92,25 +93,29 @@ (deftest invite-user-test (testing "POST /api/setup" - (testing "Check that a second admin can be created during setup, and that an invite email is sent successfully" + (testing "Check that a second admin can be created during setup, and that an invite email is sent successfully and + a Snowplow analytics event is sent" (mt/with-fake-inbox - (let [email (mt/random-email) - first-name (mt/random-name) - last-name (mt/random-name) - invitor-first-name (mt/random-name)] - (with-setup {:invite {:email email, :first_name first-name, :last_name last-name} - :user {:first_name invitor-first-name} - :site_name "Metabase"} - (let [invited-user (User :email email)] - (is (= (:first_name invited-user) first-name)) - (is (= (:last_name invited-user) last-name)) - (is (:is_superuser invited-user))) - (let [invite-email (-> (mt/regex-email-bodies - (re-pattern (str invitor-first-name " could use your help setting up Metabase.*"))) - (get email) - first)] - (is (= {(str invitor-first-name " could use your help setting up Metabase.*") true} - (:body invite-email)))))))) + (snowplow-test/with-fake-snowplow-collector + (let [email (mt/random-email) + first-name (mt/random-name) + last-name (mt/random-name) + invitor-first-name (mt/random-name)] + (with-setup {:invite {:email email, :first_name first-name, :last_name last-name} + :user {:first_name invitor-first-name} + :site_name "Metabase"} + (let [invited-user (User :email email)] + (is (= (:first_name invited-user) first-name)) + (is (= (:last_name invited-user) last-name)) + (is (:is_superuser invited-user)) + (is (partial= [{:data {"event" "invite_sent", + "invited_user_id" (u/the-id invited-user) + "source" "setup"}}] + (filter #(= (get-in % [:data "event"]) "invite_sent") + (snowplow-test/pop-event-data-and-user-id!)))) + (is (mt/received-email-body? + email + (re-pattern (str invitor-first-name " could use your help setting up Metabase.*")))))))))) (testing "No second user is created if email is not set up" (mt/with-temporary-setting-values [email-smtp-host nil] diff --git a/test/metabase/email_test.clj b/test/metabase/email_test.clj index f09a81396bd7560f2df5bf61105506a0abec4fc3..a8ca2beafa8fdedb4c711ef8707790b38f091e12 100644 --- a/test/metabase/email_test.clj +++ b/test/metabase/email_test.clj @@ -113,18 +113,18 @@ (regex-email-bodies* regexes @inbox)) (defn received-email-subject? - "Indicate whether user the user received an email whose subject matches the `regex`. User should be a keyword - like :rasta." - [user regex] - (let [address (:username (user/user->credentials user)) + "Indicate whether a user received an email whose subject matches the `regex`. First argument should be a keyword + like :rasta, or an email address string." + [user-or-email regex] + (let [address (if (string? user-or-email) user-or-email (:username (user/user->credentials user-or-email))) emails (get @inbox address)] (boolean (some #(re-find regex %) (map :subject emails))))) (defn received-email-body? - "Indicate whether user the user received an email whose body matches the `regex`. User should be a keyword - like :rasta." - [user regex] - (let [address (:username (user/user->credentials user)) + "Indicate whether a user received an email whose body matches the `regex`. First argument should be a keyword + like :rasta, or an email address string." + [user-or-email regex] + (let [address (if (string? user-or-email) user-or-email (:username (user/user->credentials user-or-email))) emails (get @inbox address)] (boolean (some #(re-find regex %) (map (comp :content first :body) emails)))))