ソースを参照

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
ilkin-overleaf 10 ヶ月 前
親
コミット
3b32b0a61e

+ 25 - 7
services/web/app/src/Features/UserMembership/UserMembershipController.mjs

@@ -7,6 +7,7 @@ import UserMembershipErrors from './UserMembershipErrors.mjs'
 import { SSOConfig } from '../../models/SSOConfig.mjs'
 import { SSOConfig } from '../../models/SSOConfig.mjs'
 import { Parser as CSVParser } from 'json2csv'
 import { Parser as CSVParser } from 'json2csv'
 import { expressify } from '@overleaf/promise-utils'
 import { expressify } from '@overleaf/promise-utils'
+import logger from '@overleaf/logger'
 import PlansLocator from '../Subscription/PlansLocator.mjs'
 import PlansLocator from '../Subscription/PlansLocator.mjs'
 import RecurlyClient from '../Subscription/RecurlyClient.mjs'
 import RecurlyClient from '../Subscription/RecurlyClient.mjs'
 import Modules from '../../infrastructure/Modules.mjs'
 import Modules from '../../infrastructure/Modules.mjs'
@@ -34,17 +35,34 @@ async function manageGroupMembers(req, res, next) {
   const isUserGroupManager =
   const isUserGroupManager =
     Boolean(subscription.manager_ids?.some(id => id.toString() === userId)) &&
     Boolean(subscription.manager_ids?.some(id => id.toString() === userId)) &&
     !isAdmin
     !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
         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 &&
     plan?.canUseFlexibleLicensing &&
-    isAdmin &&
-    recurlySubscription &&
-    !recurlySubscription.pendingChange
+      isAdmin &&
+      recurlySubscription &&
+      !recurlySubscription.pendingChange
+  )
 
 
   res.render('user_membership/group-members-react', {
   res.render('user_membership/group-members-react', {
     name: entityName,
     name: entityName,

+ 133 - 1
services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs

@@ -21,7 +21,10 @@ vi.mock('../../../../app/src/Features/Errors/Errors.js', () =>
 )
 )
 
 
 describe('UserMembershipController', () => {
 describe('UserMembershipController', () => {
+  let recurlySubscriptionId
+
   beforeEach(async ctx => {
   beforeEach(async ctx => {
+    recurlySubscriptionId = 'mock-recurly-subscription-id'
     ctx.req = new MockRequest(vi)
     ctx.req = new MockRequest(vi)
     ctx.req.params.id = 'mock-entity-id'
     ctx.req.params.id = 'mock-entity-id'
     ctx.user = { _id: 'mock-user-id' }
     ctx.user = { _id: 'mock-user-id' }
@@ -30,6 +33,8 @@ describe('UserMembershipController', () => {
       _id: 'mock-subscription-id',
       _id: 'mock-subscription-id',
       admin_id: 'mock-admin-id',
       admin_id: 'mock-admin-id',
       manager_ids: ['mock-admin-id'],
       manager_ids: ['mock-admin-id'],
+      planCode: 'group_professional',
+      recurlySubscription_id: recurlySubscriptionId,
       fetchV1Data: vi.fn(callback => callback(null, ctx.subscription)),
       fetchV1Data: vi.fn(callback => callback(null, ctx.subscription)),
     }
     }
     ctx.institution = {
     ctx.institution = {
@@ -165,10 +170,19 @@ describe('UserMembershipController', () => {
     }
     }
     ctx.RecurlyClient = {
     ctx.RecurlyClient = {
       promises: {
       promises: {
-        getSubscription: vi.fn().mockResolvedValue({}),
+        getSubscription: vi.fn().mockResolvedValue({
+          id: recurlySubscriptionId,
+        }),
       },
       },
     }
     }
 
 
+    ctx.PlansLocator = {
+      findLocalPlanInSettings: vi.fn().mockReturnValue({
+        planCode: 'group_professional',
+        canUseFlexibleLicensing: true,
+      }),
+    }
+
     vi.doMock(
     vi.doMock(
       '../../../../app/src/Features/Authentication/SessionManager',
       '../../../../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', () => ({
     vi.doMock('@overleaf/settings', () => ({
       default: ctx.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 ({
     it('render group managers view', async ({
       UserMembershipController,
       UserMembershipController,
       req,
       req,