From d4d1a23a1c5b8fb8abddaec560beaf3d6bbbc0b6 Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Wed, 12 Feb 2025 11:19:56 +0200 Subject: [PATCH] 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 --- .../web/app/src/Features/Subscription/Errors.js | 3 +++ .../Subscription/SubscriptionGroupController.mjs | 3 +++ .../Subscription/SubscriptionGroupHandler.js | 16 +++++++++++++++- .../UserMembership/UserMembershipController.mjs | 13 ++++++++++++- .../dashboard/states/active/active-new.tsx | 16 ++++++---------- .../src/UserMembershipAuthorizationTests.mjs | 7 +++++++ .../SubscriptionGroupControllerTests.mjs | 16 ++++++++++++++++ .../SubscriptionGroupHandlerTests.js | 16 ++++++++++++++++ .../UserMembershipControllerTests.mjs | 7 +++++++ 9 files changed, 85 insertions(+), 12 deletions(-) diff --git a/services/web/app/src/Features/Subscription/Errors.js b/services/web/app/src/Features/Subscription/Errors.js index 4cc7d6ab7a..6b032df319 100644 --- a/services/web/app/src/Features/Subscription/Errors.js +++ b/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, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs index db2b8012a1..470092b050 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs +++ b/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( diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js index 63e23816b3..b3ad0da9d5 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js +++ b/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, diff --git a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs index 8b8ac86e51..e17793ab0f 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs +++ b/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, diff --git a/services/web/frontend/js/features/subscription/components/dashboard/states/active/active-new.tsx b/services/web/frontend/js/features/subscription/components/dashboard/states/active/active-new.tsx index 7f069ad298..259afcf331 100644 --- a/services/web/frontend/js/features/subscription/components/dashboard/states/active/active-new.tsx +++ b/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({

{planName}

-

- {subscription.pendingPlan && ( - <> - {' '} - - - )} -

{subscription.pendingPlan && subscription.pendingPlan.name !== subscription.plan.name && (

{t('want_change_to_apply_before_plan_end')}

@@ -195,7 +186,7 @@ export function ActiveSubscriptionNew({ className="mb-1" /> )} - {!subscription.pendingPlan && subscription.recurly.totalLicenses > 0 && ( + {subscription.recurly.totalLicenses > 0 && (

{isLegacyPlan && subscription.recurly.additionalLicenses > 0 ? ( { + 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() diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js index b5024b99f3..8b1c6a7290 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js +++ b/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 diff --git a/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.mjs b/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.mjs index 7363e9309d..f6dedf2097 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipControllerTests.mjs +++ b/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 }, })