From ed501fa1c3485ea33d47ea75ac3b047782fb7bc5 Mon Sep 17 00:00:00 2001 From: Phil Booth Date: Wed, 25 Sep 2019 15:26:38 +0100 Subject: [PATCH] feat(metrics): add product_id and plan_id to more amplitude events Made the Flow model follow Backbone conventions more closely where the first argument to the constructor is attributes and the second is options. More robust checking when fetching the product ID from the path. The plan_id is now optional. Combined effort of @philbooth and @shane-tomlinson, Phil did thet hard work. issue #989 --- packages/fxa-content-server/.prettierrc | 4 +- .../app/scripts/lib/metrics.js | 47 ++++++-- .../app/scripts/lib/url-mixin.js | 9 ++ .../app/scripts/models/resume-token.js | 2 + .../app/scripts/models/subscription.ts | 101 ++++++++++++++++ .../views/mixins/resume-token-mixin.js | 5 +- .../app/scripts/views/support.js | 17 ++- .../app/tests/spec/lib/metrics.js | 73 ++++++++++-- .../app/tests/spec/models/account.js | 2 +- .../app/tests/spec/models/resume-token.js | 4 +- .../app/tests/spec/models/subscription.js | 111 ++++++++++++++++++ .../spec/views/mixins/resume-token-mixin.js | 11 ++ .../views/subscriptions_product_redirect.js | 2 +- .../app/tests/spec/views/support.js | 15 +-- .../app/tests/test_start.js | 1 + .../fxa-content-server/npm-shrinkwrap.json | 31 +++++ packages/fxa-content-server/package.json | 1 + .../server/lib/routes/post-metrics.js | 4 + packages/fxa-shared/metrics/amplitude.js | 6 +- packages/fxa-shared/test/metrics/amplitude.js | 8 +- 20 files changed, 412 insertions(+), 42 deletions(-) create mode 100644 packages/fxa-content-server/app/scripts/models/subscription.ts create mode 100644 packages/fxa-content-server/app/tests/spec/models/subscription.js diff --git a/packages/fxa-content-server/.prettierrc b/packages/fxa-content-server/.prettierrc index c1a6f667131..9e366d80f10 100644 --- a/packages/fxa-content-server/.prettierrc +++ b/packages/fxa-content-server/.prettierrc @@ -1,4 +1,6 @@ { "singleQuote": true, - "trailingComma": "es5" + "trailingComma": "es5", + "tabWidth": 2, + "useTabs": false } diff --git a/packages/fxa-content-server/app/scripts/lib/metrics.js b/packages/fxa-content-server/app/scripts/lib/metrics.js index bece2bd4e3d..45e676dff61 100644 --- a/packages/fxa-content-server/app/scripts/lib/metrics.js +++ b/packages/fxa-content-server/app/scripts/lib/metrics.js @@ -21,10 +21,11 @@ import Constants from './constants'; import Backbone from 'backbone'; import Duration from 'duration'; import Environment from './environment'; -import Flow from '../models/flow'; +import FlowModel from '../models/flow'; import NotifierMixin from './channels/notifier-mixin'; import speedTrap from 'speed-trap'; import Strings from './strings'; +import SubscriptionModel from 'models/subscription'; import xhr from './xhr'; // Speed trap is a singleton, convert it @@ -53,8 +54,8 @@ const ALLOWED_FIELDS = [ 'migration', 'navigationTiming', 'numStoredAccounts', - 'plan_id', - 'product_id', + 'planId', + 'productId', 'reason', 'referrer', 'screen', @@ -153,8 +154,6 @@ function Metrics(options = {}) { this._marketingImpressions = {}; this._migration = options.migration || NOT_REPORTED_VALUE; this._numStoredAccounts = options.numStoredAccounts || ''; - this._planId = options.planId || NOT_REPORTED_VALUE; - this._productId = options.productId || NOT_REPORTED_VALUE; this._referrer = this._window.document.referrer || NOT_REPORTED_VALUE; this._screenHeight = options.screenHeight || NOT_REPORTED_VALUE; this._screenWidth = options.screenWidth || NOT_REPORTED_VALUE; @@ -190,6 +189,8 @@ _.extend(Metrics.prototype, Backbone.Events, { // Set the initial inactivity timeout to clear navigation timing data. this._resetInactivityFlushTimeout(); + + this._initializeSubscriptionModel(); }, destroy() { @@ -205,7 +206,7 @@ _.extend(Metrics.prototype, Backbone.Events, { 'set-email-domain': '_setEmailDomain', 'set-sync-engines': '_setSyncEngines', 'set-uid': '_setUid', - 'set-plan-and-product-id': '_setPlanProductId', + 'subscription.initialize': '_initializeSubscriptionModel', 'clear-uid': '_clearUid', 'once!view-shown': '_setInitialView', /* eslint-enable sorting/sort-object-props */ @@ -222,7 +223,7 @@ _.extend(Metrics.prototype, Backbone.Events, { return; } - const flowModel = new Flow({ + const flowModel = new FlowModel({ metrics: this, sentryMetrics: this._sentryMetrics, window: this._window, @@ -233,6 +234,26 @@ _.extend(Metrics.prototype, Backbone.Events, { } }, + /** + * @private + * Initialise the subscription model. + * + * @param {Object} [model] model to initialise with. + * If unset, a fresh model is created. + */ + _initializeSubscriptionModel(model) { + if (model && model.has('productId')) { + this._subscriptionModel = model; + } else { + this._subscriptionModel = new SubscriptionModel( + {}, + { + window: this._window, + } + ); + } + }, + /** * @private * Log a flow event. If there is no flow model, do nothing. @@ -383,8 +404,10 @@ _.extend(Metrics.prototype, Backbone.Events, { marketing: flattenHashIntoArrayOfObjects(this._marketingImpressions), migration: this._migration, numStoredAccounts: this._numStoredAccounts, - plan_id: this._planId, - product_id: this._productId, + // planId and productId are optional so we can physically remove + // them from the payload instead of sending NOT_REPORTED_VALUE + planId: this._subscriptionModel.get('planId') || undefined, + productId: this._subscriptionModel.get('productId') || undefined, referrer: this._referrer, screen: { clientHeight: this._clientHeight, @@ -698,10 +721,14 @@ _.extend(Metrics.prototype, Backbone.Events, { }; }, - getFlowModel(flowModel) { + getFlowModel() { return this._flowModel; }, + getSubscriptionModel() { + return this._subscriptionModel; + }, + /** * Log the number of stored accounts * diff --git a/packages/fxa-content-server/app/scripts/lib/url-mixin.js b/packages/fxa-content-server/app/scripts/lib/url-mixin.js index 176021394dc..c021d295ae4 100644 --- a/packages/fxa-content-server/app/scripts/lib/url-mixin.js +++ b/packages/fxa-content-server/app/scripts/lib/url-mixin.js @@ -42,4 +42,13 @@ export default { getHashParams(paramNames) { return Url.hashParams(this.window.location.hash, paramNames); }, + + /** + * return the pathname of the window + * + * @returns {String} + */ + getPathname() { + return this.window.location.pathname; + }, }; diff --git a/packages/fxa-content-server/app/scripts/models/resume-token.js b/packages/fxa-content-server/app/scripts/models/resume-token.js index 4da87687ee3..ed2d522c025 100644 --- a/packages/fxa-content-server/app/scripts/models/resume-token.js +++ b/packages/fxa-content-server/app/scripts/models/resume-token.js @@ -20,6 +20,8 @@ const ALLOWED_KEYS = [ 'flowId', 'needsOptedInToMarketingEmail', 'newsletters', + 'planId', + 'productId', 'resetPasswordConfirm', 'style', 'uniqueUserId', diff --git a/packages/fxa-content-server/app/scripts/models/subscription.ts b/packages/fxa-content-server/app/scripts/models/subscription.ts new file mode 100644 index 00000000000..62c68ceef77 --- /dev/null +++ b/packages/fxa-content-server/app/scripts/models/subscription.ts @@ -0,0 +1,101 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/** + * A model to represent current subscription state, so that metrics can + * associate events with products and plans. + * + * Tries to read data from the URL or, failing that, the resume token. + */ + +import Backbone from 'backbone'; +import Cocktail from '../lib/cocktail'; +import ResumeTokenMixin from './mixins/resume-token'; +import UrlMixin from './mixins/url'; + +const SUBSCRIBE_PRODUCT_PATHNAME_REGEXP = /^\/subscriptions\/products\/(\w+)/; + +// Neither UrlMixin nor ResumeTokenMixin are classes or interfaces +interface IUrlMixin { + getSearchParam(paramName: string): string | undefined; + getPathname(): string; +} + +interface IResumeTokenMixin { + populateFromStringifiedResumeToken(resumeToken: string); +} + +const RESUME_TOKEN_FIELDS = ['planId', 'productId']; +class SubscriptionModel extends Backbone.Model { + window?: Window; + resumeTokenFields?: string[]; + + constructor(attrs = {}, options) { + super( + { + planId: null, + productId: null, + ...attrs, + }, + options + ); + } + + initialize(attrs = {}, options: { window?: Window } = {}) { + if (this.get('planId') && this.get('productId')) { + // already set, no need to look anywhere else for the values. + return; + } + + this.window = options.window || window; + this.resumeTokenFields = RESUME_TOKEN_FIELDS; + + this._setSubscriptionInfoFromUrl(); + if (this.get('productId')) { + return; + } + + this._setSubscriptionInfoFromResumeToken(); + } + + _setSubscriptionInfoFromUrl() { + const productId = this._getProductIdFromPathname(this.getPathname()); + if (productId) { + // If the user is browsing directly to /subscriptions/prod_* + this.set('productId', productId); + + // only get the planId from the query params if the user is at + // a product path. It's possible for the plan to not be + // specified, in which case the default plan will be used. + const planId = this.getSearchParam('plan'); + if (planId) { + this.set('planId', planId); + } + } + } + + _getProductIdFromPathname(pathname: string) { + const result = SUBSCRIBE_PRODUCT_PATHNAME_REGEXP.exec(pathname); + if (result) { + return result[1]; + } + return; + } + + _setSubscriptionInfoFromResumeToken() { + const resumeToken = this.getSearchParam('resume'); + if (resumeToken) { + // this.resumeTokenFields is not set until the constructor + // completes, and this method can be called from within + // the constructor. Set the value + this.populateFromStringifiedResumeToken(resumeToken); + } + } +} + +interface SubscriptionModel extends IUrlMixin, IResumeTokenMixin {} + +Cocktail.mixin(SubscriptionModel, ResumeTokenMixin, UrlMixin); + +export default SubscriptionModel; diff --git a/packages/fxa-content-server/app/scripts/views/mixins/resume-token-mixin.js b/packages/fxa-content-server/app/scripts/views/mixins/resume-token-mixin.js index c3a4d369d76..db9e0aa9cc9 100644 --- a/packages/fxa-content-server/app/scripts/views/mixins/resume-token-mixin.js +++ b/packages/fxa-content-server/app/scripts/views/mixins/resume-token-mixin.js @@ -26,12 +26,15 @@ export default { flowInfo = flowModel.pickResumeTokenInfo(); } + const subscriptionModel = this.metrics.getSubscriptionModel(); + var resumeTokenInfo = _.extend( {}, flowInfo, relierInfo, userInfo, - accountInfo + accountInfo, + subscriptionModel.pickResumeTokenInfo() ); return new ResumeToken(resumeTokenInfo); diff --git a/packages/fxa-content-server/app/scripts/views/support.js b/packages/fxa-content-server/app/scripts/views/support.js index 929e46b590d..55d27f7fe12 100644 --- a/packages/fxa-content-server/app/scripts/views/support.js +++ b/packages/fxa-content-server/app/scripts/views/support.js @@ -18,6 +18,7 @@ import PaymentServer from '../lib/payment-server'; import preventDefaultThen from './decorators/prevent_default_then'; import SettingsHeaderTemplate from 'templates/partial/settings-header.mustache'; import Strings from '../lib/strings'; +import SubscriptionModel from 'models/subscription'; import SupportForm from 'models/support-form'; import SupportFormErrorTemplate from 'templates/partial/support-form-error.mustache'; import SupportFormSuccessTemplate from 'templates/partial/support-form-success.mustache'; @@ -138,10 +139,18 @@ const SupportView = BaseView.extend({ let productName = 'Other'; if (subhubPlan) { productName = subhubPlan.product_name; - this.notifier.trigger('set-plan-and-product-id', { - planId: subhubPlan.plan_id, - productId: subhubPlan.product_id, - }); + this.notifier.trigger( + 'subscription.initialize', + new SubscriptionModel( + { + planId: subhubPlan.plan_id, + productId: subhubPlan.product_id, + }, + { + window: this.window, + } + ) + ); } this.supportForm.set({ diff --git a/packages/fxa-content-server/app/tests/spec/lib/metrics.js b/packages/fxa-content-server/app/tests/spec/lib/metrics.js index 1ffff65644b..3fdb4e9ad58 100644 --- a/packages/fxa-content-server/app/tests/spec/lib/metrics.js +++ b/packages/fxa-content-server/app/tests/spec/lib/metrics.js @@ -13,6 +13,7 @@ import Environment from 'lib/environment'; import Metrics from 'lib/metrics'; import Notifier from 'lib/channels/notifier'; import sinon from 'sinon'; +import SubscriptionModel from 'models/subscription'; import TestHelpers from '../../lib/helpers'; import WindowMock from '../../mocks/window'; @@ -46,8 +47,6 @@ describe('lib/metrics', function() { isSampledUser: true, lang: 'db_LB', notifier, - plan_id: 'plid', - product_id: 'pid', screenHeight: 1200, screenWidth: 1600, service: 'sync', @@ -61,6 +60,7 @@ describe('lib/metrics', function() { window: windowMock, }) ); + sinon.spy(metrics, '_initializeSubscriptionModel'); } beforeEach(function() { @@ -84,7 +84,10 @@ describe('lib/metrics', function() { assert.isTrue('flow.event' in metrics.notifications); assert.isTrue('set-email-domain' in metrics.notifications); assert.isTrue('set-sync-engines' in metrics.notifications); - assert.isTrue('set-plan-and-product-id' in metrics.notifications); + assert.equal( + metrics.notifications['subscription.initialize'], + '_initializeSubscriptionModel' + ); assert.isTrue('set-uid' in metrics.notifications); assert.isTrue('clear-uid' in metrics.notifications); assert.isTrue('once!view-shown' in metrics.notifications); @@ -107,6 +110,14 @@ describe('lib/metrics', function() { }); }); + it('observable subscription state is correct', () => { + assert.equal(metrics._initializeSubscriptionModel.callCount, 0); + const subscriptionModel = metrics.getSubscriptionModel(); + assert.instanceOf(subscriptionModel, SubscriptionModel); + assert.equal(subscriptionModel.get('planId'), null); + assert.equal(subscriptionModel.get('productId'), null); + }); + it('deviceId defaults to NOT_REPORTED_VALUE', () => { assert.equal(metrics.getAllData().deviceId, 'none'); }); @@ -196,6 +207,41 @@ describe('lib/metrics', function() { }); }); + describe('trigger subscription.initialize event with explicit model', () => { + let subscriptionModel; + + beforeEach(() => { + subscriptionModel = new SubscriptionModel({ + planId: 'foo', + productId: 'bar', + }); + notifier.trigger('subscription.initialize', subscriptionModel); + }); + + it('observable subscription state is correct', () => { + assert.equal(metrics._initializeSubscriptionModel.callCount, 1); + assert.equal(metrics.getSubscriptionModel(), subscriptionModel); + assert.equal(subscriptionModel.get('planId'), 'foo'); + assert.equal(subscriptionModel.get('productId'), 'bar'); + }); + }); + + describe('trigger subscription.initialize event with url params', () => { + beforeEach(() => { + windowMock.location.search = 'foo=bar&plan=wibble'; + windowMock.location.pathname = '/subscriptions/products/prod_blee'; + notifier.trigger('subscription.initialize'); + }); + + it('observable subscription state is correct', () => { + assert.equal(metrics._initializeSubscriptionModel.callCount, 1); + const subscriptionModel = metrics.getSubscriptionModel(); + assert.instanceOf(subscriptionModel, SubscriptionModel); + assert.equal(subscriptionModel.get('planId'), 'wibble'); + assert.equal(subscriptionModel.get('productId'), 'prod_blee'); + }); + }); + describe('getFilteredData', function() { it('gets data that is allowed to be sent to the server', function() { var filteredData = metrics.getFilteredData(); @@ -318,10 +364,6 @@ describe('lib/metrics', function() { xhr: xhr, }); notifier.trigger('set-uid', 'mock uid'); - notifier.trigger('set-plan-and-product-id', { - planId: 'plid', - productId: 'pid', - }); }); afterEach(function() { @@ -337,6 +379,13 @@ describe('lib/metrics', function() { notifier.trigger('view-shown', { viewName: 'wibble' }); // trigger `view-shown` twice, ensure it's only logged once. notifier.trigger('view-shown', { viewName: 'blee' }); + notifier.trigger( + 'subscription.initialize', + new SubscriptionModel({ + planId: 'plid', + productId: 'pid', + }) + ); }); describe('has sendBeacon', function() { @@ -409,8 +458,8 @@ describe('lib/metrics', function() { assert.isDefined(data.flushTime); assert.isObject(data.timers); assert.lengthOf(Object.keys(data.timers), 0); - assert.equal(data.plan_id, 'plid'); - assert.equal(data.product_id, 'pid'); + assert.equal(data.planId, 'plid'); + assert.equal(data.productId, 'pid'); assert.equal(data.uid, 'mock uid'); assert.equal(data.utm_campaign, 'none'); assert.equal(data.utm_content, 'none'); @@ -722,6 +771,10 @@ describe('lib/metrics', function() { it('sends data once', function() { assert.equal(windowMock.navigator.sendBeacon.callCount, 1); + + const data = JSON.parse(windowMock.navigator.sendBeacon.args[0][1]); + assert.isUndefined(data.planId); + assert.isUndefined(data.productId); }); }); @@ -780,7 +833,7 @@ describe('lib/metrics', function() { assert.isArray(data.timers.foo); assert.lengthOf(data.timers.foo, 1); assert.isObject(data.timers.foo[0]); - assert.isTrue(data.timers.foo[0].elapsed >= 4); + assert.isAtLeast(data.timers.foo[0].elapsed, 4); }); }); diff --git a/packages/fxa-content-server/app/tests/spec/models/account.js b/packages/fxa-content-server/app/tests/spec/models/account.js index 10c22f99bb4..b887eb90a79 100644 --- a/packages/fxa-content-server/app/tests/spec/models/account.js +++ b/packages/fxa-content-server/app/tests/spec/models/account.js @@ -2961,7 +2961,7 @@ describe('models/account', function() { describe('fetchSubscriptionPlans', () => { it('delegates to the fxa-client', () => { const token = 'tickettoride'; - const plans = [{ product_id: 'foo', plan_id: 'bar' }]; + const plans = [{ product_id: 'foo', plan: 'bar' }]; sinon .stub(account, 'createOAuthToken') .resolves(new OAuthToken({ token })); diff --git a/packages/fxa-content-server/app/tests/spec/models/resume-token.js b/packages/fxa-content-server/app/tests/spec/models/resume-token.js index b7897aeb066..287ef562997 100644 --- a/packages/fxa-content-server/app/tests/spec/models/resume-token.js +++ b/packages/fxa-content-server/app/tests/spec/models/resume-token.js @@ -19,6 +19,8 @@ const TOKEN_OBJ = { flowId: 'a-big-flow-id', needsOptedInToMarketingEmail: true, newsletters: ['knowledge-is-power'], + planId: 'wibble', + productId: 'blee', resetPasswordConfirm: false, style: 'trailhead', uniqueUserId: UNIQUE_USER_ID, @@ -31,7 +33,7 @@ const TOKEN_OBJ = { describe('models/resume-token', function() { it('expected fields are allowed in resume token', () => { - assert.lengthOf(ResumeToken.ALLOWED_KEYS, 17); + assert.lengthOf(ResumeToken.ALLOWED_KEYS, 19); assert.sameMembers(ResumeToken.ALLOWED_KEYS, Object.keys(TOKEN_OBJ)); }); diff --git a/packages/fxa-content-server/app/tests/spec/models/subscription.js b/packages/fxa-content-server/app/tests/spec/models/subscription.js new file mode 100644 index 00000000000..55228c2d3b1 --- /dev/null +++ b/packages/fxa-content-server/app/tests/spec/models/subscription.js @@ -0,0 +1,111 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +import { assert } from 'chai'; +import ResumeToken from 'models/resume-token'; +import SubscriptionModel from 'models/subscription'; +import Url from 'lib/url'; +import WindowMock from '../../mocks/window'; + +describe('models/subscription', () => { + let windowMock; + + beforeEach(() => { + windowMock = new WindowMock(); + }); + + it('has the correct defaults', () => { + const model = new SubscriptionModel(); + + assert.isNull(model.get('planId')); + assert.isNull(model.get('productId')); + assert.deepEqual(model.resumeTokenFields, ['planId', 'productId']); + }); + + it('populates model using options', () => { + windowMock.location.search = 'foo=bar&plan=baz&qux='; + windowMock.location.pathname = '/subscriptions/products/prod_wibble'; + const model = new SubscriptionModel( + { + planId: 'foo', + productId: 'bar', + }, + { + window: windowMock, + } + ); + + assert.equal(model.get('planId'), 'foo'); + assert.equal(model.get('productId'), 'bar'); + }); + + it('populates model using url params', () => { + windowMock.location.pathname = '/subscriptions/products/prod_wibble'; + windowMock.location.search = Url.objToSearchString({ + foo: 'bar', + plan: 'baz', + qux: '', + resume: ResumeToken.stringify({ + planId: 'foo', + productId: 'bar', + }), + }); + const model = new SubscriptionModel({}, { window: windowMock }); + + assert.equal(model.get('planId'), 'baz'); + assert.equal(model.get('productId'), 'prod_wibble'); + }); + + it('populates model using resume token', () => { + windowMock.location.search = Url.objToSearchString({ + resume: ResumeToken.stringify({ + planId: 'foo', + productId: 'bar', + }), + }); + const model = new SubscriptionModel({}, { window: windowMock }); + + assert.equal(model.get('planId'), 'foo'); + assert.equal(model.get('productId'), 'bar'); + }); + + describe('_getProductIdFromPathname', () => { + it('gets a product_id from a well formatted pathname', () => { + const model = new SubscriptionModel(); + assert.equal( + model._getProductIdFromPathname( + '/subscriptions/products/prod_my_product' + ), + 'prod_my_product' + ); + assert.equal( + model._getProductIdFromPathname( + '/subscriptions/products/prod_123_FASDF' + ), + 'prod_123_FASDF' + ); + assert.equal( + model._getProductIdFromPathname('/subscriptions/products/flism'), + 'flism' + ); + assert.equal( + model._getProductIdFromPathname('/subscriptions/products/prodprod'), + 'prodprod' + ); + }); + + it('returns undefined if malformed pathname', () => { + const model = new SubscriptionModel(); + assert.isUndefined(model._getProductIdFromPathname('')); + // no leading / + assert.isUndefined( + model._getProductIdFromPathname('subscriptions/products/prod_123') + ); + // no product id + assert.isUndefined( + model._getProductIdFromPathname('/subscriptions/products/') + ); + }); + }); +}); diff --git a/packages/fxa-content-server/app/tests/spec/views/mixins/resume-token-mixin.js b/packages/fxa-content-server/app/tests/spec/views/mixins/resume-token-mixin.js index 79e2f712271..bbbbf6d133d 100644 --- a/packages/fxa-content-server/app/tests/spec/views/mixins/resume-token-mixin.js +++ b/packages/fxa-content-server/app/tests/spec/views/mixins/resume-token-mixin.js @@ -28,6 +28,7 @@ describe('views/mixins/resume-token-mixin', function() { let flow; let metrics; let relier; + let subscription; let user; let view; @@ -40,10 +41,18 @@ describe('views/mixins/resume-token-mixin', function() { pickResumeTokenInfo: sinon.spy(), }; + subscription = { + pickResumeTokenInfo: sinon.spy(), + }; + metrics = { getFlowModel() { return flow; }, + + getSubscriptionModel() { + return subscription; + }, }; relier = { @@ -75,6 +84,7 @@ describe('views/mixins/resume-token-mixin', function() { assert.isTrue(account.pickResumeTokenInfo.calledOnce); assert.isTrue(flow.pickResumeTokenInfo.calledOnce); assert.isTrue(relier.pickResumeTokenInfo.calledOnce); + assert.equal(subscription.pickResumeTokenInfo.callCount, 1); assert.isTrue(user.pickResumeTokenInfo.calledOnce); }); }); @@ -86,6 +96,7 @@ describe('views/mixins/resume-token-mixin', function() { assert.isTrue(account.pickResumeTokenInfo.calledOnce); assert.isTrue(flow.pickResumeTokenInfo.calledOnce); assert.isTrue(relier.pickResumeTokenInfo.calledOnce); + assert.equal(subscription.pickResumeTokenInfo.callCount, 1); assert.isTrue(user.pickResumeTokenInfo.calledOnce); }); }); diff --git a/packages/fxa-content-server/app/tests/spec/views/subscriptions_product_redirect.js b/packages/fxa-content-server/app/tests/spec/views/subscriptions_product_redirect.js index bcd2185fed2..609f071e1d7 100644 --- a/packages/fxa-content-server/app/tests/spec/views/subscriptions_product_redirect.js +++ b/packages/fxa-content-server/app/tests/spec/views/subscriptions_product_redirect.js @@ -13,7 +13,7 @@ import WindowMock from '../../mocks/window'; import PaymentServer from 'lib/payment-server'; const PRODUCT_ID = 'pk_8675309'; -const SEARCH_QUERY = '?plan_id=plk_12345'; +const SEARCH_QUERY = '?plan=plk_12345'; describe('views/subscriptions_product_redirect', function() { let account; diff --git a/packages/fxa-content-server/app/tests/spec/views/support.js b/packages/fxa-content-server/app/tests/spec/views/support.js index ef02916dd1f..d44593d4b42 100644 --- a/packages/fxa-content-server/app/tests/spec/views/support.js +++ b/packages/fxa-content-server/app/tests/spec/views/support.js @@ -10,9 +10,10 @@ import Notifier from 'lib/channels/notifier'; import ProfileClient from 'lib/profile-client'; import Relier from 'models/reliers/relier'; import sinon from 'sinon'; +import SupportView from 'views/support'; +import SubscriptionModel from 'models/subscription'; import TestHelpers from '../../lib/helpers'; import User from 'models/user'; -import SupportView from 'views/support'; sinon.spy(AccountByUidMixin.getUidAndSetSignedInAccount); @@ -138,7 +139,7 @@ describe('views/support', function() { }); }); - it('emits plan id and product id correctly', function() { + it('emits subscription.initialize correctly', () => { return view .render() .then(function() { @@ -152,11 +153,11 @@ describe('views/support', function() { .trigger('change'); assert.equal(notifier.trigger.callCount, 5); const args = notifier.trigger.args[4]; - assert.equal(args[0], 'set-plan-and-product-id'); - assert.deepEqual(args[1], { - planId: '123done_9001', - productId: '123done_xyz', - }); + assert.lengthOf(args, 3); + assert.equal(args[0], 'subscription.initialize'); + assert.instanceOf(args[1], SubscriptionModel); + assert.equal(args[1].get('planId'), '123done_9001'); + assert.equal(args[1].get('productId'), '123done_xyz'); }); }); diff --git a/packages/fxa-content-server/app/tests/test_start.js b/packages/fxa-content-server/app/tests/test_start.js index 2fd5a1bb4d9..980cf3de0f0 100644 --- a/packages/fxa-content-server/app/tests/test_start.js +++ b/packages/fxa-content-server/app/tests/test_start.js @@ -123,6 +123,7 @@ require('./spec/models/reliers/relier'); require('./spec/models/reliers/browser'); require('./spec/models/resume-token'); require('./spec/models/security-events'); +require('./spec/models/subscription'); require('./spec/models/support-form'); require('./spec/models/sync-engines'); require('./spec/models/unique-user-id'); diff --git a/packages/fxa-content-server/npm-shrinkwrap.json b/packages/fxa-content-server/npm-shrinkwrap.json index c714ecfe22b..8af473f68cd 100644 --- a/packages/fxa-content-server/npm-shrinkwrap.json +++ b/packages/fxa-content-server/npm-shrinkwrap.json @@ -1180,6 +1180,16 @@ "integrity": "sha512-dBtBbrc+qTHy1WdfHYjBwRln4+LWqASWakLHsWHR2NWHIFkv4W3O070IGoGLEBrJBvct3r0L1BUPuvURi7kYUQ==", "dev": true }, + "@types/backbone": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/@types/backbone/-/backbone-1.4.1.tgz", + "integrity": "sha512-KYfGuQy4d2vvYXbn0uHFZ6brFLndatTMomxBlljpbWf4kFpA3BG/6LA3ec+J9iredrX6eAVI7sm9SVAvwiIM6g==", + "dev": true, + "requires": { + "@types/jquery": "*", + "@types/underscore": "*" + } + }, "@types/benchmark": { "version": "1.0.31", "resolved": "https://registry.npmjs.org/@types/benchmark/-/benchmark-1.0.31.tgz", @@ -1309,6 +1319,15 @@ "@types/istanbul-lib-report": "*" } }, + "@types/jquery": { + "version": "3.3.31", + "resolved": "https://registry.npmjs.org/@types/jquery/-/jquery-3.3.31.tgz", + "integrity": "sha512-Lz4BAJihoFw5nRzKvg4nawXPzutkv7wmfQ5121avptaSIXlDNJCUuxZxX/G+9EVidZGuO0UBlk+YjKbwRKJigg==", + "dev": true, + "requires": { + "@types/sizzle": "*" + } + }, "@types/mime": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/@types/mime/-/mime-2.0.1.tgz", @@ -1337,6 +1356,18 @@ "@types/mime": "*" } }, + "@types/sizzle": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/@types/sizzle/-/sizzle-2.3.2.tgz", + "integrity": "sha512-7EJYyKTL7tFR8+gDbB6Wwz/arpGa0Mywk1TJbNzKzHtzbwVmY4HR9WqS5VV7dsBUKQmPNr192jHr/VpBluj/hg==", + "dev": true + }, + "@types/underscore": { + "version": "1.9.3", + "resolved": "https://registry.npmjs.org/@types/underscore/-/underscore-1.9.3.tgz", + "integrity": "sha512-SwbHKB2DPIDlvYqtK5O+0LFtZAyrUSw4c0q+HWwmH1Ve3KMQ0/5PlV3RX97+3dP7yMrnNQ8/bCWWvQpPl03Mug==", + "dev": true + }, "@types/ws": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/@types/ws/-/ws-6.0.1.tgz", diff --git a/packages/fxa-content-server/package.json b/packages/fxa-content-server/package.json index deb33bc5a3f..e8edb2fd2da 100644 --- a/packages/fxa-content-server/package.json +++ b/packages/fxa-content-server/package.json @@ -132,6 +132,7 @@ "devDependencies": { "@babel/cli": "7.6.2", "@testing-library/react": "^8.0.4", + "@types/backbone": "^1.4.1", "audit-filter": "^0.5.0", "babel-eslint": "9.0.0", "babel-plugin-dynamic-import-webpack": "1.0.2", diff --git a/packages/fxa-content-server/server/lib/routes/post-metrics.js b/packages/fxa-content-server/server/lib/routes/post-metrics.js index fd33cb3724a..f86a1584865 100644 --- a/packages/fxa-content-server/server/lib/routes/post-metrics.js +++ b/packages/fxa-content-server/server/lib/routes/post-metrics.js @@ -123,8 +123,12 @@ const BODY_SCHEMA = { }) .optional(), numStoredAccounts: OFFSET_TYPE.min(0).optional(), + // TODO: Delete plan_id and product_id after the camel-cased equivalents + // have been in place for at least one train. plan_id: STRING_TYPE.optional(), + planId: STRING_TYPE.optional(), product_id: STRING_TYPE.optional(), + productId: STRING_TYPE.optional(), referrer: REFERRER_TYPE.allow('none').required(), screen: joi .object() diff --git a/packages/fxa-shared/metrics/amplitude.js b/packages/fxa-shared/metrics/amplitude.js index 8d0c24cedca..fcb805f1cdf 100644 --- a/packages/fxa-shared/metrics/amplitude.js +++ b/packages/fxa-shared/metrics/amplitude.js @@ -235,8 +235,10 @@ module.exports = { pruneUnsetValues({ service: serviceName, oauth_client_id: clientId, - plan_id: data.plan_id, - product_id: data.product_id, + // TODO: Delete data.plan_id and data.product_id after the camel-cased + // equivalents have been in place for at least one train. + plan_id: data.planId || data.plan_id, + product_id: data.productId || data.product_id, }), EVENT_PROPERTIES[eventGroup]( eventType, diff --git a/packages/fxa-shared/test/metrics/amplitude.js b/packages/fxa-shared/test/metrics/amplitude.js index 5becf02fe4d..425e059e1ed 100644 --- a/packages/fxa-shared/test/metrics/amplitude.js +++ b/packages/fxa-shared/test/metrics/amplitude.js @@ -164,8 +164,8 @@ describe('metrics/amplitude:', () => { marketingOptIn: 'o', os: 'p', osVersion: 'q', - product_id: 'pid', - plan_id: 'plid', + planId: 'plid', + productId: 'pid', region: 'r', service: 'baz', syncEngines: ['wibble', 'blee'], @@ -191,9 +191,9 @@ describe('metrics/amplitude:', () => { device_model: 'm', event_properties: { oauth_client_id: 'baz', - service: 'qux', - product_id: 'pid', plan_id: 'plid', + product_id: 'pid', + service: 'qux', }, event_type: 'fxa_sms - blee', language: 'n',