Merge pull request #23547 from overleaf/ii-flexible-group-licensing-error-assist-2

[web] Hide flexible licensing buttons for pending plans (fix)

GitOrigin-RevId: ce5b4ce4138ed7a029b840a87c5498227e3204f4

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

@@ -18,10 +18,13 @@ class MissingBillingInfoError extends OError {}
 
 class ManuallyCollectedError extends OError {}
 
+class PendingChangeError extends OError {}
+
 module.exports = {
   RecurlyTransactionError,
   DuplicateAddOnError,
   AddOnNotPresentError,
   MissingBillingInfoError,
   ManuallyCollectedError,
+  PendingChangeError,
 }

services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs

@@ -134,6 +134,9 @@ async function addSeatsToGroupSubscription(req, res) {
     await SubscriptionGroupHandler.promises.ensureSubscriptionCollectionMethodIsNotManual(
       recurlySubscription
     )
+    await SubscriptionGroupHandler.promises.ensureSubscriptionHasNoPendingChanges(
+      recurlySubscription
+    )
     // Check if the user has missing billing details
     await RecurlyClient.promises.getPaymentMethod(userId)
     await SubscriptionGroupHandler.promises.ensureSubscriptionIsActive(

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

@@ -8,7 +8,7 @@ const PlansLocator = require('./PlansLocator')
 const SubscriptionHandler = require('./SubscriptionHandler')
 const GroupPlansData = require('./GroupPlansData')
 const { MEMBERS_LIMIT_ADD_ON_CODE } = require('./RecurlyEntities')
-const { ManuallyCollectedError } = require('./Errors')
+const { ManuallyCollectedError, PendingChangeError } = require('./Errors')
 
 async function removeUserFromGroup(subscriptionId, userIdToRemove) {
   await SubscriptionUpdater.promises.removeUserFromGroup(
@@ -82,6 +82,14 @@ async function ensureSubscriptionCollectionMethodIsNotManual(
   }
 }
 
+async function ensureSubscriptionHasNoPendingChanges(recurlySubscription) {
+  if (recurlySubscription.pendingChange) {
+    throw new PendingChangeError('This subscription has a pending change', {
+      recurlySubscription_id: recurlySubscription.id,
+    })
+  }
+}
+
 async function getUsersGroupSubscriptionDetails(userId) {
   const subscription =
     await SubscriptionLocator.promises.getUsersSubscription(userId)
@@ -114,6 +122,7 @@ async function _addSeatsSubscriptionChange(userId, adding) {
   await ensureFlexibleLicensingEnabled(plan)
   await ensureSubscriptionIsActive(subscription)
   await ensureSubscriptionCollectionMethodIsNotManual(recurlySubscription)
+  await ensureSubscriptionHasNoPendingChanges(recurlySubscription)
 
   const currentAddonQuantity =
     recurlySubscription.addOns.find(
@@ -244,6 +253,7 @@ async function _getGroupPlanUpgradeChangeRequest(ownerId) {
   )
 
   await ensureSubscriptionCollectionMethodIsNotManual(recurlySubscription)
+  await ensureSubscriptionHasNoPendingChanges(recurlySubscription)
 
   return recurlySubscription.getRequestForGroupPlanUpgrade(newPlanCode)
 }
@@ -285,6 +295,9 @@ module.exports = {
   ensureSubscriptionCollectionMethodIsNotManual: callbackify(
     ensureSubscriptionCollectionMethodIsNotManual
   ),
+  ensureSubscriptionHasNoPendingChanges: callbackify(
+    ensureSubscriptionHasNoPendingChanges
+  ),
   getTotalConfirmedUsersInGroup: callbackify(getTotalConfirmedUsersInGroup),
   isUserPartOfGroup: callbackify(isUserPartOfGroup),
   getGroupPlanUpgradePreview: callbackify(getGroupPlanUpgradePreview),
@@ -295,6 +308,7 @@ module.exports = {
     ensureFlexibleLicensingEnabled,
     ensureSubscriptionIsActive,
     ensureSubscriptionCollectionMethodIsNotManual,
+    ensureSubscriptionHasNoPendingChanges,
     getTotalConfirmedUsersInGroup,
     isUserPartOfGroup,
     getUsersGroupSubscriptionDetails,

services/web/app/src/Features/UserMembership/UserMembershipController.mjs

@@ -13,6 +13,7 @@ import { Parser as CSVParser } from 'json2csv'
 import { expressify } from '@overleaf/promise-utils'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.js'
 import PlansLocator from '../Subscription/PlansLocator.js'
+import RecurlyClient from '../Subscription/RecurlyClient.js'
 
 async function manageGroupMembers(req, res, next) {
   const { entity: subscription, entityConfig } = req
@@ -40,7 +41,17 @@ async function manageGroupMembers(req, res, next) {
   const plan = PlansLocator.findLocalPlanInSettings(subscription.planCode)
   const userId = SessionManager.getLoggedInUserId(req.session)
   const isAdmin = subscription.admin_id.toString() === userId
-  const canUseAddSeatsFeature = plan?.canUseFlexibleLicensing && isAdmin
+  const recurlySubscription = subscription.recurlySubscription_id
+    ? await RecurlyClient.promises.getSubscription(
+        subscription.recurlySubscription_id
+      )
+    : undefined
+
+  const canUseAddSeatsFeature =
+    plan?.canUseFlexibleLicensing &&
+    isAdmin &&
+    recurlySubscription &&
+    !recurlySubscription.pendingChange
 
   res.render('user_membership/group-members-react', {
     name: entityName,

services/web/frontend/js/features/subscription/components/dashboard/states/active/active-new.tsx

@@ -4,7 +4,6 @@ import { useSubscriptionDashboardContext } from '../../../../context/subscriptio
 import { RecurlySubscription } from '../../../../../../../../types/subscription/dashboard/subscription'
 import { CancelSubscriptionButton } from './cancel-subscription-button'
 import { CancelSubscription } from './cancel-plan/cancel-subscription'
-import { PendingPlanChange } from './pending-plan-change'
 import { TrialEnding } from './trial-ending'
 import { ChangePlanModal } from './change-plan/modals/change-plan-modal'
 import { ConfirmChangePlanModal } from './change-plan/modals/confirm-change-plan-modal'
@@ -176,14 +175,6 @@ export function ActiveSubscriptionNew({
       <h3 className={classnames('h5 mt-0 mb-1', bsVersion({ bs5: 'fw-bold' }))}>
         {planName}
       </h3>
-      <p className="mb-1">
-        {subscription.pendingPlan && (
-          <>
-            {' '}
-            <PendingPlanChange subscription={subscription} />
-          </>
-        )}
-      </p>
       {subscription.pendingPlan &&
         subscription.pendingPlan.name !== subscription.plan.name && (
           <p className="mb-1">{t('want_change_to_apply_before_plan_end')}</p>
@@ -195,7 +186,7 @@ export function ActiveSubscriptionNew({
             className="mb-1"
           />
         )}
-      {!subscription.pendingPlan && subscription.recurly.totalLicenses > 0 && (
+      {subscription.recurly.totalLicenses > 0 && (
         <p className="mb-1">
           {isLegacyPlan && subscription.recurly.additionalLicenses > 0 ? (
             <Trans
@@ -355,6 +346,11 @@ function FlexibleGroupLicensingActions({
   subscription: RecurlySubscription
 }) {
   const { t } = useTranslation()
+
+  if (subscription.pendingPlan) {
+    return null
+  }
+
   const isProfessionalPlan = subscription.planCode
     .toLowerCase()
     .includes('professional')

services/web/test/acceptance/src/UserMembershipAuthorizationTests.mjs

@@ -4,13 +4,20 @@ import User from './helpers/User.mjs'
 import Institution from './helpers/Institution.mjs'
 import Subscription from './helpers/Subscription.mjs'
 import Publisher from './helpers/Publisher.mjs'
+import sinon from 'sinon'
+import RecurlyClient from '../../../app/src/Features/Subscription/RecurlyClient.js'
 
 describe('UserMembershipAuthorization', function () {
   beforeEach(function (done) {
     this.user = new User()
+    sinon.stub(RecurlyClient.promises, 'getSubscription').resolves({})
     async.series([this.user.ensureUserExists.bind(this.user)], done)
   })
 
+  afterEach(function () {
+    RecurlyClient.promises.getSubscription.restore()
+  })
+
   describe('group', function () {
     beforeEach(function (done) {
       this.subscription = new Subscription({

services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs

@@ -58,6 +58,7 @@ describe('SubscriptionGroupController', function () {
         ensureFlexibleLicensingEnabled: sinon.stub().resolves(),
         ensureSubscriptionIsActive: sinon.stub().resolves(),
         ensureSubscriptionCollectionMethodIsNotManual: sinon.stub().resolves(),
+        ensureSubscriptionHasNoPendingChanges: sinon.stub().resolves(),
         getGroupPlanUpgradePreview: sinon
           .stub()
           .resolves(this.previewSubscriptionChangeData),
@@ -125,6 +126,7 @@ describe('SubscriptionGroupController', function () {
     this.Errors = {
       MissingBillingInfoError: class MissingBillingInfoError extends Error {},
       ManuallyCollectedError: class ManuallyCollectedError extends Error {},
+      PendingChangeError: class PendingChangeError extends Error {},
     }
 
     this.Controller = await esmock.strict(modulePath, {
@@ -427,6 +429,20 @@ describe('SubscriptionGroupController', function () {
       this.Controller.addSeatsToGroupSubscription(this.req, res)
     })
 
+    it('should redirect to subscription page when there is a pending change', function (done) {
+      this.SubscriptionGroupHandler.promises.ensureSubscriptionHasNoPendingChanges =
+        sinon.stub().throws(new this.Errors.PendingChangeError())
+
+      const res = {
+        redirect: url => {
+          url.should.equal('/user/subscription')
+          done()
+        },
+      }
+
+      this.Controller.addSeatsToGroupSubscription(this.req, res)
+    })
+
     it('should redirect to subscription page when subscription is not active', function (done) {
       this.SubscriptionGroupHandler.promises.ensureSubscriptionIsActive = sinon
         .stub()

services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js

@@ -629,6 +629,22 @@ describe('SubscriptionGroupHandler', function () {
     })
   })
 
+  describe('ensureSubscriptionHasNoPendingChanges', function () {
+    it('should throw if the subscription has pending change', async function () {
+      await expect(
+        this.Handler.promises.ensureSubscriptionHasNoPendingChanges({
+          pendingChange: {},
+        })
+      ).to.be.rejectedWith('This subscription has a pending change')
+    })
+
+    it('should not throw if the subscription has no pending change', async function () {
+      await expect(
+        this.Handler.promises.ensureSubscriptionHasNoPendingChanges({})
+      ).to.not.be.rejected
+    })
+  })
+
   describe('upgradeGroupPlan', function () {
     it('should upgrade the subscription for flexible licensing group plans', async function () {
       this.SubscriptionLocator.promises.getUsersSubscription = sinon

services/web/test/unit/src/UserMembership/UserMembershipControllerTests.mjs

@@ -81,6 +81,11 @@ describe('UserMembershipController', function () {
       },
       getAssignment: sinon.stub().yields(null, { variant: 'default' }),
     }
+    this.RecurlyClient = {
+      promises: {
+        getSubscription: sinon.stub().resolves({}),
+      },
+    }
     this.UserMembershipController = await esmock.strict(modulePath, {
       '../../../../app/src/Features/UserMembership/UserMembershipErrors': {
         UserIsManagerError,
@@ -93,6 +98,8 @@ describe('UserMembershipController', function () {
         this.SplitTestHandler,
       '../../../../app/src/Features/UserMembership/UserMembershipHandler':
         this.UserMembershipHandler,
+      '../../../../app/src/Features/Subscription/RecurlyClient':
+        this.RecurlyClient,
       '@overleaf/settings': this.Settings,
       '../../../../app/src/models/SSOConfig': { SSOConfig: this.SSOConfig },
     })