From 16130b79db10dedf10161ba7ee9fbe15c2f46731 Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Tue, 4 Feb 2025 13:53:14 +0200 Subject: [PATCH] Merge pull request #23203 from overleaf/ii-flexible-group-licensing-no-billing-details [web] FL handle subscriptions with missing billing info GitOrigin-RevId: 34209299c039992a80da5739e086beb5d0ede7b0 --- .../app/src/Features/Subscription/Errors.js | 3 ++ .../Features/Subscription/RecurlyClient.js | 15 +++++- .../SubscriptionGroupController.mjs | 37 ++++++++++++++ .../Subscription/SubscriptionGroupHandler.js | 1 + .../Subscription/SubscriptionRouter.mjs | 8 +++ .../missing-billing-information.pug | 13 +++++ .../web/frontend/extracted-translations.json | 2 + .../missing-billing-information.tsx | 51 +++++++++++++++++++ .../missing-billing-information.tsx | 8 +++ services/web/locales/en.json | 2 + .../missing-billing-information.spec.tsx | 35 +++++++++++++ .../src/Subscription/RecurlyClientTests.js | 23 +++++++++ .../SubscriptionGroupControllerTests.mjs | 45 +++++++++++++++- .../SubscriptionGroupHandlerTests.js | 1 + 14 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 services/web/app/views/subscriptions/missing-billing-information.pug create mode 100644 services/web/frontend/js/features/group-management/components/missing-billing-information.tsx create mode 100644 services/web/frontend/js/pages/user/subscription/group-management/missing-billing-information.tsx create mode 100644 services/web/test/frontend/features/group-management/components/missing-billing-information.spec.tsx diff --git a/services/web/app/src/Features/Subscription/Errors.js b/services/web/app/src/Features/Subscription/Errors.js index edc737bc70..48fc97d02b 100644 --- a/services/web/app/src/Features/Subscription/Errors.js +++ b/services/web/app/src/Features/Subscription/Errors.js @@ -14,8 +14,11 @@ class DuplicateAddOnError extends OError {} class AddOnNotPresentError extends OError {} +class MissingBillingInfoError extends OError {} + module.exports = { RecurlyTransactionError, DuplicateAddOnError, AddOnNotPresentError, + MissingBillingInfoError, } diff --git a/services/web/app/src/Features/Subscription/RecurlyClient.js b/services/web/app/src/Features/Subscription/RecurlyClient.js index bbe7fe26bc..7c7d489797 100644 --- a/services/web/app/src/Features/Subscription/RecurlyClient.js +++ b/services/web/app/src/Features/Subscription/RecurlyClient.js @@ -16,6 +16,7 @@ const { RecurlyPlan, RecurlyImmediateCharge, } = require('./RecurlyEntities') +const { MissingBillingInfoError } = require('./Errors') /** * @import { RecurlySubscriptionChangeRequest } from './RecurlyEntities' @@ -193,7 +194,19 @@ async function resumeSubscriptionByUuid(subscriptionUuid) { * @return {Promise} */ async function getPaymentMethod(userId) { - const billingInfo = await client.getBillingInfo(`code-${userId}`) + let billingInfo + + try { + billingInfo = await client.getBillingInfo(`code-${userId}`) + } catch (error) { + if (error instanceof recurly.errors.NotFoundError) { + throw new MissingBillingInfoError('This account has no billing info', { + userId, + }) + } + throw error + } + return paymentMethodFromApi(billingInfo) } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs index b7f8617696..08778eb698 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs @@ -13,6 +13,8 @@ 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 } from './Errors.js' +import RecurlyClient from './RecurlyClient.js' /** * @import { Subscription } from "../../../../types/subscription/dashboard/subscription.js" @@ -129,6 +131,8 @@ async function addSeatsToGroupSubscription(req, res) { userId ) await SubscriptionGroupHandler.promises.ensureFlexibleLicensingEnabled(plan) + // Check if the user has missing billing details + await RecurlyClient.promises.getPaymentMethod(userId) res.render('subscriptions/add-seats', { subscriptionId: subscription._id, @@ -141,6 +145,13 @@ async function addSeatsToGroupSubscription(req, res) { { error }, 'error while getting users group subscription details' ) + + if (error instanceof MissingBillingInfoError) { + return res.redirect( + '/user/subscription/group/missing-billing-information' + ) + } + return res.redirect('/user/subscription') } } @@ -247,6 +258,13 @@ async function subscriptionUpgradePage(req, res) { }) } catch (error) { logger.err({ error }, 'error loading upgrade subscription page') + + if (error instanceof MissingBillingInfoError) { + return res.redirect( + '/user/subscription/group/missing-billing-information' + ) + } + return res.redirect('/user/subscription') } } @@ -262,6 +280,24 @@ async function upgradeSubscription(req, res) { } } +async function missingBillingInformation(req, res) { + try { + const userId = SessionManager.getLoggedInUserId(req.session) + const subscription = + await SubscriptionLocator.promises.getUsersSubscription(userId) + + res.render('subscriptions/missing-billing-information', { + groupName: subscription.teamName, + }) + } catch (error) { + logger.err( + { error }, + 'error trying to render missing billing information page' + ) + return res.render('/user/subscription') + } +} + export default { removeUserFromGroup: expressify(removeUserFromGroup), removeSelfFromGroup: expressify(removeSelfFromGroup), @@ -276,4 +312,5 @@ export default { ), subscriptionUpgradePage: expressify(subscriptionUpgradePage), upgradeSubscription: expressify(upgradeSubscription), + missingBillingInformation: expressify(missingBillingInformation), } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js index ff3c97ce61..c6a0269fea 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.js @@ -81,6 +81,7 @@ async function getUsersGroupSubscriptionDetails(userId) { ) return { + userId, subscription, recurlySubscription, plan, diff --git a/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs b/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs index 998b5f4b28..544f667697 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs @@ -119,6 +119,14 @@ export default { SubscriptionGroupController.upgradeSubscription ) + webRouter.get( + '/user/subscription/group/missing-billing-information', + AuthenticationController.requireLogin(), + RateLimiterMiddleware.rateLimit(subscriptionRateLimiter), + SubscriptionGroupController.flexibleLicensingSplitTest, + SubscriptionGroupController.missingBillingInformation + ) + // Team invites webRouter.get( '/subscription/invites/:token/', diff --git a/services/web/app/views/subscriptions/missing-billing-information.pug b/services/web/app/views/subscriptions/missing-billing-information.pug new file mode 100644 index 0000000000..ddb2ac1693 --- /dev/null +++ b/services/web/app/views/subscriptions/missing-billing-information.pug @@ -0,0 +1,13 @@ +extends ../layout-marketing + +block vars + - bootstrap5PageStatus = 'enabled' // Enforce BS5 version + +block entrypointVar + - entrypoint = 'pages/user/subscription/group-management/missing-billing-information' + +block append meta + meta(name="ol-groupName", data-type="string", content=groupName) + +block content + main.content.content-alt#missing-billing-information-root diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 243fabf537..b80b275106 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -794,6 +794,7 @@ "is_email_affiliated": "", "issued_on": "", "it_looks_like_that_didnt_work_you_can_try_again_or_get_in_touch": "", + "it_looks_like_your_payment_details_are_missing_please_update_your_billing_information": "", "join_beta_program": "", "join_now": "", "join_overleaf_labs": "", @@ -951,6 +952,7 @@ "message_received": "", "missing_field_for_entry": "", "missing_fields_for_entry": "", + "missing_payment_details": "", "money_back_guarantee": "", "month": "", "month_plural": "", diff --git a/services/web/frontend/js/features/group-management/components/missing-billing-information.tsx b/services/web/frontend/js/features/group-management/components/missing-billing-information.tsx new file mode 100644 index 0000000000..1110a5b45a --- /dev/null +++ b/services/web/frontend/js/features/group-management/components/missing-billing-information.tsx @@ -0,0 +1,51 @@ +import { Trans, useTranslation } from 'react-i18next' +import { Card, CardBody, Row, Col } from 'react-bootstrap-5' +import getMeta from '@/utils/meta' +import IconButton from '@/features/ui/components/bootstrap-5/icon-button' +import OLNotification from '@/features/ui/components/ol/ol-notification' + +function MissingBillingInformation() { + const { t } = useTranslation() + const groupName = getMeta('ol-groupName') + + return ( +
+ + +
+ +

{groupName || t('group_subscription')}

+
+ + + , + // eslint-disable-next-line jsx-a11y/anchor-has-content, react/jsx-key + , + ]} + /> + } + className="m-0" + /> + + + + +
+ ) +} + +export default MissingBillingInformation diff --git a/services/web/frontend/js/pages/user/subscription/group-management/missing-billing-information.tsx b/services/web/frontend/js/pages/user/subscription/group-management/missing-billing-information.tsx new file mode 100644 index 0000000000..449e77f9b6 --- /dev/null +++ b/services/web/frontend/js/pages/user/subscription/group-management/missing-billing-information.tsx @@ -0,0 +1,8 @@ +import '../base' +import ReactDOM from 'react-dom' +import MissingBillingInformation from '@/features/group-management/components/missing-billing-information' + +const element = document.getElementById('missing-billing-information-root') +if (element) { + ReactDOM.render(, element) +} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index f69f4ff01d..308816c667 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1052,6 +1052,7 @@ "issued_on": "Issued: __date__", "it": "Italian", "it_looks_like_that_didnt_work_you_can_try_again_or_get_in_touch": "It looks like that didn’t work. You can try again or <0>get in touch with our Support team for more help.", + "it_looks_like_your_payment_details_are_missing_please_update_your_billing_information": "It looks like your payment details are missing. Please <0>update your billing information, or <1>get in touch with our Support team for more help.", "ja": "Japanese", "january": "January", "join_beta_program": "Join beta program", @@ -1270,6 +1271,7 @@ "message_received": "Message received", "missing_field_for_entry": "Missing field for", "missing_fields_for_entry": "Missing fields for", + "missing_payment_details": "Missing payment details", "money_back_guarantee": "30-day money back guarantee, no questions asked", "month": "month", "month_plural": "months", diff --git a/services/web/test/frontend/features/group-management/components/missing-billing-information.spec.tsx b/services/web/test/frontend/features/group-management/components/missing-billing-information.spec.tsx new file mode 100644 index 0000000000..10406a154e --- /dev/null +++ b/services/web/test/frontend/features/group-management/components/missing-billing-information.spec.tsx @@ -0,0 +1,35 @@ +import '../../../helpers/bootstrap-5' +import { SplitTestProvider } from '@/shared/context/split-test-context' +import MissingBillingInformation from '@/features/group-management/components/missing-billing-information' + +describe('', function () { + beforeEach(function () { + cy.window().then(win => { + win.metaAttributesCache.set('ol-groupName', 'My Awesome Team') + }) + + cy.mount( + + + + ) + }) + + it('shows missing payment details notification', function () { + cy.findByRole('alert').within(() => { + cy.findByText(/missing payment details/i) + cy.findByText( + /it looks like your payment details are missing\. Please.*, or.*with our Support team for more help/i + ).within(() => { + cy.findByRole('link', { + name: /update your billing information/i, + }).should('have.attr', 'href', '/user/subscription') + cy.findByRole('link', { name: /get in touch/i }).should( + 'have.attr', + 'href', + '/contact' + ) + }) + }) + }) +}) diff --git a/services/web/test/unit/src/Subscription/RecurlyClientTests.js b/services/web/test/unit/src/Subscription/RecurlyClientTests.js index fac2afc200..fb2c7aa978 100644 --- a/services/web/test/unit/src/Subscription/RecurlyClientTests.js +++ b/services/web/test/unit/src/Subscription/RecurlyClientTests.js @@ -99,6 +99,7 @@ describe('RecurlyClient', function () { let client this.client = client = { getAccount: sinon.stub(), + getBillingInfo: sinon.stub(), listAccountSubscriptions: sinon.stub(), previewSubscriptionChange: sinon.stub(), } @@ -108,6 +109,9 @@ describe('RecurlyClient', function () { return client }, } + this.Errors = { + MissingBillingInfoError: class MissingBillingInfoError extends Error {}, + } return (this.RecurlyClient = SandboxedModule.require(MODULE_PATH, { globals: { @@ -124,6 +128,7 @@ describe('RecurlyClient', function () { debug: sinon.stub(), }, '../User/UserGetter': this.UserGetter, + './Errors': this.Errors, }, })) }) @@ -463,4 +468,22 @@ describe('RecurlyClient', function () { }) }) }) + + describe('getPaymentMethod', function () { + it('should throw MissingBillingInfoError', async function () { + this.client.getBillingInfo = sinon + .stub() + .throws(new recurly.errors.NotFoundError()) + await expect( + this.RecurlyClient.promises.getPaymentMethod(this.user._id) + ).to.be.rejectedWith(this.Errors.MissingBillingInfoError) + }) + + it('should rethrow errors different than MissingBillingInfoError', async function () { + this.client.getBillingInfo = sinon.stub().throws(new Error()) + await expect( + this.RecurlyClient.promises.getPaymentMethod(this.user._id) + ).to.be.rejectedWith(Error) + }) + }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs b/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs index 4c7b7b6769..04b961af2b 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupControllerTests.mjs @@ -103,7 +103,13 @@ describe('SubscriptionGroupController', function () { }, } - this.RecurlyClient = {} + this.paymentMethod = { cardType: 'Visa', lastFour: '1111' } + + this.RecurlyClient = { + promises: { + getPaymentMethod: sinon.stub().resolves(this.paymentMethod), + }, + } this.SubscriptionController = {} @@ -113,6 +119,10 @@ describe('SubscriptionGroupController', function () { isProfessionalGroupPlan: sinon.stub().returns(false), } + this.Errors = { + MissingBillingInfoError: class MissingBillingInfoError extends Error {}, + } + this.Controller = await esmock.strict(modulePath, { '../../../../app/src/Features/Subscription/SubscriptionGroupHandler': this.SubscriptionGroupHandler, @@ -135,6 +145,7 @@ describe('SubscriptionGroupController', function () { '../../../../app/src/Features/Subscription/RecurlyClient': this.RecurlyClient, '../../../../app/src/Features/Subscription/PlansHelper': this.PlansHelper, + '../../../../app/src/Features/Subscription/Errors': this.Errors, '../../../../app/src/models/Subscription': this.SubscriptionModel, '@overleaf/logger': { err: sinon.stub(), @@ -375,6 +386,23 @@ describe('SubscriptionGroupController', function () { this.Controller.addSeatsToGroupSubscription(this.req, res) }) + + it('should redirect to missing billing information page when billing information is missing', function (done) { + this.RecurlyClient.promises.getPaymentMethod = sinon + .stub() + .throws(new this.Errors.MissingBillingInfoError()) + + const res = { + redirect: url => { + url.should.equal( + '/user/subscription/group/missing-billing-information' + ) + done() + }, + } + + this.Controller.addSeatsToGroupSubscription(this.req, res) + }) }) describe('previewAddSeatsSubscriptionChange', function () { @@ -549,6 +577,21 @@ describe('SubscriptionGroupController', function () { this.Controller.subscriptionUpgradePage(this.req, res) }) + + it('should redirect to missing billing information page when billing information is missing', function (done) { + this.RecurlyClient.promises.getPaymentMethod = sinon + .stub() + .throws(new this.Errors.MissingBillingInfoError()) + + const res = { + redirect: url => { + url.should.equal('/user/subscription') + done() + }, + } + + this.Controller.subscriptionUpgradePage(this.req, res) + }) }) describe('upgradeSubscription', function () { diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js index b9ff9627d8..9825cadced 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupHandlerTests.js @@ -302,6 +302,7 @@ describe('SubscriptionGroupHandler', function () { ) expect(data).to.deep.equal({ + userId: this.adminUser_id, subscription: { groupPlan: true }, plan: { membersLimit: 5,