From 2dd860c4318038c81bbc028f65b330497664c4de Mon Sep 17 00:00:00 2001 From: Jessica Lawshe Date: Tue, 2 Feb 2021 08:26:10 -0600 Subject: [PATCH] Merge pull request #3581 from overleaf/jel-dash-consolidate-emails-requests Consolidate emails requests on the dashboard GitOrigin-RevId: acfaf92dee257712e1eb3ffbf75b536fd1619e1d --- .../src/Features/Project/ProjectController.js | 31 ++++---- .../web/app/src/Features/User/UserGetter.js | 12 +++- services/web/app/views/project/list.pug | 2 +- .../web/app/views/project/list/side-bar.pug | 2 +- .../left-hand-menu-promo-controller.js | 15 ++-- .../project-list/notifications-controller.js | 9 +-- .../src/SubscriptionDashboardTests.js | 3 +- .../src/Project/ProjectControllerTests.js | 70 +++++++++---------- .../web/test/unit/src/User/UserGetterTests.js | 6 +- 9 files changed, 67 insertions(+), 83 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 29e13c98b6..5fb18baca3 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -33,7 +33,6 @@ const NotificationsBuilder = require('../Notifications/NotificationsBuilder') const { V1ConnectionError } = require('../Errors/Errors') const Features = require('../../infrastructure/Features') const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandler') -const { getUserAffiliations } = require('../Institutions/InstitutionsAPI') const UserController = require('../User/UserController') const AnalyticsManager = require('../Analytics/AnalyticsManager') const Modules = require('../../infrastructure/Modules') @@ -416,18 +415,6 @@ const ProjectController = { cb ) }, - userAffiliations(cb) { - if (!Features.hasFeature('affiliations')) { - return cb(null, []) - } - getUserAffiliations(userId, (error, affiliations) => { - if (error && error instanceof V1ConnectionError) { - noV1Connection = true - return cb(null, []) - } - cb(error, affiliations) - }) - }, userEmailsData(cb) { const result = { list: [], allInReconfirmNotificationPeriods: [] } @@ -463,12 +450,17 @@ const ProjectController = { OError.tag(err, 'error getting data for project list page') return next(err) } - const { - notifications, - user, - userAffiliations, - userEmailsData - } = results + const { notifications, user, userEmailsData } = results + + const userEmails = userEmailsData.list || [] + + const userAffiliations = userEmails + .filter(emailData => !!emailData.affiliation) + .map(emailData => { + const result = emailData.affiliation + result.email = emailData.email + return result + }) const { allInReconfirmNotificationPeriods } = userEmailsData @@ -600,6 +592,7 @@ const ProjectController = { portalTemplates, user, userAffiliations, + userEmails, hasSubscription: results.hasSubscription, institutionLinkingError, warnings, diff --git a/services/web/app/src/Features/User/UserGetter.js b/services/web/app/src/Features/User/UserGetter.js index 1910c9fc3c..e401320ca7 100644 --- a/services/web/app/src/Features/User/UserGetter.js +++ b/services/web/app/src/Features/User/UserGetter.js @@ -194,7 +194,14 @@ var decorateFullEmails = ( aff => aff.email === emailData.email ) if (affiliation) { - const { institution, inferred, role, department, licence } = affiliation + const { + institution, + inferred, + role, + department, + licence, + portal + } = affiliation const inReconfirmNotificationPeriod = _emailInReconfirmNotificationPeriod( emailData, institution @@ -205,7 +212,8 @@ var decorateFullEmails = ( inReconfirmNotificationPeriod, role, department, - licence + licence, + portal } } diff --git a/services/web/app/views/project/list.pug b/services/web/app/views/project/list.pug index 9b2a4d754c..c5d4da0231 100644 --- a/services/web/app/views/project/list.pug +++ b/services/web/app/views/project/list.pug @@ -5,7 +5,7 @@ block vars block content script#data(type="application/json"). - !{StringHelper.stringifyJsonForScript({ projects, tags, notifications, notificationsInstitution })} + !{StringHelper.stringifyJsonForScript({ projects, tags, notifications, notificationsInstitution, userAffiliations, userEmails })} script(type="text/javascript"). window.data = JSON.parse(document.querySelector("#data").text); diff --git a/services/web/app/views/project/list/side-bar.pug b/services/web/app/views/project/list/side-bar.pug index 6ce0649aef..259f0b23aa 100644 --- a/services/web/app/views/project/list/side-bar.pug +++ b/services/web/app/views/project/list/side-bar.pug @@ -139,7 +139,7 @@ span(ng-controller="LeftHandMenuPromoController", ng-cloak) .row-spaced#userProfileInformation(ng-if="hasProjects") - div(ng-show="userEmails.length > 0 && userAffiliations.length == 0", ng-cloak) + div(ng-hide="withAffiliations", ng-cloak) hr .text-centered.user-profile p Are you affiliated with an institution? diff --git a/services/web/frontend/js/main/project-list/left-hand-menu-promo-controller.js b/services/web/frontend/js/main/project-list/left-hand-menu-promo-controller.js index 9ec06911ba..513c7faed3 100644 --- a/services/web/frontend/js/main/project-list/left-hand-menu-promo-controller.js +++ b/services/web/frontend/js/main/project-list/left-hand-menu-promo-controller.js @@ -17,17 +17,10 @@ export default App.controller('LeftHandMenuPromoController', function( } const _userHasNoAffiliation = function() { - $scope.userEmails = [] - $scope.userAffiliations = [] - return UserAffiliationsDataService.getUserEmails().then(function(emails) { - $scope.userEmails = emails - $scope.userAffiliations = emails - .filter(email => email.affiliation) - .map(email => email.affiliation) - $scope.userOnPayingUniversity = $scope.userAffiliations.some( - affiliation => affiliation.licence && affiliation.licence !== 'free' - ) - }) + $scope.withAffiliations = window.data.userAffiliations.length > 0 + $scope.userOnPayingUniversity = window.data.userAffiliations.some( + affiliation => affiliation.licence && affiliation.licence !== 'free' + ) } _userHasNoAffiliation() diff --git a/services/web/frontend/js/main/project-list/notifications-controller.js b/services/web/frontend/js/main/project-list/notifications-controller.js index d1d245e8e8..1f1912c0f2 100644 --- a/services/web/frontend/js/main/project-list/notifications-controller.js +++ b/services/web/frontend/js/main/project-list/notifications-controller.js @@ -74,7 +74,7 @@ App.controller('EmailNotificationController', function( $http, UserAffiliationsDataService ) { - $scope.userEmails = [] + $scope.userEmails = window.data.userEmails const _ssoAvailable = email => { if (!ExposedSettings.hasSamlFeature) return false if (email.samlProviderId) return true @@ -101,13 +101,6 @@ App.controller('EmailNotificationController', function( userEmail.hide = false } - const _getUserEmails = () => - UserAffiliationsDataService.getUserEmails().then(function(emails) { - $scope.userEmails = emails - $scope.$emit('project-list:notifications-received') - }) - _getUserEmails() - $scope.resendConfirmationEmail = function(userEmail) { userEmail.confirmationInflight = true userEmail.error = false diff --git a/services/web/test/acceptance/src/SubscriptionDashboardTests.js b/services/web/test/acceptance/src/SubscriptionDashboardTests.js index cc8795b906..cc1591d514 100644 --- a/services/web/test/acceptance/src/SubscriptionDashboardTests.js +++ b/services/web/test/acceptance/src/SubscriptionDashboardTests.js @@ -520,7 +520,8 @@ describe('Subscriptions', function() { institution: { name: 'Stanford', confirmed: true - } + }, + portal: undefined } ]) }) diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index aff2fb7609..cb304951b7 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -114,18 +114,6 @@ describe('ProjectController', function() { this.TpdsProjectFlusher = { flushProjectToTpdsIfNeeded: sinon.stub().yields() } - this.getUserAffiliations = sinon.stub().callsArgWith(1, null, [ - { - email: 'test@overleaf.com', - institution: { - id: 1, - confirmed: true, - name: 'Overleaf', - ssoBeta: false, - ssoEnabled: true - } - } - ]) this.Metrics = { Timer: class { done() {} @@ -173,9 +161,6 @@ describe('ProjectController', function() { '../User/UserGetter': this.UserGetter, '../BrandVariations/BrandVariationsHandler': this .BrandVariationsHandler, - '../Institutions/InstitutionsAPI': { - getUserAffiliations: this.getUserAffiliations - }, '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher, '../../models/Project': {}, '../Analytics/AnalyticsManager': { recordEvent: () => {} }, @@ -526,15 +511,6 @@ describe('ProjectController', function() { this.ProjectController.projectListPage(this.req, this.res) }) - it('should show a warning when there is an error getting affiliations from v1', function(done) { - this.getUserAffiliations.yields(new Errors.V1ConnectionError('error')) - this.res.render = (pageName, opts) => { - expect(opts.warnings).to.contain(this.connectionWarning) - done() - } - this.ProjectController.projectListPage(this.req, this.res) - }) - it('should show a warning when there is an error getting full emails due to v1', function(done) { this.UserGetter.getUserFullEmails.yields( new Errors.V1ConnectionError('error') @@ -597,6 +573,20 @@ describe('ProjectController', function() { done() }) it('should show institution SSO available notification for confirmed domains', function() { + this.UserGetter.getUserFullEmails.yields(null, [ + { + email: 'test@overleaf.com', + affiliation: { + institution: { + id: 1, + confirmed: true, + name: 'Overleaf', + ssoBeta: false, + ssoEnabled: true + } + } + } + ]) this.res.render = (pageName, opts) => { expect(opts.notificationsInstitution).to.deep.include({ email: this.institutionEmail, @@ -713,15 +703,17 @@ describe('ProjectController', function() { }) describe('for an unconfirmed domain for an SSO institution', function() { beforeEach(function(done) { - this.getUserAffiliations.yields(null, [ + this.UserGetter.getUserFullEmails.yields(null, [ { email: 'test@overleaf-uncofirmed.com', - institution: { - id: 1, - confirmed: false, - name: 'Overleaf', - ssoBeta: false, - ssoEnabled: true + affiliation: { + institution: { + id: 1, + confirmed: false, + name: 'Overleaf', + ssoBeta: false, + ssoEnabled: true + } } } ]) @@ -758,15 +750,17 @@ describe('ProjectController', function() { }) describe('Institution with SSO beta testable', function() { beforeEach(function(done) { - this.getUserAffiliations.yields(null, [ + this.UserGetter.getUserFullEmails.yields(null, [ { email: 'beta@beta.com', - institution: { - id: 2, - confirmed: true, - name: 'Beta University', - ssoBeta: true, - ssoEnabled: false + affiliation: { + institution: { + id: 2, + confirmed: true, + name: 'Beta University', + ssoBeta: true, + ssoEnabled: false + } } } ]) diff --git a/services/web/test/unit/src/User/UserGetterTests.js b/services/web/test/unit/src/User/UserGetterTests.js index 888bab7448..35fb6f48c6 100644 --- a/services/web/test/unit/src/User/UserGetterTests.js +++ b/services/web/test/unit/src/User/UserGetterTests.js @@ -174,7 +174,8 @@ describe('UserGetter', function() { name: 'University Name', isUniversity: true, confirmed: true - } + }, + portal: undefined } ] this.getUserAffiliations.resolves(affiliationsData) @@ -195,7 +196,8 @@ describe('UserGetter', function() { department: affiliationsData[0].department, role: affiliationsData[0].role, licence: affiliationsData[0].licence, - inReconfirmNotificationPeriod: false + inReconfirmNotificationPeriod: false, + portal: undefined } }, {