Merge pull request #4659 from overleaf/jel-em-duplicate-subscription-message

One Pro message per current commons institutions

GitOrigin-RevId: 21b74af8b9da1b6647a0384115f5d268b80e4bdf

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

@@ -29,6 +29,19 @@ async function getCurrentInstitutionIds(userId) {
   return [...institutionIds]
 }
 
+async function getCurrentInstitutionsWithLicence(userId) {
+  // current are those confirmed and not with lapsed reconfirmations
+  // only 1 record returned per current institution
+  const institutions = {}
+  const currentAffiliations = await _getCurrentAffiliations(userId)
+  currentAffiliations.forEach(affiliation => {
+    if (affiliation.licence && affiliation.licence !== 'free') {
+      institutions[affiliation.institution.id] = affiliation.institution
+    }
+  })
+  return Object.values(institutions)
+}
+
 const InstitutionsGetter = {
   getConfirmedAffiliations(userId, callback) {
     UserGetter.getUserFullEmails(userId, function (error, emailsData) {
@@ -51,6 +64,9 @@ const InstitutionsGetter = {
   },
 
   getCurrentInstitutionIds: callbackify(getCurrentInstitutionIds),
+  getCurrentInstitutionsWithLicence: callbackify(
+    getCurrentInstitutionsWithLicence
+  ),
 
   getManagedInstitutions(userId, callback) {
     UserMembershipsHandler.getEntitiesByUser(
@@ -63,6 +79,7 @@ const InstitutionsGetter = {
 
 InstitutionsGetter.promises = {
   getCurrentInstitutionIds,
+  getCurrentInstitutionsWithLicence,
 }
 
 module.exports = InstitutionsGetter

services/web/app/src/Features/Subscription/SubscriptionController.js

@@ -19,7 +19,6 @@ const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const RecurlyEventHandler = require('./RecurlyEventHandler')
 const { expressify } = require('../../util/promises')
 const OError = require('@overleaf/o-error')
-const _ = require('lodash')
 
 const SUBSCRIPTION_PAGE_SPLIT_TEST = 'subscription-page'
 
@@ -103,7 +102,7 @@ async function userSubscriptionPage(req, res) {
     personalSubscription,
     memberGroupSubscriptions,
     managedGroupSubscriptions,
-    confirmedMemberAffiliations,
+    currentInstitutionsWithLicence,
     managedInstitutions,
     managedPublishers,
     v1SubscriptionStatus,
@@ -121,11 +120,7 @@ async function userSubscriptionPage(req, res) {
     personalSubscription ||
     hasSubscription ||
     (memberGroupSubscriptions && memberGroupSubscriptions.length > 0) ||
-    (confirmedMemberAffiliations &&
+    currentInstitutionsWithLicence.length > 0
-      confirmedMemberAffiliations.length > 0 &&
-      _.find(confirmedMemberAffiliations, affiliation => {
-        return affiliation.licence && affiliation.licence !== 'free'
-      }))
   ) {
     AnalyticsManager.recordEvent(user._id, 'subscription-page-view')
   } else {
@@ -163,10 +158,10 @@ async function userSubscriptionPage(req, res) {
     personalSubscription,
     memberGroupSubscriptions,
     managedGroupSubscriptions,
-    confirmedMemberAffiliations,
     managedInstitutions,
     managedPublishers,
     v1SubscriptionStatus,
+    currentInstitutionsWithLicence,
   }
   res.render('subscriptions/dashboard', data)
 }

services/web/app/src/Features/Subscription/SubscriptionViewModelBuilder.js

@@ -88,8 +88,8 @@ function buildUsersSubscriptionViewModel(user, callback) {
       managedGroupSubscriptions(cb) {
         SubscriptionLocator.getManagedGroupSubscriptions(user, cb)
       },
-      confirmedMemberAffiliations(cb) {
+      currentInstitutionsWithLicence(cb) {
-        InstitutionsGetter.getConfirmedAffiliations(user._id, cb)
+        InstitutionsGetter.getCurrentInstitutionsWithLicence(user._id, cb)
       },
       managedInstitutions(cb) {
         InstitutionsGetter.getManagedInstitutions(user._id, cb)
@@ -117,7 +117,7 @@ function buildUsersSubscriptionViewModel(user, callback) {
         personalSubscription,
         memberGroupSubscriptions,
         managedGroupSubscriptions,
-        confirmedMemberAffiliations,
+        currentInstitutionsWithLicence,
         managedInstitutions,
         managedPublishers,
         v1SubscriptionStatus,
@@ -125,15 +125,13 @@ function buildUsersSubscriptionViewModel(user, callback) {
         recurlyCoupons,
         plan,
       } = results
+
       if (memberGroupSubscriptions == null) {
         memberGroupSubscriptions = []
       }
       if (managedGroupSubscriptions == null) {
         managedGroupSubscriptions = []
       }
-      if (confirmedMemberAffiliations == null) {
-        confirmedMemberAffiliations = []
-      }
       if (managedInstitutions == null) {
         managedInstitutions = []
       }
@@ -269,7 +267,7 @@ function buildUsersSubscriptionViewModel(user, callback) {
         personalSubscription,
         managedGroupSubscriptions,
         memberGroupSubscriptions,
-        confirmedMemberAffiliations,
+        currentInstitutionsWithLicence,
         managedInstitutions,
         managedPublishers,
         v1SubscriptionStatus,

services/web/app/views/subscriptions/dashboard.pug

@@ -43,7 +43,6 @@ block content
 							-hasDisplayedSubscription = true
 							include ./dashboard/_group_memberships
 
-						if (confirmedMemberAffiliations && confirmedMemberAffiliations.length > 0)
 						include ./dashboard/_institution_memberships
 
 						if (v1SubscriptionStatus)

services/web/app/views/subscriptions/dashboard/_institution_memberships.pug

@@ -1,5 +1,4 @@
-each affiliation in confirmedMemberAffiliations
+each institution in currentInstitutionsWithLicence
-	if (affiliation.licence && affiliation.licence != 'free')
 	-hasDisplayedSubscription = true
 	p
 		| You have a
@@ -8,6 +7,5 @@ each affiliation in confirmedMemberAffiliations
 		|
 		| #{settings.appName} account as a confirmed member of
 		|
-			strong(ng-non-bindable)= affiliation.institution.name
+		strong(ng-non-bindable)= institution.name
 	hr
-

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

@@ -37,6 +37,36 @@ describe('InstitutionsGetter', function () {
         pastReconfirmDate: true,
       },
     }
+    this.licencedAffiliation = {
+      confirmedAt: new Date(),
+      affiliation: {
+        licence: 'pro_plus',
+        institution: { id: 777, confirmed: true },
+        pastReconfirmDate: false,
+      },
+    }
+    this.licencedAffiliationPastReconfirmation = {
+      confirmedAt: new Date('2000-01-01'),
+      affiliation: {
+        licence: 'pro_plus',
+        institution: { id: 888, confirmed: true },
+        pastReconfirmDate: true,
+      },
+    }
+    this.unconfirmedEmailLicensedAffiliation = {
+      confirmedAt: null,
+      affiliation: {
+        licence: 'pro_plus',
+        institution: { id: 123, confirmed: true, pastReconfirmDate: false },
+      },
+    }
+    this.unconfirmedDomainLicensedAffiliation = {
+      confirmedAt: new Date(),
+      affiliation: {
+        licence: 'pro_plus',
+        institution: { id: 789, confirmed: false, pastReconfirmDate: false },
+      },
+    }
     this.userEmails = [
       {
         confirmedAt: null,
@@ -90,4 +120,23 @@ describe('InstitutionsGetter', function () {
       expect(e.message).to.equal('oops')
     })
   })
+
+  describe('getCurrentInstitutionsWithLicence', function () {
+    it('returns one result per institution and filters out affiliations without license', async function () {
+      this.UserGetter.promises.getUserFullEmails.resolves([
+        this.licencedAffiliation,
+        this.licencedAffiliation,
+        this.licencedAffiliationPastReconfirmation,
+        this.confirmedAffiliation,
+        this.unconfirmedDomainLicensedAffiliation,
+        this.unconfirmedEmailLicensedAffiliation,
+      ])
+      const institutions = await this.InstitutionsGetter.promises.getCurrentInstitutionsWithLicence(
+        this.userId
+      )
+      expect(institutions.map(institution => institution.id)).to.deep.equal([
+        this.licencedAffiliation.affiliation.institution.id,
+      ])
+    })
+  })
 })