diff --git a/services/web/app/src/Features/Subscription/RecurlyEventHandler.js b/services/web/app/src/Features/Subscription/RecurlyEventHandler.js index 7ddfcfb36a..76397ca511 100644 --- a/services/web/app/src/Features/Subscription/RecurlyEventHandler.js +++ b/services/web/app/src/Features/Subscription/RecurlyEventHandler.js @@ -1,33 +1,39 @@ const AnalyticsManager = require('../Analytics/AnalyticsManager') const SplitTestHandler = require('../SplitTests/SplitTestHandler') const SubscriptionEmailHandler = require('./SubscriptionEmailHandler') +const { ObjectID } = require('mongodb') + +async function sendRecurlyAnalyticsEvent(event, eventData) { + const userId = _getUserId(eventData) + if (!ObjectID.isValid(userId)) { + return + } -function sendRecurlyAnalyticsEvent(event, eventData) { switch (event) { case 'new_subscription_notification': - sendSubscriptionStartedEvent(eventData) + await _sendSubscriptionStartedEvent(userId, eventData) break case 'updated_subscription_notification': - _sendSubscriptionUpdatedEvent(eventData) + await _sendSubscriptionUpdatedEvent(userId, eventData) break case 'canceled_subscription_notification': - _sendSubscriptionCancelledEvent(eventData) + await _sendSubscriptionCancelledEvent(userId, eventData) break case 'expired_subscription_notification': - _sendSubscriptionExpiredEvent(eventData) + await _sendSubscriptionExpiredEvent(userId, eventData) break case 'renewed_subscription_notification': - _sendSubscriptionRenewedEvent(eventData) + await _sendSubscriptionRenewedEvent(userId, eventData) break case 'reactivated_account_notification': - _sendSubscriptionReactivatedEvent(eventData) + await _sendSubscriptionReactivatedEvent(userId, eventData) break case 'paid_charge_invoice_notification': if ( eventData.invoice.state === 'paid' && eventData.invoice.total_in_cents > 0 ) { - _sendInvoicePaidEvent(eventData) + await _sendInvoicePaidEvent(userId, eventData) } break case 'closed_invoice_notification': @@ -35,14 +41,13 @@ function sendRecurlyAnalyticsEvent(event, eventData) { eventData.invoice.state === 'collected' && eventData.invoice.total_in_cents > 0 ) { - _sendInvoicePaidEvent(eventData) + await _sendInvoicePaidEvent(userId, eventData) } break } } -async function sendSubscriptionStartedEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendSubscriptionStartedEvent(userId, eventData) { const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData) AnalyticsManager.recordEventForUser(userId, 'subscription-started', { plan_code: planCode, @@ -69,13 +74,12 @@ async function sendSubscriptionStartedEvent(eventData) { ) if (assignment.variant === 'send-email') { - SubscriptionEmailHandler.sendTrialOnboardingEmail(userId) + await SubscriptionEmailHandler.sendTrialOnboardingEmail(userId) } } } -async function _sendSubscriptionUpdatedEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendSubscriptionUpdatedEvent(userId, eventData) { const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData) AnalyticsManager.recordEventForUser(userId, 'subscription-updated', { plan_code: planCode, @@ -95,8 +99,7 @@ async function _sendSubscriptionUpdatedEvent(eventData) { ) } -async function _sendSubscriptionCancelledEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendSubscriptionCancelledEvent(userId, eventData) { const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData) AnalyticsManager.recordEventForUser(userId, 'subscription-cancelled', { plan_code: planCode, @@ -111,8 +114,7 @@ async function _sendSubscriptionCancelledEvent(eventData) { ) } -async function _sendSubscriptionExpiredEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendSubscriptionExpiredEvent(userId, eventData) { const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData) AnalyticsManager.recordEventForUser(userId, 'subscription-expired', { plan_code: planCode, @@ -132,8 +134,7 @@ async function _sendSubscriptionExpiredEvent(eventData) { ) } -async function _sendSubscriptionRenewedEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendSubscriptionRenewedEvent(userId, eventData) { const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData) AnalyticsManager.recordEventForUser(userId, 'subscription-renewed', { plan_code: planCode, @@ -153,8 +154,7 @@ async function _sendSubscriptionRenewedEvent(eventData) { ) } -async function _sendSubscriptionReactivatedEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendSubscriptionReactivatedEvent(userId, eventData) { const { planCode, quantity, state, isTrial } = _getSubscriptionData(eventData) AnalyticsManager.recordEventForUser(userId, 'subscription-reactivated', { plan_code: planCode, @@ -173,8 +173,7 @@ async function _sendSubscriptionReactivatedEvent(eventData) { ) } -async function _sendInvoicePaidEvent(eventData) { - const userId = _getUserId(eventData) +async function _sendInvoicePaidEvent(userId, eventData) { AnalyticsManager.recordEventForUser(userId, 'subscription-invoice-collected') AnalyticsManager.setUserPropertyForUser( userId, @@ -211,5 +210,4 @@ function _getSubscriptionData(eventData) { module.exports = { sendRecurlyAnalyticsEvent, - sendSubscriptionStartedEvent, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 8feb25bb49..e7aa941e2d 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -357,7 +357,12 @@ function recurlyCallback(req, res, next) { const event = Object.keys(req.body)[0] const eventData = req.body[event] - RecurlyEventHandler.sendRecurlyAnalyticsEvent(event, eventData) + RecurlyEventHandler.sendRecurlyAnalyticsEvent(event, eventData).catch(error => + logger.error( + { err: error }, + 'Failed to process analytics event on Recurly webhook' + ) + ) if ( [ diff --git a/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js b/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js index b6ff18f001..81584cbcc2 100644 --- a/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js +++ b/services/web/test/unit/src/Subscription/RecurlyEventHandlerTests.js @@ -5,7 +5,7 @@ const modulePath = describe('RecurlyEventHandler', function () { beforeEach(function () { - this.userId = '123456789abcde' + this.userId = '123abc234bcd456cde567def' this.planCode = 'collaborator-annual' this.eventData = { account: { @@ -86,12 +86,11 @@ describe('RecurlyEventHandler', function () { this.SplitTestHandler.promises.getAssignmentForUser = sinon .stub() .resolves({ variant: 'send-email' }) - this.userId = '123456789trial' - this.eventData.account.account_code = this.userId - // testing directly on the send subscription started event to ensure the split handler - // promise is resolved before checking calls - await this.RecurlyEventHandler.sendSubscriptionStartedEvent(this.eventData) + await this.RecurlyEventHandler.sendRecurlyAnalyticsEvent( + 'new_subscription_notification', + this.eventData + ) sinon.assert.calledWith( this.SplitTestHandler.promises.getAssignmentForUser, @@ -345,4 +344,18 @@ describe('RecurlyEventHandler', function () { ) sinon.assert.notCalled(this.AnalyticsManager.recordEventForUser) }) + + it('nothing is called with invalid account code', function () { + this.eventData.account.account_code = 'foo_bar' + + this.RecurlyEventHandler.sendRecurlyAnalyticsEvent( + 'new_subscription_notification', + this.eventData + ) + sinon.assert.notCalled(this.AnalyticsManager.recordEventForUser) + sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForUser) + sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForUser) + sinon.assert.notCalled(this.AnalyticsManager.setUserPropertyForUser) + sinon.assert.notCalled(this.SplitTestHandler.promises.getAssignmentForUser) + }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 0e60bbeb88..68507a19f9 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -149,7 +149,9 @@ describe('SubscriptionController', function () { './RecurlyWrapper': (this.RecurlyWrapper = { updateAccountEmailAddress: sinon.stub().yields(), }), - './RecurlyEventHandler': { sendRecurlyAnalyticsEvent: sinon.stub() }, + './RecurlyEventHandler': { + sendRecurlyAnalyticsEvent: sinon.stub().resolves(), + }, './FeaturesUpdater': (this.FeaturesUpdater = {}), './GroupPlansData': (this.GroupPlansData = {}), './V1SubscriptionManager': (this.V1SubscriptionManager = {}),