From cb2461d60475e6957dad2697736aebe174b9aa1e Mon Sep 17 00:00:00 2001
From: Jessica Lawshe <jessica.lawshe@overleaf.com>
Date: Tue, 5 Apr 2022 09:12:35 -0500
Subject: [PATCH] Merge pull request #6840 from
 overleaf/jel-onboard-non-sso-notify

[web] Include trial users when notifying commons users

GitOrigin-RevId: 439e9b0f55e5a376dc9f621b8ba76468e1e843e0
---
 .../Institutions/InstitutionsManager.js       |  6 +--
 .../Institutions/InstitutionsManagerTests.js  | 53 +++++++++++++++----
 2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/services/web/app/src/Features/Institutions/InstitutionsManager.js b/services/web/app/src/Features/Institutions/InstitutionsManager.js
index a5e89a29e5..7b44f0790b 100644
--- a/services/web/app/src/Features/Institutions/InstitutionsManager.js
+++ b/services/web/app/src/Features/Institutions/InstitutionsManager.js
@@ -326,11 +326,7 @@ const notifyUser = (
         }
       },
       function (cb) {
-        if (
-          subscription &&
-          !subscription.planCode.match(/(free|trial)/) &&
-          !subscription.groupPlan
-        ) {
+        if (subscription && !subscription.groupPlan) {
           NotificationsBuilder.redundantPersonalSubscription(
             affiliation,
             user
diff --git a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js
index 955465630e..814352d3b5 100644
--- a/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js
+++ b/services/web/test/unit/src/Institutions/InstitutionsManagerTests.js
@@ -126,22 +126,51 @@ describe('InstitutionsManager', function () {
     beforeEach(function () {
       this.user1Id = '123abc123abc123abc123abc'
       this.user2Id = '456def456def456def456def'
-      this.affiliations = [{ user_id: this.user1Id }, { user_id: this.user2Id }]
+      this.user3Id = 'trial123abc'
+      this.user4Id = 'group123abc'
+      this.affiliations = [
+        { user_id: this.user1Id },
+        { user_id: this.user2Id },
+        { user_id: this.user3Id },
+        { user_id: this.user4Id },
+      ]
       this.user1 = { _id: this.user1Id }
       this.user2 = { _id: this.user2Id }
-      this.subscription = {
-        planCode: 'pro',
-        groupPlan: false,
-      }
+      this.user3 = { _id: this.user3Id }
+      this.user4 = { _id: this.user4Id }
+
       this.UserGetter.getUser
         .withArgs(this.user1Id)
         .callsArgWith(1, null, this.user1)
       this.UserGetter.getUser
         .withArgs(this.user2Id)
         .callsArgWith(1, null, this.user2)
+      this.UserGetter.getUser
+        .withArgs(this.user3Id)
+        .callsArgWith(1, null, this.user3)
+      this.UserGetter.getUser
+        .withArgs(this.user4Id)
+        .callsArgWith(1, null, this.user4)
+
       this.SubscriptionLocator.getUsersSubscription
         .withArgs(this.user2)
-        .callsArgWith(1, null, this.subscription)
+        .callsArgWith(1, null, {
+          planCode: 'pro',
+          groupPlan: false,
+        })
+      this.SubscriptionLocator.getUsersSubscription
+        .withArgs(this.user3)
+        .callsArgWith(1, null, {
+          planCode: 'collaborator_free_trial_7_days',
+          groupPlan: false,
+        })
+      this.SubscriptionLocator.getUsersSubscription
+        .withArgs(this.user4)
+        .callsArgWith(1, null, {
+          planCode: 'collaborator-annual',
+          groupPlan: true,
+        })
+
       this.refreshFeatures.withArgs(this.user1Id).yields(null, {}, true)
       this.getInstitutionAffiliations.yields(null, this.affiliations)
     })
@@ -152,8 +181,7 @@ describe('InstitutionsManager', function () {
         false,
         error => {
           expect(error).not.to.exist
-          sinon.assert.calledTwice(this.refreshFeatures)
-
+          sinon.assert.callCount(this.refreshFeatures, 4)
           // expect no notifications
           sinon.assert.notCalled(
             this.NotificationsBuilder.featuresUpgradedByAffiliation
@@ -185,13 +213,13 @@ describe('InstitutionsManager', function () {
       )
     })
 
-    it('notifies users if they have a subscription that should be cancelled', function (done) {
+    it('notifies users if they have a subscription, or a trial subscription, that should be cancelled', function (done) {
       this.InstitutionsManager.refreshInstitutionUsers(
         this.institutionId,
         true,
         error => {
           expect(error).not.to.exist
-          sinon.assert.calledOnce(
+          sinon.assert.calledTwice(
             this.NotificationsBuilder.redundantPersonalSubscription
           )
           sinon.assert.calledWith(
@@ -199,6 +227,11 @@ describe('InstitutionsManager', function () {
             this.affiliations[1],
             this.user2
           )
+          sinon.assert.calledWith(
+            this.NotificationsBuilder.redundantPersonalSubscription,
+            this.affiliations[2],
+            this.user3
+          )
           done()
         }
       )