From 72f163f0813bcb071694001abaedfc8291a2af45 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 12:53:17 -0800 Subject: [PATCH 01/10] Add type to Account based on UserType Add new method to create account from user instead of just email, so type is preserved This is so we can filter by account type later in the process I could filter here, in getting accounts from users, but that restricts the number of available sessions This way I can filter when returning availableSessions, giving priority to pro users but falling back to basic user accounts if needed --- lib/zoom.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index d781a96f..8ee193f5 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -87,8 +87,7 @@ class Session { ) const accountMeetings = await Promise.all( - Array.from(this.users.map(u => u.email)) - .map(email => this.accountForEmail(email)) + Array.from(this.users.map(u => this.accountFromUser(u.email, u.type)) .map(async function(accountSession): Promise<[Account, boolean]> { let live = await accountSession.liveMeetings() // filter out any upcoming or scheduled meetings starting within meetingLengthBuffer @@ -126,8 +125,13 @@ class Session { } private accountForEmail(email: string) { - return new Account(email, this.apiKey, this.apiSecret) + return new Account(email, this.apiKey, this.apiSecret, null) } + + private accountFromUser(email: string, type: number) { + return new Account(email, this.apiKey, this.apiSecret, type) + } + } enum MeetingScheduleCategory { @@ -158,6 +162,7 @@ class Account { private email: string, private apiKey: string, private apiSecret: string, + private type: ? UserType, ) {} // NB: we may run into pagination issues at some point, especially for From ddb94097920610844abf18ddd253dcd3fa32c814 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 13:17:53 -0800 Subject: [PATCH 02/10] Filter pro and basic users, prioritize pro --- lib/zoom.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index 8ee193f5..f5f7d591 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -115,9 +115,15 @@ class Session { const availableSessions = accountMeetings .filter(([, availableForMeeting]) => availableForMeeting) .map(([session]) => session) - const chosenIndex = Math.floor(Math.random() * availableSessions.length) - return await availableSessions[chosenIndex].createMeeting() + const availableProSessions = availableSessions.filter(session => session.type > 1) + const availableBasicSessions = availableSessions.filter(session => session.type == 1) + + const candidateSessions = (availableProSessions.length > 0 && availableProSessions) || availableBasicSessions + + const chosenIndex = Math.floor(Math.random() * candidateSessions.length) + return await candidateSessions[chosenIndex].createMeeting() + } private get token() { From 825c185c6f7784727a9d4c4dc517c3683acdeee9 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 13:40:12 -0800 Subject: [PATCH 03/10] Move live meeting check to the end of the checks We have at least one reported instance of a meeting being created with an account that was already in use There is not enough data to debug how or why this happened In the unlikely but I guess technically feasible event that this was caused by the user starting a meeting Just after the live meetings check had been made, this change would improve chances of avoiding the problem happening again --- lib/zoom.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index f5f7d591..6371fec7 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -89,7 +89,6 @@ class Session { const accountMeetings = await Promise.all( Array.from(this.users.map(u => this.accountFromUser(u.email, u.type)) .map(async function(accountSession): Promise<[Account, boolean]> { - let live = await accountSession.liveMeetings() // filter out any upcoming or scheduled meetings starting within meetingLengthBuffer let upcoming = await accountSession.upcomingMeetings() let upcomingMeetingsInBuffer = upcoming.filter(meeting => @@ -103,6 +102,7 @@ class Session { ? moment(meeting.start_time).isBetween(now, bufferExpiryTime) : false, ) + let live = await accountSession.liveMeetings() return [ accountSession, live.length == 0 && From 0067e80dbf2456750684f59de277437e4bf2fc3c Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 15:03:46 -0800 Subject: [PATCH 04/10] Fix a couple syntax errors Add missing closing paran to accountMeetings Array from line Move question mark in Account type optional param definition --- lib/zoom.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index 6371fec7..3a0667b7 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -87,7 +87,7 @@ class Session { ) const accountMeetings = await Promise.all( - Array.from(this.users.map(u => this.accountFromUser(u.email, u.type)) + Array.from(this.users.map(u => this.accountFromUser(u.email, u.type))) .map(async function(accountSession): Promise<[Account, boolean]> { // filter out any upcoming or scheduled meetings starting within meetingLengthBuffer let upcoming = await accountSession.upcomingMeetings() @@ -168,7 +168,7 @@ class Account { private email: string, private apiKey: string, private apiSecret: string, - private type: ? UserType, + private type?: UserType, ) {} // NB: we may run into pagination issues at some point, especially for From abc8746cd9d83b303b6537de419825d05032aeb8 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 15:06:08 -0800 Subject: [PATCH 05/10] Run prettier Prvious commits were all made with prettier disabled, because I had not yet figured out what syntax error was breaking linting The commit with the syntax error fixes is easier to read without also including all the linting, so I disabled prettier on that commit as well Thus, the linting for the last few commits all in one swell foop --- lib/zoom.ts | 61 +++++++++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index 3a0667b7..80612cf9 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -87,43 +87,49 @@ class Session { ) const accountMeetings = await Promise.all( - Array.from(this.users.map(u => this.accountFromUser(u.email, u.type))) - .map(async function(accountSession): Promise<[Account, boolean]> { - // filter out any upcoming or scheduled meetings starting within meetingLengthBuffer - let upcoming = await accountSession.upcomingMeetings() - let upcomingMeetingsInBuffer = upcoming.filter(meeting => - meeting.start_time - ? moment(meeting.start_time).isBetween(now, bufferExpiryTime) - : false, - ) - let scheduled = await accountSession.scheduledMeetings() - let scheduledMeetingsInBuffer = scheduled.filter(meeting => - meeting.start_time - ? moment(meeting.start_time).isBetween(now, bufferExpiryTime) - : false, - ) - let live = await accountSession.liveMeetings() - return [ - accountSession, - live.length == 0 && - upcomingMeetingsInBuffer.length == 0 && - scheduledMeetingsInBuffer.length == 0, - ] - }), + Array.from( + this.users.map(u => this.accountFromUser(u.email, u.type)), + ).map(async function(accountSession): Promise<[Account, boolean]> { + // filter out any upcoming or scheduled meetings starting within meetingLengthBuffer + let upcoming = await accountSession.upcomingMeetings() + let upcomingMeetingsInBuffer = upcoming.filter(meeting => + meeting.start_time + ? moment(meeting.start_time).isBetween(now, bufferExpiryTime) + : false, + ) + let scheduled = await accountSession.scheduledMeetings() + let scheduledMeetingsInBuffer = scheduled.filter(meeting => + meeting.start_time + ? moment(meeting.start_time).isBetween(now, bufferExpiryTime) + : false, + ) + let live = await accountSession.liveMeetings() + return [ + accountSession, + live.length == 0 && + upcomingMeetingsInBuffer.length == 0 && + scheduledMeetingsInBuffer.length == 0, + ] + }), ) const availableSessions = accountMeetings .filter(([, availableForMeeting]) => availableForMeeting) .map(([session]) => session) - const availableProSessions = availableSessions.filter(session => session.type > 1) - const availableBasicSessions = availableSessions.filter(session => session.type == 1) + const availableProSessions = availableSessions.filter( + session => session.type > 1, + ) + const availableBasicSessions = availableSessions.filter( + session => session.type == 1, + ) - const candidateSessions = (availableProSessions.length > 0 && availableProSessions) || availableBasicSessions + const candidateSessions = + (availableProSessions.length > 0 && availableProSessions) || + availableBasicSessions const chosenIndex = Math.floor(Math.random() * candidateSessions.length) return await candidateSessions[chosenIndex].createMeeting() - } private get token() { @@ -137,7 +143,6 @@ class Session { private accountFromUser(email: string, type: number) { return new Account(email, this.apiKey, this.apiSecret, type) } - } enum MeetingScheduleCategory { From 771463a2b2765ca058a553eedd9ad71ea34c0755 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 15:08:43 -0800 Subject: [PATCH 06/10] Add a note about pagination As a warning to future us, if we scale to having more than 30 zoom user accounts --- lib/zoom.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/zoom.ts b/lib/zoom.ts index 80612cf9..cf4a36ee 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -34,6 +34,10 @@ async function getSession( if (userResponse.status != 200) { throw `Error looking up users: ${util.inspect(userResponse.data)}.` } else { + // NB: We currently do not have to handle pagination, because we have fewer + // users than the number of potential results per page (30). + // If our user count (user.data.total_records) grows to exceed that, we'll + // need to update this to handle pagination. return new Session( apiKey, apiSecret, From 18cfc332294288c8357dbc74fcd921784020695e Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Mon, 11 Nov 2019 16:46:21 -0800 Subject: [PATCH 07/10] Add user facing messaging if basic acct used Note that there is a time limit, and offer workarounds Inform them instead of annoying them... hopefully --- lib/zoom.ts | 2 +- scripts/zoom.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index cf4a36ee..bf6f88ca 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -227,7 +227,7 @@ class Account { meeting: Meeting = response.data meeting.app_url = URLs.appJoin.replace(/{meetingId}/, meeting.id) - return [meeting, this.email] + return [meeting, this.email, this.type] } private get token() { diff --git a/scripts/zoom.js b/scripts/zoom.js index f53a063c..ae8cc1e2 100644 --- a/scripts/zoom.js +++ b/scripts/zoom.js @@ -126,13 +126,16 @@ module.exports = function(robot) { return } ZOOM_SESSION.nextAvailableMeeting() - .then(([meeting, zoomUserEmail]) => { + .then(([meeting, zoomUserEmail, zoomUserType]) => { robot.logger.info( - `Created meeting: ${meeting.id}: using account for ${zoomUserEmail}`, - ) - res.send( - `All set; open in [the app](${meeting.app_url}) or [your browser](${meeting.join_url})!`, + `Created meeting: ${meeting.id}: using type ${zoomUserType} account for ${zoomUserEmail}`, ) + let replyMessage = `All set; open in [the app](${meeting.app_url}) or [your browser](${meeting.join_url})!` + if (zoomUserType < 2) { + replyMessage += + "\n\nNote: this meeting has a limited duration of 40 minutes. All accounts with unlimited meetings are currently occupied. If you need a longer meeting, please try again later, or request a new meeting when this one ends." + } + res.send(replyMessage) return meeting }) .catch(err => { From 0ff90d31dfd74056f83871e9a8edd2d4290c2b2c Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Tue, 12 Nov 2019 13:54:30 -0800 Subject: [PATCH 08/10] Remove obsolete accountForEmail This is replaced by accountFromUser Also make Account type param mandatory --- lib/zoom.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index bf6f88ca..26f4df7e 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -140,10 +140,6 @@ class Session { return tokenFrom(this.apiKey, this.apiSecret) } - private accountForEmail(email: string) { - return new Account(email, this.apiKey, this.apiSecret, null) - } - private accountFromUser(email: string, type: number) { return new Account(email, this.apiKey, this.apiSecret, type) } @@ -177,7 +173,7 @@ class Account { private email: string, private apiKey: string, private apiSecret: string, - private type?: UserType, + private type: UserType, ) {} // NB: we may run into pagination issues at some point, especially for From 602f59dd953460b09651071d4df8b1573bb6d136 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Tue, 12 Nov 2019 14:27:06 -0800 Subject: [PATCH 09/10] Check specifically whether account type is basic --- scripts/zoom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/zoom.js b/scripts/zoom.js index ae8cc1e2..cfdbed9b 100644 --- a/scripts/zoom.js +++ b/scripts/zoom.js @@ -131,7 +131,7 @@ module.exports = function(robot) { `Created meeting: ${meeting.id}: using type ${zoomUserType} account for ${zoomUserEmail}`, ) let replyMessage = `All set; open in [the app](${meeting.app_url}) or [your browser](${meeting.join_url})!` - if (zoomUserType < 2) { + if (zoomUserType == 1) { replyMessage += "\n\nNote: this meeting has a limited duration of 40 minutes. All accounts with unlimited meetings are currently occupied. If you need a longer meeting, please try again later, or request a new meeting when this one ends." } From 2270c120f63274bcc078eb92fb8b1068703ddf48 Mon Sep 17 00:00:00 2001 From: Kristen Borges Date: Tue, 12 Nov 2019 14:35:34 -0800 Subject: [PATCH 10/10] Check for UserType by enum name, not number --- lib/zoom.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/zoom.ts b/lib/zoom.ts index 26f4df7e..b3c92da8 100644 --- a/lib/zoom.ts +++ b/lib/zoom.ts @@ -122,10 +122,10 @@ class Session { .map(([session]) => session) const availableProSessions = availableSessions.filter( - session => session.type > 1, + session => session.type == UserType.Pro, ) const availableBasicSessions = availableSessions.filter( - session => session.type == 1, + session => session.type != UserType.Pro, ) const candidateSessions =