From 41280a31f27064b7a5212c4cca4cd34eb62a81a6 Mon Sep 17 00:00:00 2001 From: Dmitry S Date: Wed, 14 Jun 2023 20:03:17 -0400 Subject: [PATCH] (core) Fix recording of signups, and record as telemetry too. Summary: Previously we failed to log signup info for users who signed up via Google. This fixes that issue by recording it on first post-signup visit. It also includes signup as a new telemetry event, recorded at the same point. Test Plan: Tested locally to see that a signup produces an appropriate log message and telemetry event. Reviewers: georgegevoian Reviewed By: georgegevoian Differential Revision: https://phab.getgrist.com/D3921 --- app/common/Telemetry.ts | 23 +++++++++++++++++++++++ app/server/lib/FlexServer.ts | 3 +++ app/server/lib/GristServer.ts | 2 ++ 3 files changed, 28 insertions(+) diff --git a/app/common/Telemetry.ts b/app/common/Telemetry.ts index 38fc6b57..f05d58e5 100644 --- a/app/common/Telemetry.ts +++ b/app/common/Telemetry.ts @@ -404,6 +404,28 @@ export const TelemetryContracts: TelemetryContracts = { }, }, }, + signupFirstVisit: { + description: 'Triggered when a new user first opens the Grist app', + minimumTelemetryLevel: Level.full, + metadataContracts: { + siteId: { + description: 'The site id of first visit after signup.', + dataType: 'number', + }, + siteType: { + description: 'The site type of first visit after signup.', + dataType: 'string', + }, + userId: { + description: 'The id of the user that signed up.', + dataType: 'number', + }, + altSessionId: { + description: 'A random, session-based identifier for the user that triggered this event.', + dataType: 'string', + }, + }, + }, signupVerified: { description: 'Triggered after a user successfully verifies their account during sign-up. ' + 'Not triggered in grist-core.', @@ -583,6 +605,7 @@ export const TelemetryEvents = StringUnion( 'documentUsage', 'processMonitor', 'sendingWebhooks', + 'signupFirstVisit', 'signupVerified', 'siteMembership', 'siteUsage', diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts index 395bbadf..a99fc160 100644 --- a/app/server/lib/FlexServer.ts +++ b/app/server/lib/FlexServer.ts @@ -866,6 +866,9 @@ export class FlexServer implements GristServer { recordSignUpEvent: true }}); + // Give a chance to the login system to react to the first visit after signup. + this._loginMiddleware.onFirstVisit?.(req); + const domain = mreq.org ?? null; if (!process.env.GRIST_SINGLE_ORG && this._dbManager.isMergedOrg(domain)) { // We're logging in for the first time on the merged org; if the user has diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts index cdf72c72..643d622f 100644 --- a/app/server/lib/GristServer.ts +++ b/app/server/lib/GristServer.ts @@ -75,6 +75,8 @@ export interface GristLoginMiddleware { // or null if anonymous (and other methods of determining profile such // as a cookie should not be used), or undefined to use other methods. getProfile?(req: express.Request|IncomingMessage): Promise; + // Called on first visit to an app page after a signup, for reporting or telemetry purposes. + onFirstVisit?(req: express.Request): void; } /**