Explorar el Código

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
M Fahru hace 1 año
padre
commit
ef810a9f36

+ 11 - 6
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,
   })

+ 11 - 12
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),

+ 5 - 2
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
   }

+ 21 - 11
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
     })
   })
 

+ 10 - 11
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 () {