From a4332353105bfc6be35d11bb04864d60a01b9ca9 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Wed, 11 Mar 2020 08:21:44 -0500 Subject: [PATCH] Merge pull request #2643 from overleaf/jel-affiliations-cron-job Ensure affiliations cron job GitOrigin-RevId: 4ac6f8b29b1e1460d627a86172fcdf1fa27a59a8 --- .../app/src/Features/User/UserController.js | 28 +++++----- services/web/app/src/router.js | 2 +- .../controllers/UserAffiliationsController.js | 2 +- .../factories/UserAffiliationsDataService.js | 6 +-- .../web/scripts/backfill_sso_affiliations.js | 53 ------------------- services/web/scripts/ensure_affiliations.js | 51 ++++++++++++++++++ .../test/unit/src/User/UserControllerTests.js | 24 ++++----- 7 files changed, 83 insertions(+), 83 deletions(-) delete mode 100644 services/web/scripts/backfill_sso_affiliations.js create mode 100644 services/web/scripts/ensure_affiliations.js diff --git a/services/web/app/src/Features/User/UserController.js b/services/web/app/src/Features/User/UserController.js index eb04b5e418..b9c61517fc 100644 --- a/services/web/app/src/Features/User/UserController.js +++ b/services/web/app/src/Features/User/UserController.js @@ -19,7 +19,7 @@ const EmailHandler = require('../Email/EmailHandler') const UrlHelper = require('../Helpers/UrlHelper') const { promisify } = require('util') -async function _ensureAffiliations(userId, emailData) { +async function _ensureAffiliation(userId, emailData) { if (emailData.samlProviderId) { await UserUpdater.promises.confirmEmail(userId, emailData.email) } else { @@ -27,14 +27,10 @@ async function _ensureAffiliations(userId, emailData) { } } -async function ensureAffiliations(userId) { +async function ensureAffiliation(user) { if (!Features.hasFeature('affiliations')) { return } - const user = await UserGetter.promises.getUser(userId) - if (!user) { - return new Errors.UserNotFoundError({ info: { userId } }) - } const flaggedEmails = user.emails.filter(email => email.affiliationUnchecked) if (flaggedEmails.length === 0) { @@ -43,21 +39,27 @@ async function ensureAffiliations(userId) { if (flaggedEmails.length > 1) { logger.error( - { userId }, + { userId: user._id }, `Unexpected number of flagged emails: ${flaggedEmails.length}` ) } - await _ensureAffiliations(userId, flaggedEmails[0]) + await _ensureAffiliation(user._id, flaggedEmails[0]) } -async function ensureAffiliationsMiddleware(req, res, next) { - if (!Features.hasFeature('affiliations') || !req.query.ensureAffiliations) { +async function ensureAffiliationMiddleware(req, res, next) { + let user + if (!Features.hasFeature('affiliations') || !req.query.ensureAffiliation) { return next() } const userId = AuthenticationController.getLoggedInUserId(req) try { - await ensureAffiliations(userId) + user = await UserGetter.promises.getUser(userId) + } catch (error) { + return new Errors.UserNotFoundError({ info: { userId } }) + } + try { + await ensureAffiliation(user) } catch (error) { return next(error) } @@ -446,8 +448,8 @@ const UserController = { UserController.promises = { doLogout: promisify(UserController.doLogout), - ensureAffiliations, - ensureAffiliationsMiddleware + ensureAffiliation, + ensureAffiliationMiddleware } module.exports = UserController diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 6bdd169529..2977f775ab 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -139,7 +139,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { webRouter.get( '/user/emails', AuthenticationController.requireLogin(), - UserController.promises.ensureAffiliationsMiddleware, + UserController.promises.ensureAffiliationMiddleware, UserEmailsController.list ) webRouter.get('/user/emails/confirm', UserEmailsController.showConfirm) diff --git a/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsController.js b/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsController.js index 622aba913c..e3a2f3b15b 100644 --- a/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsController.js +++ b/services/web/frontend/js/main/affiliations/controllers/UserAffiliationsController.js @@ -295,7 +295,7 @@ define(['base'], App => _resetMakingRequestType() $scope.ui.isLoadingEmails = true _monitorRequest( - UserAffiliationsDataService.getUserEmailsEnsureAffiliations() + UserAffiliationsDataService.getUserEmailsEnsureAffiliation() ) .then(emails => { $scope.userEmails = emails.map(email => { diff --git a/services/web/frontend/js/main/affiliations/factories/UserAffiliationsDataService.js b/services/web/frontend/js/main/affiliations/factories/UserAffiliationsDataService.js index 74573cfb31..9cc795dfb0 100644 --- a/services/web/frontend/js/main/affiliations/factories/UserAffiliationsDataService.js +++ b/services/web/frontend/js/main/affiliations/factories/UserAffiliationsDataService.js @@ -410,9 +410,9 @@ define(['base'], function(App) { const getUserEmails = () => $http.get('/user/emails').then(response => response.data) - const getUserEmailsEnsureAffiliations = () => + const getUserEmailsEnsureAffiliation = () => $http - .get('/user/emails?ensureAffiliations=true') + .get('/user/emails?ensureAffiliation=true') .then(response => response.data) const getUserDefaultEmail = () => @@ -532,7 +532,7 @@ define(['base'], function(App) { getDefaultRoleHints, getDefaultDepartmentHints, getUserEmails, - getUserEmailsEnsureAffiliations, + getUserEmailsEnsureAffiliation, getUserDefaultEmail, getUniversitiesFromCountry, getUniversityDomainFromPartialDomainInput, diff --git a/services/web/scripts/backfill_sso_affiliations.js b/services/web/scripts/backfill_sso_affiliations.js deleted file mode 100644 index 92fb867b91..0000000000 --- a/services/web/scripts/backfill_sso_affiliations.js +++ /dev/null @@ -1,53 +0,0 @@ -const { User } = require('../app/src/models/User') -const UserUpdater = require('../app/src/Features/User/UserUpdater') -require('logger-sharelatex').logger.level('error') -const pLimit = require('p-limit') -const CONCURRENCY = 10 -const unexpectedUserStates = [] - -console.log('Starting SSO affiliation backfill') - -const query = { - emails: { - $elemMatch: { - samlProviderId: { $exists: true }, - confirmedAt: { $exists: false } - } - } -} - -async function backfillAffiliation(user) { - const ssoEmail = user.emails.filter( - emailData => !emailData.confirmedAt && emailData.samlProviderId - ) - if (ssoEmail.length > 1 || ssoEmail.length === 0) { - unexpectedUserStates.push(user._id) - } - const { email } = ssoEmail[0] - await UserUpdater.promises.confirmEmail(user._id, email) -} - -async function getUsers() { - return User.find(query, { emails: 1 }).exec() -} - -async function run() { - const limit = pLimit(CONCURRENCY) - const users = await getUsers() - console.log(`Found ${users.length} users`) - await Promise.all(users.map(user => limit(() => backfillAffiliation(user)))) - console.log('Finished') - console.log( - `Found ${unexpectedUserStates.length} in unexpected states`, - unexpectedUserStates - ) -} - -run() - .then(() => { - process.exit() - }) - .catch(error => { - console.log(error) - process.exit(1) - }) diff --git a/services/web/scripts/ensure_affiliations.js b/services/web/scripts/ensure_affiliations.js new file mode 100644 index 0000000000..4ed3587ad8 --- /dev/null +++ b/services/web/scripts/ensure_affiliations.js @@ -0,0 +1,51 @@ +const { User } = require('../app/src/models/User') +const UserController = require('../app/src/Features/User/UserController') +require('logger-sharelatex').logger.level('error') +const pLimit = require('p-limit') +const CONCURRENCY = 10 +const failure = [] +const success = [] +console.log('Starting ensure affiliations') + +const query = { + 'emails.affiliationUnchecked': true +} + +async function _handleEnsureAffiliation(user) { + try { + await UserController.promises.ensureAffiliation(user) + console.log(`✔ ${user._id}`) + success.push(user._id) + } catch (error) { + failure.push(user._id) + console.log(`ERROR: ${user._id}`, error) + } +} + +async function getUsers() { + return User.find(query, { emails: 1 }).exec() +} + +async function run() { + const limit = pLimit(CONCURRENCY) + const users = await getUsers() + console.log(`Found ${users.length} users`) + await Promise.all( + users.map(user => limit(() => _handleEnsureAffiliation(user))) + ) + + console.log(`${success.length} successes`) + console.log(`${failure.length} failures`) + if (failure.length > 0) { + console.log('Failed to update:', failure) + } +} + +run() + .then(() => { + process.exit() + }) + .catch(error => { + console.log(error) + process.exit(1) + }) diff --git a/services/web/test/unit/src/User/UserControllerTests.js b/services/web/test/unit/src/User/UserControllerTests.js index 85e2f231d5..63d3aca28f 100644 --- a/services/web/test/unit/src/User/UserControllerTests.js +++ b/services/web/test/unit/src/User/UserControllerTests.js @@ -604,10 +604,10 @@ describe('UserController', function() { }) }) - describe('ensureAffiliationsMiddleware', function() { + describe('ensureAffiliationMiddleware', function() { describe('without affiliations feature', function() { beforeEach(async function() { - await this.UserController.promises.ensureAffiliationsMiddleware( + await this.UserController.promises.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -623,10 +623,10 @@ describe('UserController', function() { expect(this.next).to.be.calledWith() }) }) - describe('without ensureAffiliations query parameter', function() { + describe('without ensureAffiliation query parameter', function() { beforeEach(async function() { this.Features.hasFeature.withArgs('affiliations').returns(true) - await this.UserController.promises.ensureAffiliationsMiddleware( + await this.UserController.promises.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -652,8 +652,8 @@ describe('UserController', function() { } ] this.Features.hasFeature.withArgs('affiliations').returns(true) - this.req.query.ensureAffiliations = true - await this.UserController.promises.ensureAffiliationsMiddleware( + this.req.query.ensureAffiliation = true + await this.UserController.promises.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -684,8 +684,8 @@ describe('UserController', function() { } ] this.Features.hasFeature.withArgs('affiliations').returns(true) - this.req.query.ensureAffiliations = true - await this.UserController.promises.ensureAffiliationsMiddleware( + this.req.query.ensureAffiliation = true + await this.UserController.promises.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -716,8 +716,8 @@ describe('UserController', function() { } ] this.Features.hasFeature.withArgs('affiliations').returns(true) - this.req.query.ensureAffiliations = true - await this.UserController.promises.ensureAffiliationsMiddleware( + this.req.query.ensureAffiliation = true + await this.UserController.promises.ensureAffiliationMiddleware( this.req, this.res, this.next @@ -748,8 +748,8 @@ describe('UserController', function() { } ] this.Features.hasFeature.withArgs('affiliations').returns(true) - this.req.query.ensureAffiliations = true - await this.UserController.promises.ensureAffiliationsMiddleware( + this.req.query.ensureAffiliation = true + await this.UserController.promises.ensureAffiliationMiddleware( this.req, this.res, this.next