From ef810a9f3675cd2f73b902b91a49efe16e8560d5 Mon Sep 17 00:00:00 2001 From: M Fahru Date: Tue, 3 Jun 2025 06:40:36 -0700 Subject: [PATCH] Merge pull request #25967 from overleaf/mf-sync-email-update-to-stripe-account [web] Sync Stripe customer email when user update their primary email in account setting GitOrigin-RevId: a5f4b4e960d2c9d4ba96a2b3036329f4868e1bb8 --- .../Features/Subscription/RecurlyWrapper.js | 17 ++++++---- .../Subscription/SubscriptionController.js | 23 +++++++------ .../web/app/src/Features/User/UserUpdater.js | 7 ++-- .../SubscriptionControllerTests.js | 32 ++++++++++++------- .../test/unit/src/User/UserUpdaterTests.js | 21 ++++++------ 5 files changed, 58 insertions(+), 42 deletions(-) diff --git a/services/web/app/src/Features/Subscription/RecurlyWrapper.js b/services/web/app/src/Features/Subscription/RecurlyWrapper.js index 2227597737..234f094ae0 100644 --- a/services/web/app/src/Features/Subscription/RecurlyWrapper.js +++ b/services/web/app/src/Features/Subscription/RecurlyWrapper.js @@ -11,22 +11,27 @@ const SubscriptionErrors = require('./Errors') const { callbackify } = require('@overleaf/promise-utils') /** - * @param accountId - * @param newEmail + * Updates the email address of a Recurly account + * + * @param userId + * @param newAccountEmail - the new email address to set for the Recurly account */ -async function updateAccountEmailAddress(accountId, newEmail) { +async function updateAccountEmailAddress(userId, newAccountEmail) { const data = { - email: newEmail, + email: newAccountEmail, } let requestBody try { requestBody = RecurlyWrapper._buildXml('account', data) } catch (error) { - throw OError.tag(error, 'error building xml', { accountId, newEmail }) + throw OError.tag(error, 'error building xml', { + accountId: userId, + newEmail: newAccountEmail, + }) } const { body } = await RecurlyWrapper.promises.apiRequest({ - url: `accounts/${accountId}`, + url: `accounts/${userId}`, method: 'PUT', body: requestBody, }) diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index a38b41f628..bd60fdc099 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -535,18 +535,17 @@ function cancelPendingSubscriptionChange(req, res, next) { }) } -function updateAccountEmailAddress(req, res, next) { +async function updateAccountEmailAddress(req, res, next) { const user = SessionManager.getSessionUser(req.session) - RecurlyWrapper.updateAccountEmailAddress( - user._id, - user.email, - function (error) { - if (error) { - return next(error) - } - res.sendStatus(200) - } - ) + try { + await RecurlyWrapper.promises.updateAccountEmailAddress( + user._id, + user.email + ) + return res.sendStatus(200) + } catch (error) { + return next(error) + } } function reactivateSubscription(req, res, next) { @@ -859,7 +858,7 @@ module.exports = { cancelV1Subscription, previewSubscription: expressify(previewSubscription), cancelPendingSubscriptionChange, - updateAccountEmailAddress, + updateAccountEmailAddress: expressify(updateAccountEmailAddress), reactivateSubscription, recurlyCallback, extendTrial: expressify(extendTrial), diff --git a/services/web/app/src/Features/User/UserUpdater.js b/services/web/app/src/Features/User/UserUpdater.js index 627e73875d..f21ee9a1ed 100644 --- a/services/web/app/src/Features/User/UserUpdater.js +++ b/services/web/app/src/Features/User/UserUpdater.js @@ -11,7 +11,6 @@ const EmailHandler = require('../Email/EmailHandler') const EmailHelper = require('../Helpers/EmailHelper') const Errors = require('../Errors/Errors') const NewsletterManager = require('../Newsletter/NewsletterManager') -const RecurlyWrapper = require('../Subscription/RecurlyWrapper') const UserAuditLogHandler = require('./UserAuditLogHandler') const AnalyticsManager = require('../Analytics/AnalyticsManager') const SubscriptionLocator = require('../Subscription/SubscriptionLocator') @@ -252,7 +251,11 @@ async function setDefaultEmailAddress( } try { - await RecurlyWrapper.promises.updateAccountEmailAddress(user._id, email) + await Modules.promises.hooks.fire( + 'updateAccountEmailAddress', + user._id, + email + ) } catch (error) { // errors are ignored } diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index b3ae6610e1..546f10f17b 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -153,7 +153,9 @@ describe('SubscriptionController', function () { '@overleaf/settings': this.settings, '../User/UserGetter': this.UserGetter, './RecurlyWrapper': (this.RecurlyWrapper = { - updateAccountEmailAddress: sinon.stub().yields(), + promises: { + updateAccountEmailAddress: sinon.stub().resolves(), + }, }), './RecurlyEventHandler': { sendRecurlyAnalyticsEvent: sinon.stub().resolves(), @@ -309,31 +311,39 @@ describe('SubscriptionController', function () { }) describe('updateAccountEmailAddress via put', function () { - it('should send the user and subscriptionId to RecurlyWrapper', function () { + it('should send the user and subscriptionId to RecurlyWrapper', async function () { this.res.sendStatus = sinon.spy() - this.SubscriptionController.updateAccountEmailAddress(this.req, this.res) - this.RecurlyWrapper.updateAccountEmailAddress + await this.SubscriptionController.updateAccountEmailAddress( + this.req, + this.res + ) + this.RecurlyWrapper.promises.updateAccountEmailAddress .calledWith(this.user._id, this.user.email) .should.equal(true) }) - it('should respond with 200', function () { + it('should respond with 200', async function () { this.res.sendStatus = sinon.spy() - this.SubscriptionController.updateAccountEmailAddress(this.req, this.res) + await this.SubscriptionController.updateAccountEmailAddress( + this.req, + this.res + ) this.res.sendStatus.calledWith(200).should.equal(true) }) - it('should send the error to the next handler when updating recurly account email fails', function (done) { - this.RecurlyWrapper.updateAccountEmailAddress.yields(new Error()) + it('should send the error to the next handler when updating recurly account email fails', async function () { + this.RecurlyWrapper.promises.updateAccountEmailAddress.rejects( + new Error() + ) this.next = sinon.spy(error => { - expect(error).instanceOf(Error) - done() + expect(error).to.be.instanceOf(Error) }) - this.SubscriptionController.updateAccountEmailAddress( + await this.SubscriptionController.updateAccountEmailAddress( this.req, this.res, this.next ) + expect(this.next.calledOnce).to.be.true }) }) diff --git a/services/web/test/unit/src/User/UserUpdaterTests.js b/services/web/test/unit/src/User/UserUpdaterTests.js index 5832bc4656..2803e6d6f2 100644 --- a/services/web/test/unit/src/User/UserUpdaterTests.js +++ b/services/web/test/unit/src/User/UserUpdaterTests.js @@ -59,11 +59,6 @@ describe('UserUpdater', function () { changeEmail: sinon.stub().resolves(), }, } - this.RecurlyWrapper = { - promises: { - updateAccountEmailAddress: sinon.stub().resolves(), - }, - } this.AnalyticsManager = { recordEventForUserInBackground: sinon.stub(), } @@ -264,9 +259,11 @@ describe('UserUpdater', function () { expect( this.NewsletterManager.promises.changeEmail ).to.have.been.calledWith(this.user, this.newEmail) - expect( - this.RecurlyWrapper.promises.updateAccountEmailAddress - ).to.have.been.calledWith(this.user._id, this.newEmail) + expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( + 'updateAccountEmailAddress', + this.user._id, + this.newEmail + ) }) it('validates email', async function () { @@ -615,9 +612,11 @@ describe('UserUpdater', function () { expect( this.NewsletterManager.promises.changeEmail ).to.have.been.calledWith(this.user, this.newEmail) - expect( - this.RecurlyWrapper.promises.updateAccountEmailAddress - ).to.have.been.calledWith(this.user._id, this.newEmail) + expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( + 'updateAccountEmailAddress', + this.user._id, + this.newEmail + ) }) it('handles Mongo errors', async function () {