From 9f1784b4c46197303dfd720c0a01f9387df9d2f0 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Wed, 16 Jun 2021 16:22:15 +0200 Subject: [PATCH] Merge pull request #4200 from overleaf/ab-feature-set-user-property Resolve and send feature set user property to analytics GitOrigin-RevId: 08ddd0fe9202b02f7d37547dab1d078bf441a8cf --- .../Features/Subscription/FeaturesUpdater.js | 20 +++++ .../src/Subscription/FeaturesUpdaterTests.js | 75 ++++++++++++++++--- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/services/web/app/src/Features/Subscription/FeaturesUpdater.js b/services/web/app/src/Features/Subscription/FeaturesUpdater.js index ca77ad388d..7952200c7a 100644 --- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/FeaturesUpdater.js @@ -10,6 +10,7 @@ const ReferalFeatures = require('../Referal/ReferalFeatures') const V1SubscriptionManager = require('./V1SubscriptionManager') const InstitutionsFeatures = require('../Institutions/InstitutionsFeatures') const UserGetter = require('../User/UserGetter') +const AnalyticsManager = require('../Analytics/AnalyticsManager') const FeaturesUpdater = { refreshFeatures(userId, reason, callback = () => {}) { @@ -23,6 +24,16 @@ const FeaturesUpdater = { return callback(error) } logger.log({ userId, features }, 'updating user features') + + const matchedFeatureSet = FeaturesUpdater._getMatchedFeatureSet( + features + ) + AnalyticsManager.setUserProperty( + userId, + 'feature-set', + matchedFeatureSet + ) + UserFeaturesUpdater.updateFeatures( userId, features, @@ -301,6 +312,15 @@ const FeaturesUpdater = { } ) }, + + _getMatchedFeatureSet(features) { + for (const [name, featureSet] of Object.entries(Settings.features)) { + if (_.isEqual(features, featureSet)) { + return name + } + } + return 'mixed' + }, } const refreshFeaturesPromise = (userId, reason) => diff --git a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js index def1ec1224..deb9793bdf 100644 --- a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js @@ -13,11 +13,35 @@ describe('FeaturesUpdater', function () { './UserFeaturesUpdater': (this.UserFeaturesUpdater = {}), './SubscriptionLocator': (this.SubscriptionLocator = {}), './PlansLocator': (this.PlansLocator = {}), - 'settings-sharelatex': (this.Settings = {}), + 'settings-sharelatex': (this.Settings = { + features: { + personal: { + collaborators: 1, + dropbox: false, + compileTimeout: 60, + compileGroup: 'standard', + }, + collaborator: { + collaborators: 10, + dropbox: true, + compileTimeout: 240, + compileGroup: 'priority', + }, + professional: { + collaborators: -1, + dropbox: true, + compileTimeout: 240, + compileGroup: 'priority', + }, + }, + }), '../Referal/ReferalFeatures': (this.ReferalFeatures = {}), './V1SubscriptionManager': (this.V1SubscriptionManager = {}), '../Institutions/InstitutionsFeatures': (this.InstitutionsFeatures = {}), '../User/UserGetter': (this.UserGetter = {}), + '../Analytics/AnalyticsManager': (this.AnalyticsManager = { + setUserProperty: sinon.stub(), + }), '../../infrastructure/Modules': (this.Modules = { hooks: { fire: sinon.stub() }, }), @@ -55,7 +79,6 @@ describe('FeaturesUpdater', function () { this.UserGetter.getUser = sinon.stub().yields(null, this.user) this.callback = sinon.stub() }) - it('should return features and featuresChanged', function () { this.FeaturesUpdater.refreshFeatures( this.user_id, @@ -67,7 +90,6 @@ describe('FeaturesUpdater', function () { } ) }) - describe('normally', function () { beforeEach(function () { this.FeaturesUpdater.refreshFeatures( @@ -76,37 +98,31 @@ describe('FeaturesUpdater', function () { this.callback ) }) - it('should get the individual features', function () { this.FeaturesUpdater._getIndividualFeatures .calledWith(this.user_id) .should.equal(true) }) - it('should get the group features', function () { this.FeaturesUpdater._getGroupFeatureSets .calledWith(this.user_id) .should.equal(true) }) - it('should get the institution features', function () { this.InstitutionsFeatures.getInstitutionsFeatures .calledWith(this.user_id) .should.equal(true) }) - it('should get the v1 features', function () { this.FeaturesUpdater._getV1Features .calledWith(this.user_id) .should.equal(true) }) - it('should get the bonus features', function () { this.ReferalFeatures.getBonusFeatures .calledWith(this.user_id) .should.equal(true) }) - it('should merge from the default features', function () { this.FeaturesUpdater._mergeFeatures .calledWith(this.Settings.defaultFeatures) @@ -152,6 +168,47 @@ describe('FeaturesUpdater', function () { .should.equal(true) }) }) + + describe('analytics user properties', function () { + it('should send the corresponding feature set user property', function () { + this.FeaturesUpdater._mergeFeatures = sinon + .stub() + .returns(this.Settings.features.personal) + + this.FeaturesUpdater.refreshFeatures( + this.user_id, + 'test', + this.callback + ) + + sinon.assert.calledWith( + this.AnalyticsManager.setUserProperty, + this.user_id, + 'feature-set', + 'personal' + ) + }) + + it('should send mixed feature set user property', function () { + this.FeaturesUpdater._mergeFeatures = sinon + .stub() + .returns({ dropbox: true, feature: 'some' }) + + this.FeaturesUpdater.refreshFeatures( + this.user_id, + 'test', + this.callback + ) + + sinon.assert.calledWith( + this.AnalyticsManager.setUserProperty, + this.user_id, + 'feature-set', + 'mixed' + ) + }) + }) + describe('when losing dropbox feature', function () { beforeEach(function () { this.user = {