Send snowplow event when user is invited during setup (#19808)

snowplow/iglu-client-embedded/schemas/com.metabase/invite/jsonschema/1-0-1

+{
+  "$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
+}

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"})

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."

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)))))
 

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"})

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]

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)))))