github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2025-04-06 17:22:06 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #3800 from overleaf/ab-queue-onboarding-emails

Browse source 
Implement queuing for onboarding emails

GitOrigin-RevId: f1eca149a6a2cab35b4cf9c3889dc384372fd453
This commit is contained in:
Miguel Serrano 2021-03-31 11:24:39 +02:00 • committed by Copybot
parent 09ccceb38a
commit 80dff8d42c
14 changed files with 268 additions and 301 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
services/web/app.js
View file

@ -23,12 +23,10 @@ if ((Settings.sentry != null ? Settings.sentry.dsn : undefined) != null) {
}
metrics.memory.monitor(logger)
const Server = require('./app/src/infrastructure/Server')
const mongodb = require('./app/src/infrastructure/mongodb')
const mongoose = require('./app/src/infrastructure/Mongoose')
const Queues = require('./app/src/infrastructure/Queues')
Queues.initialize()
if (Settings.catchErrors) {
process.removeAllListeners('uncaughtException')

9
services/web/app/src/Features/Analytics/AnalyticsManager.js
View file

@ -2,12 +2,15 @@ const Settings = require('settings-sharelatex')
const Metrics = require('../../infrastructure/Metrics')
const Queues = require('../../infrastructure/Queues')
const analyticsEventsQueue = Queues.getAnalyticsEventsQueue()
const analyticsEditingSessionsQueue = Queues.getAnalyticsEditingSessionsQueue()
function identifyUser(userId, oldUserId) {
if (isAnalyticsDisabled() || isSmokeTestUser(userId)) {
return
}
Metrics.analyticsQueue.inc({ status: 'adding', event_type: 'identify' })
Queues.analytics.events
analyticsEventsQueue
.add('identify', { userId, oldUserId })
.then(() => {
Metrics.analyticsQueue.inc({ status: 'added', event_type: 'identify' })
@ -22,7 +25,7 @@ function recordEvent(userId, event, segmentation) {
return
}
Metrics.analyticsQueue.inc({ status: 'adding', event_type: 'event' })
Queues.analytics.events
analyticsEventsQueue
.add('event', { userId, event, segmentation })
.then(() => {
Metrics.analyticsQueue.inc({ status: 'added', event_type: 'event' })
@ -40,7 +43,7 @@ function updateEditingSession(userId, projectId, countryCode) {
status: 'adding',
event_type: 'editing-session'
})
Queues.analytics.editingSessions
analyticsEditingSessionsQueue
.add({ userId, projectId, countryCode })
.then(() => {
Metrics.analyticsQueue.inc({

9
services/web/app/src/Features/User/UserCreator.js
View file

@ -7,6 +7,7 @@ const UserDeleter = require('./UserDeleter')
const UserGetter = require('./UserGetter')
const UserUpdater = require('./UserUpdater')
const Analytics = require('../Analytics/AnalyticsManager')
const UserOnboardingEmailQueueManager = require('./UserOnboardingEmailManager')
async function _addAffiliation(user, affiliationOptions) {
try {
@ -84,6 +85,14 @@ async function createNewUser(attributes, options = {}) {
}
Analytics.recordEvent(user._id, 'user-registered')
try {
await UserOnboardingEmailQueueManager.scheduleOnboardingEmail(user)
} catch (error) {
logger.error(
`Failed to schedule sending of onboarding email for user '${user._id}'`,
error
)
}
return user
}

59
services/web/app/src/Features/User/UserOnboardingController.js
View file

@ -1,59 +0,0 @@
const { db, ObjectId } = require('../../infrastructure/mongodb')
const UserUpdater = require('./UserUpdater')
const EmailHandler = require('../Email/EmailHandler')
const logger = require('logger-sharelatex')
const async = require('async')
const _ = require('underscore')
module.exports = {
sendRecentSignupOnboardingEmails(req, res, next) {
res.setTimeout(600 * 1000) // increase timeout to handle days with a lot of signups
// find all the users with no onboardingEmailSentAt and
// have signed up in the last 7 days
db.users
.find(
{
onboardingEmailSentAt: null,
_id: {
$gt: ObjectId.createFromTime(Date.now() / 1000 - 7 * 24 * 60 * 60)
}
},
{ projection: { email: 1 } }
)
.toArray(function(error, users) {
if (error) {
return next(error)
}
const ids = _.map(users, function(user) {
return user._id
})
logger.log('SENDING USER ONBOARDING EMAILS TO: ', ids)
async.mapLimit(users, 10, sendOne, function(error) {
if (error) {
return next(error)
}
logger.log('DONE SENDING ONBOARDING EMAILS')
res.send(ids)
})
})
}
}
function sendOne(user, callback) {
var opts = {
to: user.email
}
EmailHandler.sendEmail('userOnboardingEmail', opts, function(error) {
if (error) {
return callback(error)
}
UserUpdater.updateUser(
user._id,
{ $set: { onboardingEmailSentAt: new Date() } },
function() {
callback()
}
)
})
}

37
services/web/app/src/Features/User/UserOnboardingEmailManager.js Normal file
View file

@ -0,0 +1,37 @@
const Queues = require('../../infrastructure/Queues')
const EmailHandler = require('../Email/EmailHandler')
const UserUpdater = require('./UserUpdater')
const UserGetter = require('./UserGetter')
const ONE_DAY_MS = 24 * 60 * 60 * 1000
class UserOnboardingEmailManager {
constructor() {
this.queue = Queues.getOnboardingEmailsQueue()
this.queue.process(async job => {
const { userId } = job.data
await this._sendOnboardingEmail(userId)
})
}
async scheduleOnboardingEmail(user) {
await this.queue.add({ userId: user._id }, { delay: ONE_DAY_MS })
}
async _sendOnboardingEmail(userId) {
const user = await UserGetter.promises.getUser(
{ _id: userId },
{ email: 1 }
)
if (user) {
await EmailHandler.promises.sendEmail('userOnboardingEmail', {
to: user.email
})
await UserUpdater.promises.updateUser(user._id, {
$set: { onboardingEmailSentAt: new Date() }
})
}
}
}
module.exports = new UserOnboardingEmailManager()

71
services/web/app/src/Features/User/UserRegistrationHandler.js
View file

@ -13,21 +13,17 @@ const settings = require('settings-sharelatex')
const EmailHelper = require('../Helpers/EmailHelper')
const UserRegistrationHandler = {
_registrationRequestIsValid(body, callback) {
_registrationRequestIsValid(body) {
const invalidEmail = AuthenticationManager.validateEmail(body.email || '')
const invalidPassword = AuthenticationManager.validatePassword(
body.password || '',
body.email
)
if (invalidEmail != null || invalidPassword != null) {
return false
} else {
return true
}
return !(invalidEmail || invalidPassword)
},
_createNewUserIfRequired(user, userDetails, callback) {
if (user == null) {
if (!user) {
userDetails.holdingAccount = false
UserCreator.createNewUser(
{
@ -51,48 +47,48 @@ const UserRegistrationHandler = {
return callback(new Error('request is not valid'))
}
userDetails.email = EmailHelper.parseEmail(userDetails.email)
UserGetter.getUserByAnyEmail(userDetails.email, (err, user) => {
if (err != null) {
return callback(err)
UserGetter.getUserByAnyEmail(userDetails.email, (error, user) => {
if (error) {
return callback(error)
}
if ((user != null ? user.holdingAccount : undefined) === false) {
if (user && user.holdingAccount === false) {
return callback(new Error('EmailAlreadyRegistered'), user)
}
self._createNewUserIfRequired(user, userDetails, (err, user) => {
if (err != null) {
return callback(err)
self._createNewUserIfRequired(user, userDetails, (error, user) => {
if (error) {
return callback(error)
}
async.series(
[
cb =>
callback =>
User.updateOne(
{ _id: user._id },
{ $set: { holdingAccount: false } },
cb
callback
),
cb =>
callback =>
AuthenticationManager.setUserPassword(
user,
userDetails.password,
cb
callback
),
cb => {
callback => {
if (userDetails.subscribeToNewsletter === 'true') {
NewsletterManager.subscribe(user, err => {
if (err != null) {
NewsletterManager.subscribe(user, error => {
if (error) {
logger.warn(
{ err, user },
{ err: error, user },
'Failed to subscribe user to newsletter'
)
}
})
}
cb()
callback()
} // this can be slow, just fire it off
],
err => {
error => {
Analytics.recordEvent(user._id, 'user-registered')
callback(err, user)
callback(error, user)
}
)
})
@ -105,17 +101,12 @@ const UserRegistrationHandler = {
email,
password: crypto.randomBytes(32).toString('hex')
},
(err, user) => {
if (
err != null &&
(err != null ? err.message : undefined) !== 'EmailAlreadyRegistered'
) {
return callback(err)
(error, user) => {
if (error && error.message !== 'EmailAlreadyRegistered') {
return callback(error)
}
if (
(err != null ? err.message : undefined) === 'EmailAlreadyRegistered'
) {
if (error && error.message === 'EmailAlreadyRegistered') {
logger.log({ email }, 'user already exists, resending welcome email')
}
@ -124,9 +115,9 @@ const UserRegistrationHandler = {
'password',
{ user_id: user._id.toString(), email: user.email },
{ expiresIn: ONE_WEEK },
(err, token) => {
if (err != null) {
return callback(err)
(error, token) => {
if (error) {
return callback(error)
}
const setNewPasswordUrl = `${settings.siteUrl}/user/activate?token=${token}&user_id=${user._id}`
@ -137,9 +128,9 @@ const UserRegistrationHandler = {
to: user.email,
setNewPasswordUrl
},
err => {
if (err != null) {
logger.warn({ err }, 'failed to send activation email')
error => {
if (error) {
logger.warn({ err: error }, 'failed to send activation email')
}
}
)

54
services/web/app/src/infrastructure/Queues.js
View file

@ -1,34 +1,50 @@
const Queue = require('bull')
const Settings = require('settings-sharelatex')
const analyticsQueues = {}
// Bull will keep a fixed number of the most recently completed jobs. This is
// useful to inspect recently completed jobs. The bull prometheus exporter also
// uses the completed job records to report on job duration.
const MAX_COMPLETED_JOBS_RETAINED = 10000
const MAX_FAILED_JOBS_RETAINED = 50000
function initialize() {
const queues = {}
function getAnalyticsEventsQueue() {
if (Settings.analytics.enabled) {
analyticsQueues.events = createQueue('analytics-events')
analyticsQueues.editingSessions = createQueue('analytics-editing-sessions')
return getOrCreateQueue('analytics-events')
}
}
function createQueue(queueName, defaultJobOptions) {
return new Queue(queueName, {
redis: Settings.redis.queues,
defaultJobOptions: {
removeOnComplete: MAX_COMPLETED_JOBS_RETAINED,
removeOnFail: MAX_FAILED_JOBS_RETAINED,
attempts: 11,
backoff: {
type: 'exponential',
delay: 3000
}
}
})
function getAnalyticsEditingSessionsQueue() {
if (Settings.analytics.enabled) {
return getOrCreateQueue('analytics-editing-sessions')
}
}
module.exports = { initialize, analytics: analyticsQueues }
function getOnboardingEmailsQueue() {
return getOrCreateQueue('emails-onboarding')
}
function getOrCreateQueue(queueName, defaultJobOptions) {
if (!queues[queueName]) {
queues[queueName] = new Queue(queueName, {
redis: Settings.redis.queues,
defaultJobOptions: {
removeOnComplete: MAX_COMPLETED_JOBS_RETAINED,
removeOnFail: MAX_FAILED_JOBS_RETAINED,
attempts: 11,
backoff: {
type: 'exponential',
delay: 3000
}
}
})
}
return queues[queueName]
}
module.exports = {
getAnalyticsEventsQueue,
getAnalyticsEditingSessionsQueue,
getOnboardingEmailsQueue
}

7
services/web/app/src/router.js
View file

@ -18,7 +18,6 @@ const UserInfoController = require('./Features/User/UserInfoController')
const UserController = require('./Features/User/UserController')
const UserEmailsController = require('./Features/User/UserEmailsController')
const UserPagesController = require('./Features/User/UserPagesController')
const UserOnboardingController = require('./Features/User/UserOnboardingController')
const DocumentController = require('./Features/Documents/DocumentController')
const CompileManager = require('./Features/Compile/CompileManager')
const CompileController = require('./Features/Compile/CompileController')
@ -232,12 +231,6 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
UserInfoController.getPersonalInfo
)
privateApiRouter.post(
'/user/onboarding_emails',
AuthenticationController.httpAuth,
UserOnboardingController.sendRecentSignupOnboardingEmails
)
webRouter.get(
'/user/reconfirm',
UserPagesController.renderReconfirmAccountPage

2
services/web/package.json
View file

@ -24,7 +24,7 @@
"test:karma": "karma start",
"test:karma:single": "karma start --no-auto-watch --single-run",
"start": "node $NODE_APP_OPTIONS app.js",
"nodemon": "nodemon --config nodemon.json",
"nodemon": "nodemon $NODE_APP_OPTIONS --config nodemon.json",
"webpack": "webpack-dev-server --config webpack.config.dev.js",
"webpack:production": "webpack --config webpack.config.prod.js",
"format": "prettier --list-different $PWD/'**/*.js'",

65
services/web/test/acceptance/src/UserOnboardingTests.js
View file

@ -1,65 +0,0 @@
const { expect } = require('chai')
const async = require('async')
const User = require('./helpers/User')
const request = require('./helpers/request')
const { db, ObjectId } = require('../../../app/src/infrastructure/mongodb')
const _ = require('underscore')
describe('UserOnboardingTests', function() {
beforeEach(function(done) {
// 2 new users
this.user1 = new User()
this.user2 = new User()
// 1 older
this.user3 = new User()
this.user3._id = ObjectId('5d15fca20000000000000000')
async.series(
[
cb => db.users.insert(this.user3, cb),
this.user1.ensureUserExists.bind(this.user1),
this.user2.ensureUserExists.bind(this.user2)
],
done
)
})
it('should send emails to the new users only', function(done) {
request(
{
method: 'POST',
url: '/user/onboarding_emails',
auth: {
username: 'sharelatex',
password: 'password',
sendImmediately: true
}
},
(error, response, body) => {
if (error != null) {
throw error
}
// should have sent two emails to new users
expect(response.statusCode).to.equal(200)
expect(response.body).to.include(this.user1._id)
expect(response.body).to.include(this.user2._id)
expect(response.body).to.not.include(this.user3._id)
// user 3 should still not have had an email sent
const user3 = this.user3
db.users
.find({
onboardingEmailSentAt: null
})
.toArray((error, users) => {
if (error != null) {
throw error
}
const ids = _.map(users, user => user._id.toString())
expect(ids.length).to.equal(1)
expect(ids).to.include(user3._id.toString())
done()
})
}
)
})
})

49
services/web/test/unit/src/Analytics/AnalyticsManagerTests.js
View file

@ -13,15 +13,28 @@ describe('AnalyticsManager', function() {
this.Settings = {
analytics: { enabled: true }
}
this.analyticsEventsQueue = {
add: sinon.stub().resolves(),
process: sinon.stub().resolves()
}
this.analyticsEditingSessionQueue = {
add: sinon.stub().resolves(),
process: sinon.stub().resolves()
}
this.onboardingEmailsQueue = {
add: sinon.stub().resolves(),
process: sinon.stub().resolves()
}
const self = this
this.Queues = {
analytics: {
events: {
add: sinon.stub().resolves()
},
editingSessions: {
add: sinon.stub().resolves()
}
getAnalyticsEventsQueue: () => {
return self.analyticsEventsQueue
},
getAnalyticsEditingSessionsQueue: () => {
return self.analyticsEditingSessionQueue
},
getOnboardingEmailsQueue: () => {
return self.onboardingEmailsQueue
}
}
this.backgroundRequest = sinon.stub().yields()
@ -44,13 +57,13 @@ describe('AnalyticsManager', function() {
it('user is smoke test user', function() {
this.Settings.smokeTest = { userId: this.fakeUserId }
this.AnalyticsManager.identifyUser(this.fakeUserId, '')
sinon.assert.notCalled(this.Queues.analytics.events.add)
sinon.assert.notCalled(this.analyticsEventsQueue.add)
})
it('analytics service is disabled', function() {
this.Settings.analytics.enabled = false
this.AnalyticsManager.identifyUser(this.fakeUserId, '')
sinon.assert.notCalled(this.Queues.analytics.events.add)
sinon.assert.notCalled(this.analyticsEventsQueue.add)
})
})
@ -58,20 +71,16 @@ describe('AnalyticsManager', function() {
it('identifyUser', function() {
const oldUserId = '456def'
this.AnalyticsManager.identifyUser(this.fakeUserId, oldUserId)
sinon.assert.calledWithMatch(
this.Queues.analytics.events.add,
'identify',
{
userId: this.fakeUserId,
oldUserId
}
)
sinon.assert.calledWithMatch(this.analyticsEventsQueue.add, 'identify', {
userId: this.fakeUserId,
oldUserId
})
})
it('recordEvent', function() {
const event = 'fake-event'
this.AnalyticsManager.recordEvent(this.fakeUserId, event, null)
sinon.assert.calledWithMatch(this.Queues.analytics.events.add, 'event', {
sinon.assert.calledWithMatch(this.analyticsEventsQueue.add, 'event', {
event,
userId: this.fakeUserId,
segmentation: null
@ -86,7 +95,7 @@ describe('AnalyticsManager', function() {
projectId,
countryCode
)
sinon.assert.calledWithMatch(this.Queues.analytics.editingSessions.add, {
sinon.assert.calledWithMatch(this.analyticsEditingSessionQueue.add, {
userId: this.fakeUserId,
projectId,
countryCode

14
services/web/test/unit/src/User/UserCreatorTests.js
View file

@ -50,6 +50,9 @@ describe('UserCreator', function() {
}),
'../Analytics/AnalyticsManager': (this.Analytics = {
recordEvent: sinon.stub()
}),
'./UserOnboardingEmailManager': (this.UserOnboardingEmailManager = {
scheduleOnboardingEmail: sinon.stub()
})
}
})
@ -276,6 +279,17 @@ describe('UserCreator', function() {
'user-registered'
)
})
it('should schedule an onboarding email on registration', async function() {
const user = await this.UserCreator.promises.createNewUser({
email: this.email
})
assert.equal(user.email, this.email)
sinon.assert.calledWith(
this.UserOnboardingEmailManager.scheduleOnboardingEmail,
user
)
})
})
})
})

84
services/web/test/unit/src/User/UserOnboardingControllerTests.js
View file

@ -1,84 +0,0 @@
const SandboxedModule = require('sandboxed-module')
const modulePath =
'../../../../app/src/Features/User/UserOnboardingController.js'
const { ObjectId } = require('mongodb')
const sinon = require('sinon')
describe('UserOnboardingController', function() {
beforeEach(function() {
this.date = new Date().getTime()
sinon.useFakeTimers(this.date)
this.users = [
{
_id: ObjectId('00000001f037be01a0e3a541')
},
{
_id: ObjectId('00000001f037be01a0e3a542')
},
{
_id: ObjectId('00000001f037be01a0e3a543')
}
]
this.mongodb = {
db: {
users: {
find: sinon
.stub()
.returns({ toArray: sinon.stub().yields(null, this.users) })
}
},
ObjectId: ObjectId
}
this.logger = {
log() {}
}
this.UserUpdater = {
updateUser: sinon.stub().callsArgWith(2, null)
}
this.EmailHandler = {
sendEmail: sinon.stub().callsArgWith(2)
}
this.UserOnboardingController = SandboxedModule.require(modulePath, {
requires: {
'../../infrastructure/mongodb': this.mongodb,
'./UserUpdater': this.UserUpdater,
'../Email/EmailHandler': this.EmailHandler,
'logger-sharelatex': this.logger
}
})
this.req = {}
this.res = {
setTimeout: sinon.stub()
}
})
it('sends onboarding emails', function(done) {
this.res.send = ids => {
ids.length.should.equal(3)
this.mongodb.db.users.find.calledOnce.should.equal(true)
this.EmailHandler.sendEmail.calledThrice.should.equal(true)
this.UserUpdater.updateUser.calledThrice.should.equal(true)
for (var i = 0; i < 3; i++) {
this.UserUpdater.updateUser
.calledWith(
this.users[0]._id,
sinon.match({
$set: { onboardingEmailSentAt: new Date(this.date) }
})
)
.should.equal(true)
}
done()
}
this.UserOnboardingController.sendRecentSignupOnboardingEmails(
this.req,
this.res
)
})
})

105
services/web/test/unit/src/User/UserOnboardingEmailManagerTests.js Normal file
View file

@ -0,0 +1,105 @@
const SandboxedModule = require('sandboxed-module')
const path = require('path')
const sinon = require('sinon')
const MODULE_PATH = path.join(
__dirname,
'../../../../app/src/Features/User/UserOnboardingEmailManager'
)
describe('UserOnboardingEmailManager', function() {
beforeEach(function() {
this.fakeUserId = '123abc'
this.fakeUserEmail = 'frog@overleaf.com'
this.onboardingEmailsQueue = {
add: sinon.stub().resolves(),
process: callback => {
this.queueProcessFunction = callback
}
}
const self = this
this.Queues = {
getOnboardingEmailsQueue: () => {
return self.onboardingEmailsQueue
}
}
this.UserGetter = {
promises: {
getUser: sinon.stub().resolves({
_id: this.fakeUserId,
email: this.fakeUserEmail
})
}
}
this.EmailHandler = {
promises: {
sendEmail: sinon.stub().resolves()
}
}
this.UserUpdater = {
promises: {
updateUser: sinon.stub().resolves()
}
}
this.request = sinon.stub().yields()
this.UserOnboardingEmailManager = SandboxedModule.require(MODULE_PATH, {
globals: {
console: console
},
requires: {
'../../infrastructure/Queues': this.Queues,
'../Email/EmailHandler': this.EmailHandler,
'./UserGetter': this.UserGetter,
'./UserUpdater': this.UserUpdater
}
})
})
describe('schedule email', function() {
it('should schedule delayed job on queue', function() {
this.UserOnboardingEmailManager.scheduleOnboardingEmail({
_id: this.fakeUserId
})
sinon.assert.calledWith(
this.onboardingEmailsQueue.add,
{ userId: this.fakeUserId },
{ delay: 24 * 60 * 60 * 1000 }
)
})
it('queue process callback should send onboarding email and update user', async function() {
await this.queueProcessFunction({ data: { userId: this.fakeUserId } })
sinon.assert.calledWith(
this.UserGetter.promises.getUser,
{ _id: this.fakeUserId },
{ email: 1 }
)
sinon.assert.calledWith(
this.EmailHandler.promises.sendEmail,
'userOnboardingEmail',
{
to: this.fakeUserEmail
}
)
sinon.assert.calledWith(
this.UserUpdater.promises.updateUser,
this.fakeUserId,
{
$set: { onboardingEmailSentAt: sinon.match.date }
}
)
})
it('queue process callback should stop if user is not found', async function() {
this.UserGetter.promises.getUser = sinon.stub().resolves()
await this.queueProcessFunction({ data: { userId: 'deleted-user' } })
sinon.assert.calledWith(
this.UserGetter.promises.getUser,
{ _id: 'deleted-user' },
{ email: 1 }
)
sinon.assert.notCalled(this.EmailHandler.promises.sendEmail)
sinon.assert.notCalled(this.UserUpdater.promises.updateUser)
})
})
})