Merge pull request #23563 from overleaf/ii-flexible-group-licensing-errors

[web] Flexible licensing error handling

GitOrigin-RevId: 9fd4992a81e67b0684d3e286492e0037dd56e2ea

services/web/app/src/Features/Subscription/Errors.js

@@ -20,6 +20,8 @@ class ManuallyCollectedError extends OError {}
 
 class PendingChangeError extends OError {}
 
+class InactiveError extends OError {}
+
 module.exports = {
   RecurlyTransactionError,
   DuplicateAddOnError,
@@ -27,4 +29,5 @@ module.exports = {
   MissingBillingInfoError,
   ManuallyCollectedError,
   PendingChangeError,
+  InactiveError,
 }

services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs

@@ -13,7 +13,12 @@ import ErrorController from '../Errors/ErrorController.js'
 import UserGetter from '../User/UserGetter.js'
 import { Subscription } from '../../models/Subscription.js'
 import { isProfessionalGroupPlan } from './PlansHelper.mjs'
-import { MissingBillingInfoError, ManuallyCollectedError } from './Errors.js'
+import {
+  MissingBillingInfoError,
+  ManuallyCollectedError,
+  PendingChangeError,
+  InactiveError,
+} from './Errors.js'
 import RecurlyClient from './RecurlyClient.js'
 
 /**
@@ -150,11 +155,6 @@ async function addSeatsToGroupSubscription(req, res) {
       isProfessional: isProfessionalGroupPlan(subscription),
     })
   } catch (error) {
-    logger.err(
-      { error },
-      'error while getting users group subscription details'
-    )
-
     if (error instanceof MissingBillingInfoError) {
       return res.redirect(
         '/user/subscription/group/missing-billing-information'
@@ -167,6 +167,15 @@ async function addSeatsToGroupSubscription(req, res) {
       )
     }
 
+    if (error instanceof PendingChangeError || error instanceof InactiveError) {
+      return res.redirect('/user/subscription')
+    }
+
+    logger.err(
+      { error },
+      'error while getting users group subscription details'
+    )
+
     return res.redirect('/user/subscription')
   }
 }
@@ -187,11 +196,21 @@ async function previewAddSeatsSubscriptionChange(req, res) {
 
     res.json(preview)
   } catch (error) {
+    if (
+      error instanceof MissingBillingInfoError ||
+      error instanceof ManuallyCollectedError ||
+      error instanceof PendingChangeError ||
+      error instanceof InactiveError
+    ) {
+      return res.status(422).end()
+    }
+
     logger.err(
       { error },
       'error trying to preview "add seats" subscription change'
     )
-    return res.status(400).end()
+
+    return res.status(500).end()
   }
 }
 
@@ -211,11 +230,21 @@ async function createAddSeatsSubscriptionChange(req, res) {
 
     res.json(create)
   } catch (error) {
+    if (
+      error instanceof MissingBillingInfoError ||
+      error instanceof ManuallyCollectedError ||
+      error instanceof PendingChangeError ||
+      error instanceof InactiveError
+    ) {
+      return res.status(422).end()
+    }
+
     logger.err(
       { error },
       'error trying to create "add seats" subscription change'
     )
-    return res.status(400).end()
+
+    return res.status(500).end()
   }
 }
 
@@ -272,8 +301,6 @@ async function subscriptionUpgradePage(req, res) {
       groupName: olSubscription.teamName,
     })
   } catch (error) {
-    logger.err({ error }, 'error loading upgrade subscription page')
-
     if (error instanceof MissingBillingInfoError) {
       return res.redirect(
         '/user/subscription/group/missing-billing-information'
@@ -286,6 +313,12 @@ async function subscriptionUpgradePage(req, res) {
       )
     }
 
+    if (error instanceof PendingChangeError || error instanceof InactiveError) {
+      return res.redirect('/user/subscription')
+    }
+
+    logger.err({ error }, 'error loading upgrade subscription page')
+
     return res.redirect('/user/subscription')
   }
 }

services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js

@@ -8,7 +8,11 @@ const PlansLocator = require('./PlansLocator')
 const SubscriptionHandler = require('./SubscriptionHandler')
 const GroupPlansData = require('./GroupPlansData')
 const { MEMBERS_LIMIT_ADD_ON_CODE } = require('./RecurlyEntities')
-const { ManuallyCollectedError, PendingChangeError } = require('./Errors')
+const {
+  ManuallyCollectedError,
+  PendingChangeError,
+  InactiveError,
+} = require('./Errors')
 
 async function removeUserFromGroup(subscriptionId, userIdToRemove) {
   await SubscriptionUpdater.promises.removeUserFromGroup(
@@ -65,7 +69,9 @@ async function ensureFlexibleLicensingEnabled(plan) {
 
 async function ensureSubscriptionIsActive(subscription) {
   if (subscription?.recurlyStatus?.state !== 'active') {
-    throw new Error('The subscription is not active')
+    throw new InactiveError('The subscription is not active', {
+      subscriptionId: subscription._id.toString(),
+    })
   }
 }
 

services/web/frontend/js/features/group-management/components/add-seats/add-seats.tsx

@@ -27,11 +27,16 @@ import {
   AddOnUpdate,
   SubscriptionChangePreview,
 } from '../../../../../../types/subscription/subscription-change-preview'
-import { MergeAndOverride } from '../../../../../../types/utils'
+import { MergeAndOverride, Nullable } from '../../../../../../types/utils'
 import { sendMB } from '../../../../infrastructure/event-tracking'
 
 export const MAX_NUMBER_OF_USERS = 50
 
+type CostSummaryData = MergeAndOverride<
+  SubscriptionChangePreview,
+  { change: AddOnUpdate }
+>
+
 function AddSeats() {
   const { t } = useTranslation()
   const groupName = getMeta('ol-groupName')
@@ -45,12 +50,11 @@ function AddSeats() {
   const { signal: contactSalesSignal } = useAbortController()
   const {
     isLoading: isLoadingCostSummary,
+    isError: isErrorCostSummary,
     runAsync: runAsyncCostSummary,
     data: costSummaryData,
     reset: resetCostSummaryData,
-  } = useAsync<
-    MergeAndOverride<SubscriptionChangePreview, { change: AddOnUpdate }>
-  >()
+  } = useAsync<CostSummaryData>()
   const {
     isLoading: isAddingSeats,
     isError: isErrorAddingSeats,
@@ -319,30 +323,13 @@ function AddSeats() {
                     )}
                   </FormGroup>
                 </div>
-                {isLoadingCostSummary ? (
-                  <LoadingSpinner className="ms-auto me-auto" />
-                ) : shouldContactSales ? (
-                  <div>
-                    <Notification
-                      content={
-                        <Trans
-                          i18nKey="if_you_want_more_than_x_users_on_your_plan_we_need_to_add_them_for_you"
-                          // eslint-disable-next-line react/jsx-key
-                          components={[<b />]}
-                          values={{ count: 50 }}
-                          shouldUnescape
-                          tOptions={{ interpolation: { escapeValue: true } }}
-                        />
-                      }
-                      type="info"
-                    />
-                  </div>
-                ) : (
-                  <CostSummary
-                    subscriptionChange={costSummaryData}
-                    totalLicenses={totalLicenses}
-                  />
-                )}
+                <CostSummarySection
+                  isLoadingCostSummary={isLoadingCostSummary}
+                  isErrorCostSummary={isErrorCostSummary}
+                  shouldContactSales={shouldContactSales}
+                  costSummaryData={costSummaryData}
+                  totalLicenses={totalLicenses}
+                />
                 <div className="d-flex align-items-center justify-content-end gap-2">
                   {!isProfessional && (
                     <a
@@ -389,4 +376,57 @@ function AddSeats() {
   )
 }
 
+type CostSummarySectionProps = {
+  isLoadingCostSummary: boolean
+  isErrorCostSummary: boolean
+  shouldContactSales: boolean
+  costSummaryData: Nullable<CostSummaryData>
+  totalLicenses: number
+}
+
+function CostSummarySection({
+  isLoadingCostSummary,
+  isErrorCostSummary,
+  shouldContactSales,
+  costSummaryData,
+  totalLicenses,
+}: CostSummarySectionProps) {
+  const { t } = useTranslation()
+
+  if (isLoadingCostSummary) {
+    return <LoadingSpinner className="ms-auto me-auto" />
+  }
+
+  if (shouldContactSales) {
+    return (
+      <Notification
+        content={
+          <Trans
+            i18nKey="if_you_want_more_than_x_users_on_your_plan_we_need_to_add_them_for_you"
+            // eslint-disable-next-line react/jsx-key
+            components={[<b />]}
+            values={{ count: 50 }}
+            shouldUnescape
+            tOptions={{ interpolation: { escapeValue: true } }}
+          />
+        }
+        type="info"
+      />
+    )
+  }
+
+  if (isErrorCostSummary) {
+    return (
+      <Notification type="error" content={t('generic_something_went_wrong')} />
+    )
+  }
+
+  return (
+    <CostSummary
+      subscriptionChange={costSummaryData}
+      totalLicenses={totalLicenses}
+    />
+  )
+}
+
 export default withErrorBoundary(AddSeats)

services/web/test/unit/src/Subscription/RecurlyClientTests.js

@@ -110,7 +110,7 @@ describe('RecurlyClient', function () {
       },
     }
     this.Errors = {
-      MissingBillingInfoError: class MissingBillingInfoError extends Error {},
+      MissingBillingInfoError: class extends Error {},
     }
 
     return (this.RecurlyClient = SandboxedModule.require(MODULE_PATH, {

services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs

@@ -124,9 +124,10 @@ describe('SubscriptionGroupController', function () {
     }
 
     this.Errors = {
-      MissingBillingInfoError: class MissingBillingInfoError extends Error {},
-      ManuallyCollectedError: class ManuallyCollectedError extends Error {},
-      PendingChangeError: class PendingChangeError extends Error {},
+      MissingBillingInfoError: class extends Error {},
+      ManuallyCollectedError: class extends Error {},
+      PendingChangeError: class extends Error {},
+      InactiveError: class extends Error {},
     }
 
     this.Controller = await esmock.strict(modulePath, {
@@ -482,7 +483,7 @@ describe('SubscriptionGroupController', function () {
 
       const res = {
         status: statusCode => {
-          statusCode.should.equal(400)
+          statusCode.should.equal(500)
 
           return {
             end: () => {
@@ -519,7 +520,7 @@ describe('SubscriptionGroupController', function () {
 
       const res = {
         status: statusCode => {
-          statusCode.should.equal(400)
+          statusCode.should.equal(500)
 
           return {
             end: () => {

services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js

@@ -594,7 +594,7 @@ describe('SubscriptionGroupHandler', function () {
   describe('ensureSubscriptionIsActive', function () {
     it('should throw if the subscription is not active', async function () {
       await expect(
-        this.Handler.promises.ensureSubscriptionIsActive({})
+        this.Handler.promises.ensureSubscriptionIsActive(this.subscription)
       ).to.be.rejectedWith('The subscription is not active')
     })