Merge pull request #4316 from overleaf/jel-user-confirmed-institutions

Exclude lapsed reconfirmations from confirmed affiliations

GitOrigin-RevId: 5987299ead2717abd54e313693f1cfc141915f8d

services/web/app/src/Features/Institutions/InstitutionsGetter.js

@@ -1,9 +1,35 @@
-let InstitutionsGetter
+const { callbackify } = require('util')
 const UserGetter = require('../User/UserGetter')
 const UserMembershipsHandler = require('../UserMembership/UserMembershipsHandler')
 const UserMembershipEntityConfigs = require('../UserMembership/UserMembershipEntityConfigs')
 
-module.exports = InstitutionsGetter = {
+async function _getCurrentAffiliations(userId) {
+  const fullEmails = await UserGetter.promises.getUserFullEmails(userId)
+  // current are those confirmed and not with lapsed reconfirmations
+  return fullEmails
+    .filter(
+      emailData =>
+        emailData.confirmedAt &&
+        emailData.affiliation &&
+        emailData.affiliation.institution &&
+        emailData.affiliation.institution.confirmed &&
+        !emailData.affiliation.pastReconfirmDate
+    )
+    .map(emailData => emailData.affiliation)
+}
+
+async function getCurrentInstitutionIds(userId) {
+  // current are those confirmed and not with lapsed reconfirmations
+  // only 1 record returned per current institutionId
+  const institutionIds = new Set()
+  const currentAffiliations = await _getCurrentAffiliations(userId)
+  currentAffiliations.forEach(affiliation => {
+    institutionIds.add(affiliation.institution.id)
+  })
+  return [...institutionIds]
+}
+
+const InstitutionsGetter = {
   getConfirmedAffiliations(userId, callback) {
     UserGetter.getUserFullEmails(userId, function (error, emailsData) {
       if (error) {
@@ -24,23 +50,7 @@ module.exports = InstitutionsGetter = {
     })
   },
 
-  getConfirmedInstitutions(userId, callback) {
+  getCurrentInstitutionIds: callbackify(getCurrentInstitutionIds),
-    InstitutionsGetter.getConfirmedAffiliations(
-      userId,
-      (error, confirmedAffiliations) => {
-        if (error) {
-          return callback(error)
-        }
-
-        const confirmedInstitutions = confirmedAffiliations.map(
-          confirmedAffiliation =>
-            confirmedAffiliation ? confirmedAffiliation.institution : undefined
-        )
-
-        callback(null, confirmedInstitutions)
-      }
-    )
-  },
 
   getManagedInstitutions(userId, callback) {
     UserMembershipsHandler.getEntitiesByUser(
@@ -50,3 +60,9 @@ module.exports = InstitutionsGetter = {
     )
   },
 }
+
+InstitutionsGetter.promises = {
+  getCurrentInstitutionIds,
+}
+
+module.exports = InstitutionsGetter

services/web/test/unit/src/Institutions/InstitutionsGetterTests.js

@@ -1,14 +1,3 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const SandboxedModule = require('sandboxed-module')
 const { expect } = require('chai')
 const sinon = require('sinon')
@@ -19,7 +8,12 @@ const modulePath = require('path').join(
 
 describe('InstitutionsGetter', function () {
   beforeEach(function () {
-    this.UserGetter = { getUserFullEmails: sinon.stub() }
+    this.UserGetter = {
+      getUserFullEmails: sinon.stub(),
+      promises: {
+        getUserFullEmails: sinon.stub(),
+      },
+    }
     this.InstitutionsGetter = SandboxedModule.require(modulePath, {
       requires: {
         '../User/UserGetter': this.UserGetter,
@@ -28,60 +22,72 @@ describe('InstitutionsGetter', function () {
       },
     })
 
-    return (this.userId = '12345abcde')
+    this.userId = '12345abcde'
+    this.confirmedAffiliation = {
+      confirmedAt: new Date(),
+      affiliation: {
+        institution: { id: 456, confirmed: true },
+        pastReconfirmDate: false,
+      },
+    }
+    this.confirmedAffiliationPastReconfirmation = {
+      confirmedAt: new Date('2000-01-01'),
+      affiliation: {
+        institution: { id: 135, confirmed: true },
+        pastReconfirmDate: true,
+      },
+    }
+    this.userEmails = [
+      {
+        confirmedAt: null,
+        affiliation: {
+          institution: { id: 123, confirmed: true, pastReconfirmDate: false },
+        },
+      },
+      this.confirmedAffiliation,
+      this.confirmedAffiliation,
+      this.confirmedAffiliationPastReconfirmation,
+      { confirmedAt: new Date(), affiliation: null, pastReconfirmDate: false },
+      {
+        confirmedAt: new Date(),
+        affiliation: { institution: null, pastReconfirmDate: false },
+      },
+      {
+        confirmedAt: new Date(),
+        affiliation: {
+          institution: { id: 789, confirmed: false, pastReconfirmDate: false },
+        },
+      },
+    ]
   })
 
-  describe('getConfirmedInstitutions', function () {
+  describe('getCurrentInstitutionIds', function () {
-    it('filters unconfirmed affiliations', function (done) {
+    it('filters unconfirmed affiliations, those past reconfirmation, and returns only 1 result per institution', async function () {
-      this.userEmails = [
+      this.UserGetter.promises.getUserFullEmails.resolves(this.userEmails)
-        {
+      const institutions = await this.InstitutionsGetter.promises.getCurrentInstitutionIds(
-          confirmedAt: null,
+        this.userId
-          affiliation: { institution: { id: 123, confirmed: true } },
-        },
-        {
-          confirmedAt: new Date(),
-          affiliation: { institution: { id: 456, confirmed: true } },
-        },
-        { confirmedAt: new Date(), affiliation: null },
-        { confirmedAt: new Date(), affiliation: { institution: null } },
-        {
-          confirmedAt: new Date(),
-          affiliation: { institution: { id: 789, confirmed: false } },
-        },
-      ]
-      this.UserGetter.getUserFullEmails.yields(null, this.userEmails)
-      return this.InstitutionsGetter.getConfirmedInstitutions(
-        this.userId,
-        (error, institutions) => {
-          expect(error).to.not.exist
-          institutions.length.should.equal(1)
-          institutions[0].id.should.equal(456)
-          return done()
-        }
       )
+      expect(institutions.length).to.equal(1)
+      expect(institutions[0]).to.equal(456)
     })
-
+    it('handles empty response', async function () {
-    it('should handle empty response', function (done) {
+      this.UserGetter.promises.getUserFullEmails.resolves([])
-      this.UserGetter.getUserFullEmails.yields(null, [])
+      const institutions = await this.InstitutionsGetter.promises.getCurrentInstitutionIds(
-      return this.InstitutionsGetter.getConfirmedInstitutions(
+        this.userId
-        this.userId,
-        (error, institutions) => {
-          expect(error).to.not.exist
-          institutions.length.should.equal(0)
-          return done()
-        }
       )
+      expect(institutions).to.deep.equal([])
     })
-
+    it('handles errors', async function () {
-    it('should handle error', function (done) {
+      this.UserGetter.promises.getUserFullEmails.rejects(new Error('oops'))
-      this.UserGetter.getUserFullEmails.yields(new Error('Nope'))
+      let e
-      return this.InstitutionsGetter.getConfirmedInstitutions(
+      try {
-        this.userId,
+        await this.InstitutionsGetter.promises.getCurrentInstitutionIds(
-        (error, institutions) => {
+          this.userId
-          expect(error).to.exist
+        )
-          return done()
+      } catch (error) {
-        }
+        e = error
-      )
+      }
+      expect(e.message).to.equal('oops')
     })
   })
 })