From 9b29fa7cbca8f5319d7304f04cbd36e6c4f80296 Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Tue, 18 May 2021 08:40:56 -0500 Subject: [PATCH] Merge pull request #4035 from overleaf/jel-reconfirmation-dropbox-notification Notification for Dropbox unlinked due to reconfirmation lapse GitOrigin-RevId: 03d2bed922e1d3dd993f9227b8e7675af42eda4b --- .../InstitutionsReconfirmationHandler.js | 89 --------------- .../Notifications/NotificationsBuilder.js | 24 ++++ .../Features/Subscription/FeaturesUpdater.js | 7 +- .../ThirdPartyDataStore/TpdsUpdateHandler.js | 2 +- .../views/_mixins/reconfirm_affiliation.pug | 2 +- .../app/views/project/list/notifications.pug | 11 ++ services/web/locales/en.json | 3 +- .../scripts/process_lapsed_reconfirmations.js | 2 +- .../InstitutionsReconfirmationHandlerTests.js | 70 ------------ .../InstitutionsReconfirmationHandlerTests.js | 108 ------------------ .../NotificationsBuilderTests.js | 58 ++++++++-- .../src/Subscription/FeaturesUpdaterTests.js | 2 +- .../TpdsUpdateHandlerTests.js | 3 +- 13 files changed, 95 insertions(+), 286 deletions(-) delete mode 100644 services/web/app/src/Features/Institutions/InstitutionsReconfirmationHandler.js delete mode 100644 services/web/test/acceptance/src/InstitutionsReconfirmationHandlerTests.js delete mode 100644 services/web/test/unit/src/Institutions/InstitutionsReconfirmationHandlerTests.js diff --git a/services/web/app/src/Features/Institutions/InstitutionsReconfirmationHandler.js b/services/web/app/src/Features/Institutions/InstitutionsReconfirmationHandler.js deleted file mode 100644 index 2af57c9fd3..0000000000 --- a/services/web/app/src/Features/Institutions/InstitutionsReconfirmationHandler.js +++ /dev/null @@ -1,89 +0,0 @@ -const _ = require('lodash') -const { ObjectId, waitForDb } = require('../../infrastructure/mongodb') -const async = require('async') -const logger = require('logger-sharelatex') -const FeaturesUpdater = require('../Subscription/FeaturesUpdater') -const InstitutionsAPI = require('./InstitutionsAPI') - -const ASYNC_LIMIT = 10 - -const processLapsedLogger = { - refreshedUsers: [], - failedToRefresh: [], - printSummary: () => { - logger.log( - `Reconfirmations lapsed processed. ${processLapsedLogger.refreshedUsers.length} successfull and ${processLapsedLogger.failedToRefresh.length} failed.`, - { - refreshedUsers: processLapsedLogger.refreshedUsers, - failedToRefresh: processLapsedLogger.failedToRefresh, - } - ) - }, -} - -function _validateUserIdList(userIds) { - if (!Array.isArray(userIds)) throw new Error('users is not an array') - - userIds.forEach(userId => { - if (!ObjectId.isValid(userId)) throw new Error('user ID not valid') - }) -} - -function _refreshUser(userId, callback) { - FeaturesUpdater.refreshFeatures(userId, 'reconfirmation-lapsed', error => { - if (error) { - logger.warn(`Failed to refresh features for ${userId}`, error) - processLapsedLogger.failedToRefresh.push(userId) - } else { - processLapsedLogger.refreshedUsers.push(userId) - } - return callback() - }) -} - -async function _loopRefreshUsers(userIds) { - await new Promise((resolve, reject) => { - async.eachLimit(userIds, ASYNC_LIMIT, _refreshUser, error => { - if (error) return reject(error) - resolve() - }) - }) -} - -async function processLapsed() { - logger.log('Begin processing lapsed reconfirmations') - await waitForDb() - - const result = await InstitutionsAPI.promises.getUsersNeedingReconfirmationsLapsedProcessed() - const userIds = _.get(result, ['data', 'users']) - - _validateUserIdList(userIds) - logger.log( - `Starting to process ${userIds.length} users with lapsed reconfirmations` - ) - await _loopRefreshUsers(userIds) - - processLapsedLogger.printSummary() - - try { - logger.log('Updating reconfirmations lapsed processed dates') - await InstitutionsAPI.promises.sendUsersWithReconfirmationsLapsedProcessed( - processLapsedLogger.refreshedUsers - ) - } catch (error) { - logger.log('Error updating features_refreshed_at', error) - } - - logger.log('Done processing lapsed reconfirmations') - - return { - refreshedUsers: processLapsedLogger.refreshedUsers, - failedToRefresh: processLapsedLogger.failedToRefresh, - } -} - -const InstitutionsReconfirmationHandler = { - processLapsed, -} - -module.exports = InstitutionsReconfirmationHandler diff --git a/services/web/app/src/Features/Notifications/NotificationsBuilder.js b/services/web/app/src/Features/Notifications/NotificationsBuilder.js index 33d25d0fe7..9f02699269 100644 --- a/services/web/app/src/Features/Notifications/NotificationsBuilder.js +++ b/services/web/app/src/Features/Notifications/NotificationsBuilder.js @@ -29,6 +29,26 @@ function dropboxDuplicateProjectNames(userId) { } } +function dropboxUnlinkedDueToLapsedReconfirmation(userId) { + return { + key: 'drobox-unlinked-due-to-lapsed-reconfirmation', + create(callback) { + NotificationsHandler.createNotification( + userId, + this.key, + 'notification_dropbox_unlinked_due_to_lapsed_reconfirmation', + {}, + null, + true, + callback + ) + }, + read(callback) { + NotificationsHandler.markAsReadWithKey(userId, this.key, callback) + }, + } +} + function featuresUpgradedByAffiliation(affiliation, user) { return { key: `features-updated-by=${affiliation.institutionId}`, @@ -206,6 +226,7 @@ function tpdsFileLimit(userId) { const NotificationsBuilder = { // Note: notification keys should be url-safe + dropboxUnlinkedDueToLapsedReconfirmation, dropboxDuplicateProjectNames, featuresUpgradedByAffiliation, redundantPersonalSubscription, @@ -215,6 +236,9 @@ const NotificationsBuilder = { } NotificationsBuilder.promises = { + dropboxUnlinkedDueToLapsedReconfirmation: function (userId) { + return promisifyAll(dropboxUnlinkedDueToLapsedReconfirmation(userId)) + }, redundantPersonalSubscription: function (affiliation, user) { return promisifyAll(redundantPersonalSubscription(affiliation, user)) }, diff --git a/services/web/app/src/Features/Subscription/FeaturesUpdater.js b/services/web/app/src/Features/Subscription/FeaturesUpdater.js index d03b7ef6f4..ca77ad388d 100644 --- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/FeaturesUpdater.js @@ -33,13 +33,16 @@ const FeaturesUpdater = { if (oldFeatures.dropbox === true && features.dropbox === false) { logger.log({ userId }, '[FeaturesUpdater] must unlink dropbox') const Modules = require('../../infrastructure/Modules') - Modules.hooks.fire('removeDropbox', userId, err => { + Modules.hooks.fire('removeDropbox', userId, reason, err => { if (err) { logger.error(err) } + + return callback(null, newFeatures, featuresChanged) }) + } else { + return callback(null, newFeatures, featuresChanged) } - return callback(null, newFeatures, featuresChanged) } ) }) diff --git a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js index d6cf06d6ae..7cf8f7a29d 100644 --- a/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js +++ b/services/web/app/src/Features/ThirdPartyDataStore/TpdsUpdateHandler.js @@ -142,7 +142,7 @@ function getOrCreateProject(userId, projectName, callback) { } function handleDuplicateProjects(userId, projectName, callback) { - Modules.hooks.fire('removeDropbox', userId, err => { + Modules.hooks.fire('removeDropbox', userId, 'duplicate-projects', err => { if (err) { return callback(err) } diff --git a/services/web/app/views/_mixins/reconfirm_affiliation.pug b/services/web/app/views/_mixins/reconfirm_affiliation.pug index 9e2aabde56..7f9443e231 100644 --- a/services/web/app/views/_mixins/reconfirm_affiliation.pug +++ b/services/web/app/views/_mixins/reconfirm_affiliation.pug @@ -19,7 +19,7 @@ mixin reconfirmAffiliationNotification(location) | !{translate("please_reconfirm_institutional_email", {}, [{name: 'a', attrs: {href: '/user/settings?remove={{userEmail.email}}' }}])} |   - a(href="/learn/how-to/Reconfirm_an_institutional_email_address") #{translate("learn_more")} + a(href="/learn/how-to/Institutional_Email_Reconfirmation") #{translate("learn_more")} div(ng-if="reconfirm[userEmail.email].reconfirmationSent") | !{translate("please_check_your_inbox_to_confirm", {institutionName: '{{userEmail.affiliation.institution.name}}'}, ['strong'])} diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug index cda1a1cbbc..7c9eaa17c7 100644 --- a/services/web/app/views/project/list/notifications.pug +++ b/services/web/app/views/project/list/notifications.pug @@ -114,6 +114,17 @@ include ../../_mixins/reconfirm_affiliation span(aria-hidden="true") × span.sr-only #{translate("close")} + .alert.alert-info( + ng-switch-when="notification_dropbox_unlinked_due_to_lapsed_reconfirmation" + ) + .notification-body + | !{translate("dropbox_unlinked_premium_feature", {}, ['strong'])} + |   + a(href="/learn/how-to/Institutional_Email_Reconfirmation") #{translate("learn_more")} + .notification-close + button.btn-sm(ng-click="dismiss(notification)").close.pull-right + span(aria-hidden="true") × + span.sr-only #{translate("close")} .alert.alert-info( ng-switch-default ) diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 574b76bb21..550722d911 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1445,5 +1445,6 @@ "no_symbols_found": "No symbols found", "search": "Search", "also": "Also", - "add_email": "Add Email" + "add_email": "Add Email", + "dropbox_unlinked_premium_feature": "<0>Your Dropbox account has been unlinked because Dropbox Sync is a premium feature that you had through an institutional license. Please confirm you are still at the institution or upgrade your account in order to relink your Dropbox account." } diff --git a/services/web/scripts/process_lapsed_reconfirmations.js b/services/web/scripts/process_lapsed_reconfirmations.js index 97dcc47953..7bd7c8c4d1 100644 --- a/services/web/scripts/process_lapsed_reconfirmations.js +++ b/services/web/scripts/process_lapsed_reconfirmations.js @@ -1,4 +1,4 @@ -const InstitutionsReconfirmationHandler = require('../app/src/Features/Institutions/InstitutionsReconfirmationHandler') +const InstitutionsReconfirmationHandler = require('../modules/overleaf-integration/app/src/Institutions/InstitutionsReconfirmationHandler') InstitutionsReconfirmationHandler.processLapsed() .then(() => { diff --git a/services/web/test/acceptance/src/InstitutionsReconfirmationHandlerTests.js b/services/web/test/acceptance/src/InstitutionsReconfirmationHandlerTests.js deleted file mode 100644 index cb9cf1431b..0000000000 --- a/services/web/test/acceptance/src/InstitutionsReconfirmationHandlerTests.js +++ /dev/null @@ -1,70 +0,0 @@ -const { expect } = require('chai') -const Settings = require('settings-sharelatex') -const UserHelper = require('./helpers/UserHelper') -const MockV1ApiClass = require('./mocks/MockV1Api') - -const InstitutionsReconfirmationHandler = require('../../../app/src/Features/Institutions/InstitutionsReconfirmationHandler') - -let MockV1Api -let userHelper = new UserHelper() - -before(function () { - MockV1Api = MockV1ApiClass.instance() -}) - -describe('InstitutionsReconfirmationHandler', function () { - const institutionUsers = [] - let result - - beforeEach(async function () { - // create institution - const domain = 'institution-1.com' - const maxConfirmationMonths = 6 - MockV1Api.createInstitution({ - commonsAccount: true, - confirmed: true, - hostname: domain, - maxConfirmationMonths, - }) - - // create users affiliated with institution - async function _createInstitutionUserPastReconfirmation() { - userHelper = await UserHelper.createUser() - const userId = userHelper.user._id - - // add the affiliation - userHelper = await UserHelper.loginUser( - userHelper.getDefaultEmailPassword() - ) - const institutionEmail = `${userId}@${domain}` - await userHelper.addEmailAndConfirm(userId, institutionEmail) - institutionUsers.push(userId) - - // backdate confirmation - await userHelper.changeConfirmedToPastReconfirmation( - userId, - institutionEmail, - maxConfirmationMonths - ) - - // verify user has features before script run - const result = await UserHelper.getUser( - { _id: userHelper.user._id }, - { features: 1 } - ) - expect(result.user.features).to.deep.equal(Settings.features.professional) - - return userId - } - await _createInstitutionUserPastReconfirmation() - await _createInstitutionUserPastReconfirmation() - await _createInstitutionUserPastReconfirmation() - - result = await InstitutionsReconfirmationHandler.processLapsed() - }) - - it('should refresh features', async function () { - expect(result.failedToRefresh.length).to.equal(0) - expect(result.refreshedUsers.length).to.equal(3) - }) -}) diff --git a/services/web/test/unit/src/Institutions/InstitutionsReconfirmationHandlerTests.js b/services/web/test/unit/src/Institutions/InstitutionsReconfirmationHandlerTests.js deleted file mode 100644 index 8d284937d6..0000000000 --- a/services/web/test/unit/src/Institutions/InstitutionsReconfirmationHandlerTests.js +++ /dev/null @@ -1,108 +0,0 @@ -const SandboxedModule = require('sandboxed-module') -const path = require('path') -const sinon = require('sinon') -const { ObjectId } = require('mongodb') -const { expect } = require('chai') -const modulePath = path.join( - __dirname, - '../../../../app/src/Features/Institutions/InstitutionsReconfirmationHandler' -) - -describe('InstitutionsReconfirmationHandler', function () { - beforeEach(function () { - this.InstitutionsReconfirmationHandler = SandboxedModule.require( - modulePath, - { - requires: { - '../../infrastructure/mongodb': (this.mongodb = { - ObjectId, - waitForDb: sinon.stub().resolves(), - }), - '../Subscription/FeaturesUpdater': (this.FeaturesUpdater = { - refreshFeatures: sinon.stub(), - }), - './InstitutionsAPI': (this.InstitutionsAPI = { - promises: { - getUsersNeedingReconfirmationsLapsedProcessed: sinon.stub(), - sendUsersWithReconfirmationsLapsedProcessed: sinon.stub(), - }, - }), - }, - } - ) - }) - - describe('userId list', function () { - it('should throw an error if IDs not an array', async function () { - let error - try { - await this.InstitutionsReconfirmationHandler.processLapsed() - } catch (e) { - error = e - } - expect(error).to.exist - expect(error.message).to.equal('users is not an array') - }) - it('should throw an error if IDs not valid ObjectIds', async function () { - this.InstitutionsAPI.promises.getUsersNeedingReconfirmationsLapsedProcessed.resolves( - { - data: { users: ['not an objectid'] }, - } - ) - let error - try { - await this.InstitutionsReconfirmationHandler.processLapsed() - } catch (e) { - error = e - } - expect(error).to.exist - expect(error.message).to.equal('user ID not valid') - }) - }) - - it('should log users that have refreshFeatures errors', async function () { - const anError = new Error('oops') - const aUserId = '5efb8b6e9b647b0027e4c0b0' - this.FeaturesUpdater.refreshFeatures.yields(anError) - this.InstitutionsAPI.promises.getUsersNeedingReconfirmationsLapsedProcessed.resolves( - { - data: { users: [aUserId] }, - } - ) - this.InstitutionsAPI.promises.sendUsersWithReconfirmationsLapsedProcessed.resolves() - let error, result - try { - result = await this.InstitutionsReconfirmationHandler.processLapsed() - } catch (e) { - error = e - } - expect(error).to.not.exist - expect(result.failedToRefresh.length).to.equal(1) - expect(result.failedToRefresh[0]).to.equal(aUserId) - expect(result.refreshedUsers.length).to.equal(0) - }) - - it('should log but not return errors from sendUsersWithReconfirmationsLapsedProcessed', async function () { - const anError = new Error('oops') - const aUserId = '5efb8b6e9b647b0027e4c0b0' - this.FeaturesUpdater.refreshFeatures.yields() - this.InstitutionsAPI.promises.getUsersNeedingReconfirmationsLapsedProcessed.resolves( - { - data: { users: [aUserId] }, - } - ) - this.InstitutionsAPI.promises.sendUsersWithReconfirmationsLapsedProcessed.rejects( - anError - ) - let error, result - try { - result = await this.InstitutionsReconfirmationHandler.processLapsed() - } catch (e) { - error = e - } - expect(error).to.not.exist - expect(result.refreshedUsers.length).to.equal(1) - expect(result.refreshedUsers[0]).to.equal(aUserId) - expect(result.failedToRefresh.length).to.equal(0) - }) -}) diff --git a/services/web/test/unit/src/Notifications/NotificationsBuilderTests.js b/services/web/test/unit/src/Notifications/NotificationsBuilderTests.js index 3a810303b6..3ad158038f 100644 --- a/services/web/test/unit/src/Notifications/NotificationsBuilderTests.js +++ b/services/web/test/unit/src/Notifications/NotificationsBuilderTests.js @@ -1,4 +1,5 @@ const SandboxedModule = require('sandboxed-module') +const { expect } = require('chai') const sinon = require('sinon') const modulePath = require('path').join( __dirname, @@ -8,20 +9,55 @@ const modulePath = require('path').join( describe('NotificationsBuilder', function () { const userId = '123nd3ijdks' - describe('ipMatcherAffiliation', function () { - beforeEach(function () { - this.handler = { createNotification: sinon.stub().callsArgWith(6) } - this.settings = { apis: { v1: { url: 'v1.url', user: '', pass: '' } } } - this.request = sinon.stub() - this.controller = SandboxedModule.require(modulePath, { - requires: { - './NotificationsHandler': this.handler, - 'settings-sharelatex': this.settings, - request: this.request, - }, + beforeEach(function () { + this.handler = { createNotification: sinon.stub().callsArgWith(6) } + this.settings = { apis: { v1: { url: 'v1.url', user: '', pass: '' } } } + this.request = sinon.stub() + this.controller = SandboxedModule.require(modulePath, { + requires: { + './NotificationsHandler': this.handler, + 'settings-sharelatex': this.settings, + request: this.request, + }, + }) + }) + + describe('dropboxUnlinkedDueToLapsedReconfirmation', function (done) { + it('should create the notification', function (done) { + this.controller + .dropboxUnlinkedDueToLapsedReconfirmation(userId) + .create(error => { + expect(error).to.not.exist + expect(this.handler.createNotification).to.have.been.calledWith( + userId, + 'drobox-unlinked-due-to-lapsed-reconfirmation', + 'notification_dropbox_unlinked_due_to_lapsed_reconfirmation', + {}, + null, + true + ) + done() + }) + }) + describe('NotificationsHandler error', function () { + let anError + beforeEach(function () { + anError = new Error('oops') + this.handler.createNotification.yields(anError) + }) + it('should return errors from NotificationsHandler', function (done) { + this.controller + .dropboxUnlinkedDueToLapsedReconfirmation(userId) + .create(error => { + expect(error).to.exist + expect(error).to.deep.equal(anError) + done() + }) }) }) + }) + describe('ipMatcherAffiliation', function () { describe('with portal and with SSO', function () { beforeEach(function () { this.body = { diff --git a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js index 6b23700466..def1ec1224 100644 --- a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js @@ -170,7 +170,7 @@ describe('FeaturesUpdater', function () { }) it('should fire module hook to unlink dropbox', function () { this.Modules.hooks.fire - .calledWith('removeDropbox', this.user._id) + .calledWith('removeDropbox', this.user._id, 'test') .should.equal(true) }) }) diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandlerTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandlerTests.js index 0c08a1576f..5fd46ad32d 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandlerTests.js +++ b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateHandlerTests.js @@ -376,7 +376,8 @@ function expectDropboxUnlinked() { it('unlinks Dropbox', function () { expect(this.Modules.hooks.fire).to.have.been.calledWith( 'removeDropbox', - this.userId + this.userId, + 'duplicate-projects' ) })