From d893bb76cf6d066ddad4f2a1b3b54c740761f71c Mon Sep 17 00:00:00 2001 From: Liangjun Song <146005915+adai26@users.noreply.github.com> Date: Tue, 28 Jan 2025 11:06:53 +0000 Subject: [PATCH] Merge pull request #22816 from overleaf/enable-group-plan-upgrade-for-legacy-plans Enable group plan upgrade for legacy plans GitOrigin-RevId: 9dde0371eeb791a6331ab50733fd457e28837ba9 --- .../Features/Subscription/RecurlyEntities.js | 29 ++++++++++----- .../Subscription/SubscriptionGroupHandler.js | 33 +++++++++++------ services/web/locales/en.json | 2 +- .../src/Subscription/RecurlyEntitiesTest.js | 27 ++++++++++++++ .../SubscriptionGroupHandlerTests.js | 35 +++++++++++++++---- 5 files changed, 101 insertions(+), 25 deletions(-) diff --git a/services/web/app/src/Features/Subscription/RecurlyEntities.js b/services/web/app/src/Features/Subscription/RecurlyEntities.js index c21eb62634..18288c72d8 100644 --- a/services/web/app/src/Features/Subscription/RecurlyEntities.js +++ b/services/web/app/src/Features/Subscription/RecurlyEntities.js @@ -221,22 +221,35 @@ class RecurlySubscription { * Upgrade group plan with the plan code provided * * @param {string} newPlanCode + * @param {number} membersLimit * @return {RecurlySubscriptionChangeRequest} */ - getRequestForFlexibleLicensingGroupPlanUpgrade(newPlanCode) { + getRequestForGroupPlanUpgrade(newPlanCode, membersLimit) { // Ensure all the existing add-ons are added to the new plan - const existingAddOns = this.addOns.map( - addOn => - new RecurlySubscriptionAddOnUpdate({ - code: addOn.code, - quantity: addOn.quantity, - }) + // Except for the additional license, which will be added below + const addOns = this.addOns + .filter(addOn => addOn.code !== 'additional-license') + .map( + addOn => + new RecurlySubscriptionAddOnUpdate({ + code: addOn.code, + quantity: addOn.quantity, + }) + ) + + // Get the number of licenses from the membersLimit field in the Subscription model + // This is necessary because legacy group plans do not fully use add-ons to represent seats + addOns.push( + new RecurlySubscriptionAddOnUpdate({ + code: 'additional-license', + quantity: membersLimit, + }) ) return new RecurlySubscriptionChangeRequest({ subscription: this, timeframe: 'now', - addOnUpdates: existingAddOns, + addOnUpdates: addOns, planCode: newPlanCode, }) } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js index 8e48a6faf4..f6baea3852 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js @@ -8,11 +8,6 @@ const RecurlyClient = require('./RecurlyClient') const PlansLocator = require('./PlansLocator') const SubscriptionHandler = require('./SubscriptionHandler') -const PLAN_UPGRADE_MAP = { - group_collaborator: 'group_professional', - group_collaborator_educational: 'group_professional_educational', -} - async function removeUserFromGroup(subscriptionId, userIdToRemove) { await SubscriptionUpdater.promises.removeUserFromGroup( subscriptionId, @@ -152,11 +147,16 @@ async function createAddSeatsSubscriptionChange(req) { } async function _getUpgradeTargetPlanCodeMaybeThrow(subscription) { - if (!Object.keys(PLAN_UPGRADE_MAP).includes(subscription.planCode)) { + if ( + subscription.planCode.includes('professional') || + !subscription.groupPlan + ) { throw new Error('Not eligible for group plan upgrade') } - return PLAN_UPGRADE_MAP[subscription.planCode] + return subscription.planCode.includes('educational') + ? 'group_professional_educational' + : 'group_professional' } async function _getGroupPlanUpgradeChangeRequest(ownerId) { @@ -168,11 +168,24 @@ async function _getGroupPlanUpgradeChangeRequest(ownerId) { const recurlySubscription = await RecurlyClient.promises.getSubscription( olSubscription.recurlySubscription_id ) - return recurlySubscription.getRequestForFlexibleLicensingGroupPlanUpgrade( - newPlanCode + return recurlySubscription.getRequestForGroupPlanUpgrade( + newPlanCode, + olSubscription.membersLimit ) } +function _getPlanNameForDisplay(subscription) { + if (/^group_collaborator_\d+_enterprise$/.test(subscription.planCode)) { + return 'Overleaf Standard Group' + } + + if (/^group_collaborator_\d+_educational$/.test(subscription.planCode)) { + return 'Overleaf Standard Group Educational' + } + + return subscription.planName +} + async function getGroupPlanUpgradePreview(ownerId) { const changeRequest = await _getGroupPlanUpgradeChangeRequest(ownerId) const subscriptionChange = @@ -182,7 +195,7 @@ async function getGroupPlanUpgradePreview(ownerId) { { type: 'group-plan-upgrade', prevPlan: { - name: subscriptionChange.subscription.planName, + name: _getPlanNameForDisplay(subscriptionChange.subscription), }, }, subscriptionChange, diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 268db06df5..002c0a5020 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -863,7 +863,7 @@ "group_managed_by_group_administrator": "User accounts in this group are managed by the group administrator.", "group_plan_admins_can_easily_add_and_remove_users_from_a_group": "Group plan admins can easily add and remove users from a group. For site-wide plans, users are automatically upgraded when they register or add their email address to Overleaf (domain-based enrollment or SSO).", "group_plan_tooltip": "You are on the __plan__ plan as a member of a group subscription. Click to find out how to make the most of your Overleaf premium features.", - "group_plan_upgrade_description": "You’re on the <0>__currentPlan__ plan and you’re upgrading to the <0>__nextPlan__ plan. If you’re interested in a site-wide Overleaf Commons plan please <1>get in touch.", + "group_plan_upgrade_description": "You’re on the <0>__currentPlan__ plan and you’re upgrading to the <0>__nextPlan__ plan. If you’re interested in a site-wide Overleaf Commons plan, please <1>get in touch.", "group_plan_with_name_tooltip": "You are on the __plan__ plan as a member of a group subscription, __groupName__. Click to find out how to make the most of your Overleaf premium features.", "group_professional": "Group Professional", "group_sso_configuration_idp_metadata": "The information you provide here comes from your Identity Provider (IdP). This is often referred to as its <0>SAML metadata. You can add this manually or click <1>Import IdP metadata to import an XML file.", diff --git a/services/web/test/unit/src/Subscription/RecurlyEntitiesTest.js b/services/web/test/unit/src/Subscription/RecurlyEntitiesTest.js index 0c6cdb6117..66280e1a7f 100644 --- a/services/web/test/unit/src/Subscription/RecurlyEntitiesTest.js +++ b/services/web/test/unit/src/Subscription/RecurlyEntitiesTest.js @@ -246,6 +246,33 @@ describe('RecurlyEntities', function () { }) }) + describe('getRequestForGroupPlanUpgrade()', function () { + it('returns a correct change request', function () { + const changeRequest = this.subscription.getRequestForGroupPlanUpgrade( + 'test_plan_code', + 10 + ) + const addOns = [ + new RecurlySubscriptionAddOnUpdate({ + code: 'add-on-code', + quantity: 1, + }), + new RecurlySubscriptionAddOnUpdate({ + code: 'additional-license', + quantity: 10, + }), + ] + expect(changeRequest).to.deep.equal( + new RecurlySubscriptionChangeRequest({ + subscription: this.subscription, + timeframe: 'now', + addOnUpdates: addOns, + planCode: 'test_plan_code', + }) + ) + }) + }) + describe('without add-ons', function () { beforeEach(function () { const { RecurlySubscription } = this.RecurlyEntities diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js index 9ac472b9e3..aacfa916d3 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js @@ -42,9 +42,7 @@ describe('SubscriptionGroupHandler', function () { }, ], getRequestForAddOnUpdate: sinon.stub().returns(this.changeRequest), - getRequestForFlexibleLicensingGroupPlanUpgrade: sinon - .stub() - .returns(this.changeRequest), + getRequestForGroupPlanUpgrade: sinon.stub().returns(this.changeRequest), } this.SubscriptionLocator = { @@ -108,7 +106,7 @@ describe('SubscriptionGroupHandler', function () { } this.PlansLocator = { - findLocalPlanInSettings: sinon.stub(this.localPlanInSettings), + findLocalPlanInSettings: sinon.stub().returns(this.localPlanInSettings), } this.SubscriptionHandler = { @@ -373,7 +371,7 @@ describe('SubscriptionGroupHandler', function () { }) describe('upgradeGroupPlan', function () { - it('should upgrade the subscription', async function () { + it('should upgrade the subscription for flexible licensing group plans', async function () { this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() .resolves({ groupPlan: true, planCode: 'group_collaborator' }) @@ -386,7 +384,23 @@ describe('SubscriptionGroupHandler', function () { .should.equal(true) }) - it('should fail the upgrade if not eligible', async function () { + it('should upgrade the subscription for legacy group plans', async function () { + this.SubscriptionLocator.promises.getUsersSubscription = sinon + .stub() + .resolves({ + groupPlan: true, + planCode: 'group_collaborator_10_educational', + }) + await this.Handler.promises.upgradeGroupPlan(this.user_id) + this.RecurlyClient.promises.applySubscriptionChangeRequest + .calledWith(this.changeRequest) + .should.equal(true) + this.SubscriptionHandler.promises.syncSubscription + .calledWith({ uuid: this.changeRequest.subscription.id }, this.user_id) + .should.equal(true) + }) + + it('should fail the upgrade if is professional already', async function () { this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() .resolves({ groupPlan: true, planCode: 'group_professional' }) @@ -394,6 +408,15 @@ describe('SubscriptionGroupHandler', function () { this.Handler.promises.upgradeGroupPlan(this.user_id) ).to.be.rejectedWith('Not eligible for group plan upgrade') }) + + it('should fail the upgrade if not group plan', async function () { + this.SubscriptionLocator.promises.getUsersSubscription = sinon + .stub() + .resolves({ groupPlan: false, planCode: 'test_plan_code' }) + await expect( + this.Handler.promises.upgradeGroupPlan(this.user_id) + ).to.be.rejectedWith('Not eligible for group plan upgrade') + }) }) describe('getGroupPlanUpgradePreview', function () {