From 3b32b0a61ebdd515a95f72785617dcb064bf047c Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Mon, 15 Dec 2025 12:29:17 +0200 Subject: [PATCH] Merge pull request #30317 from overleaf/ii-recurly-subscription-id-not-found [web] Show group members page when recurly subscription does not exist GitOrigin-RevId: 8855af6ca1ad92cec437cabd2fe9425671d8ea23 --- .../UserMembershipController.mjs | 32 ++++- .../UserMembershipController.test.mjs | 134 +++++++++++++++++- 2 files changed, 158 insertions(+), 8 deletions(-) diff --git a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs index d0194dd0bb..b6c59bdee6 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs @@ -7,6 +7,7 @@ import UserMembershipErrors from './UserMembershipErrors.mjs' import { SSOConfig } from '../../models/SSOConfig.mjs' import { Parser as CSVParser } from 'json2csv' import { expressify } from '@overleaf/promise-utils' +import logger from '@overleaf/logger' import PlansLocator from '../Subscription/PlansLocator.mjs' import RecurlyClient from '../Subscription/RecurlyClient.mjs' import Modules from '../../infrastructure/Modules.mjs' @@ -34,17 +35,34 @@ async function manageGroupMembers(req, res, next) { const isUserGroupManager = Boolean(subscription.manager_ids?.some(id => id.toString() === userId)) && !isAdmin - const recurlySubscription = subscription.recurlySubscription_id - ? await RecurlyClient.promises.getSubscription( + + let recurlySubscription + try { + if (subscription.recurlySubscription_id) { + recurlySubscription = await RecurlyClient.promises.getSubscription( subscription.recurlySubscription_id ) - : undefined + } + } catch (err) { + // do not block page rendering + logger.error( + { + err, + subscription: { + _id: subscription._id, + recurlySubscription_id: subscription.recurlySubscription_id, + }, + }, + 'Error fetching Recurly subscription' + ) + } - const canUseAddSeatsFeature = + const canUseAddSeatsFeature = Boolean( plan?.canUseFlexibleLicensing && - isAdmin && - recurlySubscription && - !recurlySubscription.pendingChange + isAdmin && + recurlySubscription && + !recurlySubscription.pendingChange + ) res.render('user_membership/group-members-react', { name: entityName, diff --git a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs index a93e666995..146267dc14 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs +++ b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs @@ -21,7 +21,10 @@ vi.mock('../../../../app/src/Features/Errors/Errors.js', () => ) describe('UserMembershipController', () => { + let recurlySubscriptionId + beforeEach(async ctx => { + recurlySubscriptionId = 'mock-recurly-subscription-id' ctx.req = new MockRequest(vi) ctx.req.params.id = 'mock-entity-id' ctx.user = { _id: 'mock-user-id' } @@ -30,6 +33,8 @@ describe('UserMembershipController', () => { _id: 'mock-subscription-id', admin_id: 'mock-admin-id', manager_ids: ['mock-admin-id'], + planCode: 'group_professional', + recurlySubscription_id: recurlySubscriptionId, fetchV1Data: vi.fn(callback => callback(null, ctx.subscription)), } ctx.institution = { @@ -165,10 +170,19 @@ describe('UserMembershipController', () => { } ctx.RecurlyClient = { promises: { - getSubscription: vi.fn().mockResolvedValue({}), + getSubscription: vi.fn().mockResolvedValue({ + id: recurlySubscriptionId, + }), }, } + ctx.PlansLocator = { + findLocalPlanInSettings: vi.fn().mockReturnValue({ + planCode: 'group_professional', + canUseFlexibleLicensing: true, + }), + } + vi.doMock( '../../../../app/src/Features/Authentication/SessionManager', () => ({ @@ -197,6 +211,13 @@ describe('UserMembershipController', () => { }) ) + vi.doMock( + '../../../../app/src/Features/Subscription/PlansLocator.mjs', + () => ({ + default: ctx.PlansLocator, + }) + ) + vi.doMock('@overleaf/settings', () => ({ default: ctx.Settings, })) @@ -294,6 +315,117 @@ describe('UserMembershipController', () => { }) }) + describe('canUseAddSeatsFeature', () => { + beforeEach(ctx => { + ctx.subscription.admin_id = 'mock-admin-id' + ctx.SessionManager.getLoggedInUserId.mockReturnValue('mock-admin-id') + }) + + it('should be true when all conditions are met', async ({ + UserMembershipController, + req, + }) => { + expect.assertions(1) + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(true) + }, + }) + }) + + it('should be false when plan does not support flexible licensing', async ({ + UserMembershipController, + req, + PlansLocator, + }) => { + expect.assertions(1) + PlansLocator.findLocalPlanInSettings.mockReturnValue({ + planCode: 'group_professional', + canUseFlexibleLicensing: false, + }) + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(false) + }, + }) + }) + + it('should be false when user is not admin', async ({ + UserMembershipController, + req, + SessionManager, + }) => { + expect.assertions(1) + SessionManager.getLoggedInUserId.mockReturnValue('mock-user-id') + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(false) + }, + }) + }) + + it('should be false when recurly subscription does not exist', async ({ + UserMembershipController, + req, + subscription, + }) => { + expect.assertions(1) + subscription.recurlySubscription_id = null + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(false) + }, + }) + }) + + it('should be false when recurly subscription has pending changes', async ({ + UserMembershipController, + req, + RecurlyClient, + }) => { + expect.assertions(1) + RecurlyClient.promises.getSubscription.mockResolvedValue({ + id: recurlySubscriptionId, + pendingChange: {}, + }) + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(false) + }, + }) + }) + + it('should be false when fetching recurly subscription fails', async ({ + UserMembershipController, + req, + RecurlyClient, + }) => { + expect.assertions(1) + RecurlyClient.promises.getSubscription.mockRejectedValue( + new Error('Recurly error') + ) + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(false) + }, + }) + }) + + it('should be false when plan is not found', async ({ + UserMembershipController, + req, + PlansLocator, + }) => { + expect.assertions(1) + PlansLocator.findLocalPlanInSettings.mockReturnValue(null) + await UserMembershipController.manageGroupMembers(req, { + render: (viewPath, viewParams) => { + expect(viewParams.canUseAddSeatsFeature).to.equal(false) + }, + }) + }) + }) + it('render group managers view', async ({ UserMembershipController, req,