diff --git a/services/web/app/src/Features/Subscription/Errors.js b/services/web/app/src/Features/Subscription/Errors.js index 6b032df319..0e1c82498d 100644 --- a/services/web/app/src/Features/Subscription/Errors.js +++ b/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, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs index 470092b050..637802a665 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs +++ b/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') } } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js index b3ad0da9d5..05befd6ca5 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js +++ b/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(), + }) } } diff --git a/services/web/frontend/js/features/group-management/components/add-seats/add-seats.tsx b/services/web/frontend/js/features/group-management/components/add-seats/add-seats.tsx index e6b1c5ab82..79e59d3a44 100644 --- a/services/web/frontend/js/features/group-management/components/add-seats/add-seats.tsx +++ b/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 - >() + } = useAsync() const { isLoading: isAddingSeats, isError: isErrorAddingSeats, @@ -319,30 +323,13 @@ function AddSeats() { )} - {isLoadingCostSummary ? ( - - ) : shouldContactSales ? ( -
- ]} - values={{ count: 50 }} - shouldUnescape - tOptions={{ interpolation: { escapeValue: true } }} - /> - } - type="info" - /> -
- ) : ( - - )} +
{!isProfessional && ( + totalLicenses: number +} + +function CostSummarySection({ + isLoadingCostSummary, + isErrorCostSummary, + shouldContactSales, + costSummaryData, + totalLicenses, +}: CostSummarySectionProps) { + const { t } = useTranslation() + + if (isLoadingCostSummary) { + return + } + + if (shouldContactSales) { + return ( + ]} + values={{ count: 50 }} + shouldUnescape + tOptions={{ interpolation: { escapeValue: true } }} + /> + } + type="info" + /> + ) + } + + if (isErrorCostSummary) { + return ( + + ) + } + + return ( + + ) +} + export default withErrorBoundary(AddSeats) diff --git a/services/web/test/unit/src/Subscription/RecurlyClientTests.js b/services/web/test/unit/src/Subscription/RecurlyClientTests.js index d46e5c6cfa..8640d8defd 100644 --- a/services/web/test/unit/src/Subscription/RecurlyClientTests.js +++ b/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, { diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs b/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs index c2209f6a7a..ac6e0b2f72 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs +++ b/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: () => { diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js index 8b1c6a7290..cdb3c15089 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js +++ b/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') })