From 72be034435962d761a607e0b287c0f1826db569a Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Tue, 4 Feb 2025 13:31:17 +0200 Subject: [PATCH] Merge pull request #23263 from overleaf/ii-flexible-licensing-subscription-group-handler [web] FL check subscription existence GitOrigin-RevId: b564d681245137955a8f1e7367b9bd1a6b404268 --- .../SubscriptionGroupController.mjs | 11 +++++-- .../Subscription/SubscriptionGroupHandler.js | 29 +++++++++---------- .../SubscriptionGroupControllerTests.mjs | 10 +++++-- .../SubscriptionGroupHandlerTests.js | 27 +++++++++++------ 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs index 53de39071f..b7f8617696 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs @@ -123,9 +123,10 @@ async function _removeUserFromGroup( */ async function addSeatsToGroupSubscription(req, res) { try { + const userId = SessionManager.getLoggedInUserId(req.session) const { subscription, plan } = await SubscriptionGroupHandler.promises.getUsersGroupSubscriptionDetails( - req + userId ) await SubscriptionGroupHandler.promises.ensureFlexibleLicensingEnabled(plan) @@ -151,9 +152,11 @@ async function addSeatsToGroupSubscription(req, res) { */ async function previewAddSeatsSubscriptionChange(req, res) { try { + const userId = SessionManager.getLoggedInUserId(req.session) const preview = await SubscriptionGroupHandler.promises.previewAddSeatsSubscriptionChange( - req + userId, + req.body.adding ) res.json(preview) @@ -173,9 +176,11 @@ async function previewAddSeatsSubscriptionChange(req, res) { */ async function createAddSeatsSubscriptionChange(req, res) { try { + const userId = SessionManager.getLoggedInUserId(req.session) const create = await SubscriptionGroupHandler.promises.createAddSeatsSubscriptionChange( - req + userId, + req.body.adding ) res.json(create) diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js index 51b70f2d8e..ff3c97ce61 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js @@ -3,7 +3,6 @@ const SubscriptionUpdater = require('./SubscriptionUpdater') const SubscriptionLocator = require('./SubscriptionLocator') const SubscriptionController = require('./SubscriptionController') const { Subscription } = require('../../models/Subscription') -const SessionManager = require('../Authentication/SessionManager') const RecurlyClient = require('./RecurlyClient') const PlansLocator = require('./PlansLocator') const SubscriptionHandler = require('./SubscriptionHandler') @@ -63,11 +62,14 @@ async function ensureFlexibleLicensingEnabled(plan) { } } -async function getUsersGroupSubscriptionDetails(req) { - const userId = SessionManager.getLoggedInUserId(req.session) +async function getUsersGroupSubscriptionDetails(userId) { const subscription = await SubscriptionLocator.promises.getUsersSubscription(userId) + if (!subscription) { + throw new Error('No subscription was found') + } + if (!subscription.groupPlan) { throw new Error('User subscription is not a group plan') } @@ -85,12 +87,10 @@ async function getUsersGroupSubscriptionDetails(req) { } } -async function _addSeatsSubscriptionChange(req) { - const adding = req.body.adding +async function _addSeatsSubscriptionChange(userId, adding) { const { recurlySubscription, plan } = - await getUsersGroupSubscriptionDetails(req) + await getUsersGroupSubscriptionDetails(userId) await ensureFlexibleLicensingEnabled(plan) - const userId = SessionManager.getLoggedInUserId(req.session) const currentAddonQuantity = recurlySubscription.addOns.find( addOn => addOn.code === MEMBERS_LIMIT_ADD_ON_CODE @@ -135,15 +135,14 @@ async function _addSeatsSubscriptionChange(req) { return { changeRequest, - userId, currentAddonQuantity, recurlySubscription, } } -async function previewAddSeatsSubscriptionChange(req) { - const { changeRequest, userId, currentAddonQuantity } = - await _addSeatsSubscriptionChange(req) +async function previewAddSeatsSubscriptionChange(userId, adding) { + const { changeRequest, currentAddonQuantity } = + await _addSeatsSubscriptionChange(userId, adding) const paymentMethod = await RecurlyClient.promises.getPaymentMethod(userId) const subscriptionChange = await RecurlyClient.promises.previewSubscriptionChange(changeRequest) @@ -166,16 +165,16 @@ async function previewAddSeatsSubscriptionChange(req) { return subscriptionChangePreview } -async function createAddSeatsSubscriptionChange(req) { - const { changeRequest, userId, recurlySubscription } = - await _addSeatsSubscriptionChange(req) +async function createAddSeatsSubscriptionChange(userId, adding) { + const { changeRequest, recurlySubscription } = + await _addSeatsSubscriptionChange(userId, adding) await RecurlyClient.promises.applySubscriptionChangeRequest(changeRequest) await SubscriptionHandler.promises.syncSubscription( { uuid: recurlySubscription.id }, userId ) - return { adding: req.body.adding } + return { adding } } async function _getUpgradeTargetPlanCodeMaybeThrow(subscription) { diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs b/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs index d55d7fe9dd..4c7b7b6769 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs @@ -331,7 +331,7 @@ describe('SubscriptionGroupController', function () { const res = { render: (page, props) => { this.SubscriptionGroupHandler.promises.getUsersGroupSubscriptionDetails - .calledWith(this.req) + .calledWith(this.req.session.user._id) .should.equal(true) this.SubscriptionGroupHandler.promises.ensureFlexibleLicensingEnabled .calledWith(this.plan) @@ -379,10 +379,12 @@ describe('SubscriptionGroupController', function () { describe('previewAddSeatsSubscriptionChange', function () { it('should preview "add seats" change', function (done) { + this.req.body = { adding: 2 } + const res = { json: data => { this.SubscriptionGroupHandler.promises.previewAddSeatsSubscriptionChange - .calledWith(this.req) + .calledWith(this.req.session.user._id, this.req.body.adding) .should.equal(true) data.should.deep.equal(this.previewSubscriptionChangeData) done() @@ -414,10 +416,12 @@ describe('SubscriptionGroupController', function () { describe('createAddSeatsSubscriptionChange', function () { it('should apply "add seats" change', function (done) { + this.req.body = { adding: 2 } + const res = { json: data => { this.SubscriptionGroupHandler.promises.createAddSeatsSubscriptionChange - .calledWith(this.req) + .calledWith(this.req.session.user._id, this.req.body.adding) .should.equal(true) data.should.deep.equal(this.createSubscriptionChangeData) done() diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js index 8d9b8772a9..b9ff9627d8 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js @@ -290,13 +290,15 @@ describe('SubscriptionGroupHandler', function () { .resolves({ groupPlan: false }) await expect( - this.Handler.promises.getUsersGroupSubscriptionDetails(this.req) + this.Handler.promises.getUsersGroupSubscriptionDetails( + this.adminUser_id + ) ).to.be.rejectedWith('User subscription is not a group plan') }) it('should return users group subscription details', async function () { const data = await this.Handler.promises.getUsersGroupSubscriptionDetails( - this.req + this.adminUser_id ) expect(data).to.deep.equal({ @@ -355,10 +357,11 @@ describe('SubscriptionGroupHandler', function () { it('should return the subscription change preview', async function () { const preview = await this.Handler.promises.previewAddSeatsSubscriptionChange( - this.req + this.adminUser_id, + this.adding ) this.RecurlyClient.promises.getPaymentMethod - .calledWith(this.user_id) + .calledWith(this.adminUser_id) .should.equal(true) this.RecurlyClient.promises.previewSubscriptionChange .calledWith(this.changeRequest) @@ -386,14 +389,18 @@ describe('SubscriptionGroupHandler', function () { it('should change the subscription', async function () { const result = await this.Handler.promises.createAddSeatsSubscriptionChange( - this.req + this.adminUser_id, + this.adding ) this.RecurlyClient.promises.applySubscriptionChangeRequest .calledWith(this.changeRequest) .should.equal(true) this.SubscriptionHandler.promises.syncSubscription - .calledWith({ uuid: this.recurlySubscription.id }, this.user_id) + .calledWith( + { uuid: this.recurlySubscription.id }, + this.adminUser_id + ) .should.equal(true) expect(result).to.deep.equal({ adding: this.req.body.adding, @@ -430,7 +437,7 @@ describe('SubscriptionGroupHandler', function () { afterEach(function () { this.RecurlyClient.promises.getPaymentMethod - .calledWith(this.user_id) + .calledWith(this.adminUser_id) .should.equal(true) this.RecurlyClient.promises.previewSubscriptionChange .calledWith(this.changeRequest) @@ -458,7 +465,8 @@ describe('SubscriptionGroupHandler', function () { preview = await this.Handler.promises.previewAddSeatsSubscriptionChange( - this.req + this.adminUser_id, + this.adding ) this.recurlySubscription.getRequestForAddOnPurchase .calledWithExactly( @@ -475,7 +483,8 @@ describe('SubscriptionGroupHandler', function () { preview = await this.Handler.promises.previewAddSeatsSubscriptionChange( - this.req + this.adminUser_id, + this.adding ) this.recurlySubscription.getRequestForAddOnPurchase .calledWithExactly(