Merge pull request #6375 from overleaf/jel-reconfirm-check

[web] Use v1 date for reconfirm notification check

GitOrigin-RevId: e14f1b6a1a6ab629628858d962a3757a6078cf79

services/web/app/src/Features/User/UserGetter.js

@@ -42,28 +42,17 @@ function _pastReconfirmDate(lastDayToReconfirm) {
   return moment(lastDayToReconfirm).isBefore()
 }
 
-function _emailInReconfirmNotificationPeriod(
-  lastDayToReconfirm,
-  cachedPastReconfirmDate
-) {
+function _emailInReconfirmNotificationPeriod(cachedLastDayToReconfirm) {
   const globalReconfirmPeriod = settings.reconfirmNotificationDays
 
-  if (!globalReconfirmPeriod || !lastDayToReconfirm) return false
+  if (!globalReconfirmPeriod || !cachedLastDayToReconfirm) return false
 
-  const notificationStarts = moment(lastDayToReconfirm).subtract(
+  const notificationStarts = moment(cachedLastDayToReconfirm).subtract(
     globalReconfirmPeriod,
     'days'
   )
 
-  let inNotificationPeriod = moment().isAfter(notificationStarts)
-
-  if (!inNotificationPeriod && cachedPastReconfirmDate) {
-    // show notification if cached date is past,
-    // even if non-cached date is not past
-    inNotificationPeriod = true
-  }
-
-  return inNotificationPeriod
+  return moment().isAfter(notificationStarts)
 }
 
 async function getUserFullEmails(userId) {
@@ -271,8 +260,7 @@ const decorateFullEmails = (
       }
       const pastReconfirmDate = _pastReconfirmDate(lastDayToReconfirm)
       const inReconfirmNotificationPeriod = _emailInReconfirmNotificationPeriod(
-        lastDayToReconfirm,
-        cachedPastReconfirmDate
+        cachedLastDayToReconfirm
       )
       emailData.affiliation = {
         institution,

services/web/test/acceptance/src/helpers/UserHelper.js

@@ -1,6 +1,7 @@
 const { expect } = require('chai')
 const AuthenticationManager = require('../../../../app/src/Features/Authentication/AuthenticationManager')
 const Settings = require('@overleaf/settings')
+const InstitutionsAPI = require('../../../../app/src/Features/Institutions/InstitutionsAPI')
 const UserCreator = require('../../../../app/src/Features/User/UserCreator')
 const UserGetter = require('../../../../app/src/Features/User/UserGetter')
 const UserUpdater = require('../../../../app/src/Features/User/UserUpdater')
@@ -335,6 +336,9 @@ class UserHelper {
       },
     }
     await UserUpdater.promises.updateUser(query, update)
+    await InstitutionsAPI.promises.addAffiliation(userId, email, {
+      confirmedAt: date,
+    })
   }
 
   async changeConfirmedToNotificationPeriod(

services/web/test/acceptance/src/mocks/MockV1Api.js

@@ -1,4 +1,5 @@
 const AbstractMockApi = require('./AbstractMockApi')
+const moment = require('moment')
 const sinon = require('sinon')
 
 class MockV1Api extends AbstractMockApi {
@@ -91,7 +92,7 @@ class MockV1Api extends AbstractMockApi {
     )
   }
 
-  addAffiliation(userId, email, entitlement) {
+  addAffiliation(userId, email, entitlement, confirmedAt) {
     let newAffiliation = true
     const institution = {}
     if (!email) return
@@ -133,6 +134,17 @@ class MockV1Api extends AbstractMockApi {
         }
       })
     }
+
+    if (confirmedAt) {
+      this.affiliations[userId].forEach(affiliation => {
+        if (affiliation.email === email) {
+          if (!affiliation.cached_confirmed_at) {
+            affiliation.cached_confirmed_at = confirmedAt
+          }
+          affiliation.cached_reconfirmed_at = confirmedAt
+        }
+      })
+    }
   }
 
   setDocExported(token, info) {
@@ -235,6 +247,18 @@ class MockV1Api extends AbstractMockApi {
           ) {
             affiliation.licence = 'pro_plus'
           }
+
+          if (
+            institutionData.maxConfirmationMonths &&
+            affiliation.cached_reconfirmed_at
+          ) {
+            const lastDayToReconfirm = moment(
+              affiliation.cached_reconfirmed_at
+            ).add(institutionData.maxConfirmationMonths, 'months')
+            affiliation.last_day_to_reconfirm = lastDayToReconfirm.toDate()
+            affiliation.past_reconfirm_date = lastDayToReconfirm.isBefore()
+          }
+
           return affiliation
         }
       )
@@ -245,7 +269,8 @@ class MockV1Api extends AbstractMockApi {
       this.addAffiliation(
         req.params.userId,
         req.body.email,
-        req.body.entitlement
+        req.body.entitlement,
+        req.body.confirmedAt
       )
       res.sendStatus(201)
     })

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

@@ -29,6 +29,7 @@ describe('InstitutionsGetter', function () {
       confirmedAt: new Date(),
       affiliation: {
         institution: { id: 456, confirmed: true },
+        cachedPastReconfirmDate: false,
         pastReconfirmDate: false,
       },
     }
@@ -36,6 +37,7 @@ describe('InstitutionsGetter', function () {
       confirmedAt: new Date('2000-01-01'),
       affiliation: {
         institution: { id: 135, confirmed: true },
+        cachedPastReconfirmDate: true,
         pastReconfirmDate: true,
       },
     }
@@ -44,6 +46,7 @@ describe('InstitutionsGetter', function () {
       affiliation: {
         licence: 'pro_plus',
         institution: { id: 777, confirmed: true },
+        cachedPastReconfirmDate: false,
         pastReconfirmDate: false,
       },
     }
@@ -52,6 +55,7 @@ describe('InstitutionsGetter', function () {
       affiliation: {
         licence: 'pro_plus',
         institution: { id: 888, confirmed: true },
+        cachedPastReconfirmDate: true,
         pastReconfirmDate: true,
       },
     }
@@ -59,35 +63,64 @@ describe('InstitutionsGetter', function () {
       confirmedAt: null,
       affiliation: {
         licence: 'pro_plus',
-        institution: { id: 123, confirmed: true, pastReconfirmDate: false },
+        institution: {
+          id: 123,
+          confirmed: true,
+          cachedPastReconfirmDate: false,
+          pastReconfirmDate: false,
+        },
       },
     }
     this.unconfirmedDomainLicensedAffiliation = {
       confirmedAt: new Date(),
       affiliation: {
         licence: 'pro_plus',
-        institution: { id: 789, confirmed: false, pastReconfirmDate: false },
+        institution: {
+          id: 789,
+          confirmed: false,
+          cachedPastReconfirmDate: false,
+          pastReconfirmDate: false,
+        },
       },
     }
     this.userEmails = [
       {
         confirmedAt: null,
         affiliation: {
-          institution: { id: 123, confirmed: true, pastReconfirmDate: false },
+          institution: {
+            id: 123,
+            confirmed: true,
+            cachedPastReconfirmDate: false,
+            pastReconfirmDate: false,
+          },
         },
       },
       this.confirmedAffiliation,
       this.confirmedAffiliation,
       this.confirmedAffiliationPastReconfirmation,
-      { confirmedAt: new Date(), affiliation: null, pastReconfirmDate: false },
       {
         confirmedAt: new Date(),
-        affiliation: { institution: null, pastReconfirmDate: false },
+        affiliation: null,
+        cachedPastReconfirmDate: false,
+        pastReconfirmDate: false,
+      },
+      {
+        confirmedAt: new Date(),
+        affiliation: {
+          institution: null,
+          cachedPastReconfirmDate: false,
+          pastReconfirmDate: false,
+        },
       },
       {
         confirmedAt: new Date(),
         affiliation: {
-          institution: { id: 789, confirmed: false, pastReconfirmDate: false },
+          institution: {
+            id: 789,
+            confirmed: false,
+            cachedPastReconfirmDate: false,
+            pastReconfirmDate: false,
+          },
         },
       },
     ]

services/web/test/unit/src/User/UserGetterTests.js

@@ -328,6 +328,19 @@ describe('UserGetter', function () {
           },
         ]
         it('should flag inReconfirmNotificationPeriod for all affiliations in period', function (done) {
+          const { maxConfirmationMonths } = institutionNonSSO
+          const confirmed1 = moment()
+            .subtract(maxConfirmationMonths + 2, 'months')
+            .toDate()
+          const lastDayToReconfirm1 = moment(confirmed1)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
+          const confirmed2 = moment()
+            .subtract(maxConfirmationMonths + 1, 'months')
+            .toDate()
+          const lastDayToReconfirm2 = moment(confirmed2)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
           const user = {
             _id: '12390i',
             email: email1,
@@ -335,27 +348,20 @@ describe('UserGetter', function () {
               {
                 email: email1,
                 reversedHostname: 'moc.noitailiffa-elpmaxe',
-                confirmedAt: moment()
-                  .subtract(
-                    institutionNonSSO.maxConfirmationMonths + 2,
-                    'months'
-                  )
-                  .toDate(),
+                confirmedAt: confirmed1,
                 default: true,
               },
               {
                 email: email2,
                 reversedHostname: 'moc.noitailiffa-elpmaxe',
-                confirmedAt: moment()
-                  .subtract(
-                    institutionNonSSO.maxConfirmationMonths + 1,
-                    'months'
-                  )
-                  .toDate(),
+                confirmedAt: confirmed2,
               },
             ],
           }
-          this.getUserAffiliations.resolves(affiliationsData)
+          const affiliations = [...affiliationsData]
+          affiliations[0].last_day_to_reconfirm = lastDayToReconfirm1
+          affiliations[1].last_day_to_reconfirm = lastDayToReconfirm2
+          this.getUserAffiliations.resolves(affiliations)
           this.UserGetter.promises.getUser = sinon.stub().resolves(user)
           this.UserGetter.getUserFullEmails(
             this.fakeUser._id,
@@ -372,10 +378,18 @@ describe('UserGetter', function () {
           )
         })
         it('should not flag affiliations outside of notification period', function (done) {
-          const aboutToBeWithinPeriod = moment()
-            .subtract(institutionNonSSO.maxConfirmationMonths, 'months')
+          const { maxConfirmationMonths } = institutionNonSSO
+          const confirmed1 = new Date()
+          const lastDayToReconfirm1 = moment(confirmed1)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
+          const confirmed2 = moment()
+            .subtract(maxConfirmationMonths, 'months')
             .add(15, 'days')
             .toDate() // expires in 15 days
+          const lastDayToReconfirm2 = moment(confirmed2)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
           const user = {
             _id: '12390i',
             email: email1,
@@ -383,17 +397,20 @@ describe('UserGetter', function () {
               {
                 email: email1,
                 reversedHostname: 'moc.noitailiffa-elpmaxe',
-                confirmedAt: new Date(),
+                confirmedAt: confirmed1,
                 default: true,
               },
               {
                 email: email2,
                 reversedHostname: 'moc.noitailiffa-elpmaxe',
-                confirmedAt: aboutToBeWithinPeriod,
+                confirmedAt: confirmed2,
               },
             ],
           }
-          this.getUserAffiliations.resolves(affiliationsData)
+          const affiliations = [...affiliationsData]
+          affiliations[0].last_day_to_reconfirm = lastDayToReconfirm1
+          affiliations[1].last_day_to_reconfirm = lastDayToReconfirm2
+          this.getUserAffiliations.resolves(affiliations)
           this.UserGetter.promises.getUser = sinon.stub().resolves(user)
           this.UserGetter.getUserFullEmails(
             this.fakeUser._id,
@@ -413,37 +430,14 @@ describe('UserGetter', function () {
 
       describe('SSO institutions', function () {
         it('should flag only linked email, if in notification period', function (done) {
+          const { maxConfirmationMonths } = institutionSSO
           const email1 = 'email1@sso.bar'
           const email2 = 'email2@sso.bar'
           const email3 = 'email3@sso.bar'
-
-          const affiliationsData = [
-            {
-              email: email1,
-              role: 'Prof',
-              department: 'Maths',
-              inferred: false,
-              licence: 'pro_plus',
-              institution: institutionSSO,
-            },
-            {
-              email: email2,
-              role: 'Prof',
-              department: 'Maths',
-              inferred: false,
-              licence: 'pro_plus',
-              institution: institutionSSO,
-            },
-            {
-              email: email3,
-              role: 'Prof',
-              department: 'Maths',
-              inferred: false,
-              licence: 'pro_plus',
-              institution: institutionSSO,
-            },
-          ]
-
+          const reconfirmedAt = new Date('2019-09-24')
+          const lastDayToReconfirm = moment(reconfirmedAt)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
           const user = {
             _id: '12390i',
             email: email1,
@@ -452,21 +446,21 @@ describe('UserGetter', function () {
                 email: email1,
                 reversedHostname: 'rab.oss',
                 confirmedAt: new Date('2019-09-24'),
-                reconfirmedAt: new Date('2019-09-24'),
+                reconfirmedAt: reconfirmedAt,
                 default: true,
               },
               {
                 email: email2,
                 reversedHostname: 'rab.oss',
                 confirmedAt: new Date('2019-09-24'),
-                reconfirmedAt: new Date('2019-09-24'),
+                reconfirmedAt: reconfirmedAt,
                 samlProviderId: institutionSSO.id,
               },
               {
                 email: email3,
                 reversedHostname: 'rab.oss',
                 confirmedAt: new Date('2019-09-24'),
-                reconfirmedAt: new Date('2019-09-24'),
+                reconfirmedAt: reconfirmedAt,
               },
             ],
             samlIdentifiers: [
@@ -476,7 +470,36 @@ describe('UserGetter', function () {
               },
             ],
           }
-          this.getUserAffiliations.resolves(affiliationsData)
+          const affiliations = [
+            {
+              email: email1,
+              role: 'Prof',
+              department: 'Maths!',
+              inferred: false,
+              licence: 'pro_plus',
+              institution: institutionSSO,
+              last_day_to_reconfirm: lastDayToReconfirm,
+            },
+            {
+              email: email2,
+              role: 'Prof',
+              department: 'Maths',
+              inferred: false,
+              licence: 'pro_plus',
+              institution: institutionSSO,
+              last_day_to_reconfirm: lastDayToReconfirm,
+            },
+            {
+              email: email3,
+              role: 'Prof',
+              department: 'Maths',
+              inferred: false,
+              licence: 'pro_plus',
+              institution: institutionSSO,
+              last_day_to_reconfirm: lastDayToReconfirm,
+            },
+          ]
+          this.getUserAffiliations.resolves(affiliations)
           this.UserGetter.promises.getUser = sinon.stub().resolves(user)
           this.UserGetter.getUserFullEmails(
             this.fakeUser._id,
@@ -499,10 +522,19 @@ describe('UserGetter', function () {
 
       describe('multiple institution affiliations', function () {
         it('should flag each institution', function (done) {
+          const { maxConfirmationMonths } = institutionSSO
           const email1 = 'email1@sso.bar'
           const email2 = 'email2@sso.bar'
           const email3 = 'email3@foo.bar'
           const email4 = 'email4@foo.bar'
+          const confirmed1 = new Date('2019-09-24T20:25:08.503Z')
+          const lastDayToReconfirm1 = moment(confirmed1)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
+          const confirmed2 = new Date('2019-10-24T20:25:08.503Z')
+          const lastDayToReconfirm2 = moment(confirmed2)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
 
           const affiliationsData = [
             {
@@ -512,6 +544,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionSSO,
+              last_day_to_reconfirm: lastDayToReconfirm1,
             },
             {
               email: email2,
@@ -520,6 +553,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionSSO,
+              last_day_to_reconfirm: lastDayToReconfirm1,
             },
             {
               email: email3,
@@ -528,6 +562,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionNonSSO,
+              last_day_to_reconfirm: lastDayToReconfirm2,
             },
             {
               email: email4,
@@ -536,6 +571,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionNonSSO,
+              last_day_to_reconfirm: lastDayToReconfirm1,
             },
           ]
           const user = {
@@ -545,24 +581,24 @@ describe('UserGetter', function () {
               {
                 email: email1,
                 reversedHostname: 'rab.oss',
-                confirmedAt: '2019-09-24T20:25:08.503Z',
+                confirmedAt: confirmed1,
                 default: true,
               },
               {
                 email: email2,
                 reversedHostname: 'rab.oss',
-                confirmedAt: new Date('2019-09-24T20:25:08.503Z'),
+                confirmedAt: confirmed1,
                 samlProviderId: institutionSSO.id,
               },
               {
                 email: email3,
                 reversedHostname: 'rab.oof',
-                confirmedAt: new Date('2019-10-24T20:25:08.503Z'),
+                confirmedAt: confirmed2,
               },
               {
                 email: email4,
                 reversedHostname: 'rab.oof',
-                confirmedAt: new Date('2019-09-24T20:25:08.503Z'),
+                confirmedAt: confirmed1,
               },
             ],
             samlIdentifiers: [
@@ -572,6 +608,7 @@ describe('UserGetter', function () {
               },
             ],
           }
+
           this.getUserAffiliations.resolves(affiliationsData)
           this.UserGetter.promises.getUser = sinon.stub().resolves(user)
           this.UserGetter.getUserFullEmails(
@@ -598,9 +635,31 @@ describe('UserGetter', function () {
 
       describe('reconfirmedAt', function () {
         it('only use confirmedAt when no reconfirmedAt', function (done) {
+          const { maxConfirmationMonths } = institutionSSO
           const email1 = 'email1@foo.bar'
+          const reconfirmed1 = moment().subtract(
+            maxConfirmationMonths * 3,
+            'months'
+          )
+          const lastDayToReconfirm1 = moment(reconfirmed1)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
           const email2 = 'email2@foo.bar'
+          const reconfirmed2 = moment().subtract(
+            maxConfirmationMonths * 2,
+            'months'
+          )
+          const lastDayToReconfirm2 = moment(reconfirmed2)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
           const email3 = 'email3@foo.bar'
+          const reconfirmed3 = moment().subtract(
+            maxConfirmationMonths * 4,
+            'months'
+          )
+          const lastDayToReconfirm3 = moment(reconfirmed3)
+            .add(maxConfirmationMonths, 'months')
+            .toDate()
 
           const affiliationsData = [
             {
@@ -610,6 +669,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionNonSSO,
+              last_day_to_reconfirm: lastDayToReconfirm1,
             },
             {
               email: email2,
@@ -618,6 +678,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionNonSSO,
+              last_day_to_reconfirm: lastDayToReconfirm2,
             },
             {
               email: email3,
@@ -626,6 +687,7 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionNonSSO,
+              last_day_to_reconfirm: lastDayToReconfirm3,
             },
           ]
           const user = {
@@ -639,10 +701,7 @@ describe('UserGetter', function () {
                   institutionSSO.maxConfirmationMonths * 2,
                   'months'
                 ),
-                reconfirmedAt: moment().subtract(
-                  institutionSSO.maxConfirmationMonths * 3,
-                  'months'
-                ),
+                reconfirmedAt: reconfirmed1,
                 default: true,
               },
               {
@@ -652,10 +711,7 @@ describe('UserGetter', function () {
                   institutionSSO.maxConfirmationMonths * 3,
                   'months'
                 ),
-                reconfirmedAt: moment().subtract(
-                  institutionSSO.maxConfirmationMonths * 2,
-                  'months'
-                ),
+                reconfirmedAt: reconfirmed2,
               },
               {
                 email: email3,
@@ -664,10 +720,7 @@ describe('UserGetter', function () {
                   institutionSSO.maxConfirmationMonths * 4,
                   'months'
                 ),
-                reconfirmedAt: moment().subtract(
-                  institutionSSO.maxConfirmationMonths * 4,
-                  'months'
-                ),
+                reconfirmedAt: reconfirmed3,
               },
             ],
           }
@@ -695,8 +748,9 @@ describe('UserGetter', function () {
       describe('before reconfirmation period expires and within reconfirmation notification period', function () {
         const email = 'leonard@example-affiliation.com'
         it('should flag the email', function (done) {
+          const { maxConfirmationMonths } = institutionNonSSO
           const confirmedAt = moment()
-            .subtract(institutionNonSSO.maxConfirmationMonths, 'months')
+            .subtract(maxConfirmationMonths, 'months')
             .subtract(14, 'days')
             .toDate() // expires in 14 days
           const affiliationsData = [
@@ -707,6 +761,9 @@ describe('UserGetter', function () {
               inferred: false,
               licence: 'pro_plus',
               institution: institutionNonSSO,
+              last_day_to_reconfirm: moment(confirmedAt)
+                .add(maxConfirmationMonths, 'months')
+                .toDate(),
             },
           ]
           const user = {
@@ -844,6 +901,45 @@ describe('UserGetter', function () {
         )
       })
 
+      it('should flag to show notification if v1 shows as reconfirmation upcoming but v2 does not', function (done) {
+        const email = 'abc123@test.com'
+        const { maxConfirmationMonths } = institutionNonSSO
+        const affiliationsData = [
+          {
+            email,
+            licence: 'free',
+            institution: institutionNonSSO,
+            last_day_to_reconfirm: moment()
+              .subtract(maxConfirmationMonths, 'months')
+              .add(3, 'day')
+              .toDate(),
+          },
+        ]
+        const user = {
+          _id: '12390i',
+          email,
+          emails: [
+            {
+              email,
+              confirmedAt: new Date(),
+              default: true,
+            },
+          ],
+        }
+        this.getUserAffiliations.resolves(affiliationsData)
+        this.UserGetter.promises.getUser = sinon.stub().resolves(user)
+        this.UserGetter.getUserFullEmails(
+          this.fakeUser._id,
+          (error, fullEmails) => {
+            expect(error).to.not.exist
+            expect(
+              fullEmails[0].affiliation.inReconfirmNotificationPeriod
+            ).to.equal(true)
+            done()
+          }
+        )
+      })
+
       describe('cachedLastDayToReconfirm', function () {
         const email = 'abc123@test.com'
         const confirmedAt = new Date('2019-07-11T18:25:01.639Z')