From c192002f539ed2a4652fadf55869007983cd5a0b Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 24 Mar 2022 15:40:40 +0000 Subject: [PATCH] Merge pull request #6837 from overleaf/jpa-drop-v1-subscription-lookup [web] drop support for v1 subscriptions GitOrigin-RevId: ddffa60398d5319959f9e5455520a61fa58fab37 --- .../Features/Subscription/FeaturesUpdater.js | 32 ++++------- .../Subscription/V1SubscriptionManager.js | 44 --------------- .../src/Subscription/FeaturesUpdaterTests.js | 11 +--- .../V1SusbcriptionManagerTests.js | 56 ------------------- 4 files changed, 12 insertions(+), 131 deletions(-) diff --git a/services/web/app/src/Features/Subscription/FeaturesUpdater.js b/services/web/app/src/Features/Subscription/FeaturesUpdater.js index adfedcd955..0f418fceb5 100644 --- a/services/web/app/src/Features/Subscription/FeaturesUpdater.js +++ b/services/web/app/src/Features/Subscription/FeaturesUpdater.js @@ -70,9 +70,13 @@ async function computeFeatures(userId) { const groupFeatureSets = await _getGroupFeatureSets(userId) const institutionFeatures = await InstitutionsFeatures.promises.getInstitutionsFeatures(userId) - const v1Features = await _getV1Features(userId) + const user = await UserGetter.promises.getUser(userId, { + featuresOverrides: 1, + 'overleaf.id': 1, + }) + const v1Features = await _getV1Features(user) const bonusFeatures = await ReferalFeatures.promises.getBonusFeatures(userId) - const featuresOverrides = await _getFeaturesOverrides(userId) + const featuresOverrides = await _getFeaturesOverrides(user) logger.log( { userId, @@ -114,10 +118,7 @@ async function _getGroupFeatureSets(userId) { return (subs || []).map(_subscriptionToFeatures) } -async function _getFeaturesOverrides(userId) { - const user = await UserGetter.promises.getUser(userId, { - featuresOverrides: 1, - }) +async function _getFeaturesOverrides(user) { if (!user || !user.featuresOverrides || user.featuresOverrides.length === 0) { return {} } @@ -138,22 +139,9 @@ async function _getFeaturesOverrides(userId) { return features } -async function _getV1Features(userId) { - let planCode, v1Id - try { - ;({ planCode, v1Id } = - await V1SubscriptionManager.promises.getPlanCodeFromV1(userId)) - } catch (err) { - if (err.name === 'NotFoundError') { - return {} - } else { - throw err - } - } - return FeaturesHelper.mergeFeatures( - V1SubscriptionManager.getGrandfatheredFeaturesForV1User(v1Id) || {}, - _planCodeToFeatures(planCode) - ) +async function _getV1Features(user) { + const v1Id = user?.overleaf?.id + return V1SubscriptionManager.getGrandfatheredFeaturesForV1User(v1Id) || {} } function _subscriptionToFeatures(subscription) { diff --git a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js index 8636df141e..0faa7330e6 100644 --- a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js +++ b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js @@ -12,7 +12,6 @@ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ let V1SubscriptionManager -const logger = require('@overleaf/logger') const UserGetter = require('../User/UserGetter') const request = require('requestretry') const settings = require('@overleaf/settings') @@ -20,46 +19,6 @@ const { V1ConnectionError, NotFoundError } = require('../Errors/Errors') const { promisifyAll } = require('../../util/promises') module.exports = V1SubscriptionManager = { - // Returned planCode = 'v1_pro' | 'v1_pro_plus' | 'v1_student' | 'v1_free' | null - // For this to work, we need plans in settings with plan-codes: - // - 'v1_pro' - // - 'v1_pro_plus' - // - 'v1_student' - // - 'v1_free' - getPlanCodeFromV1(userId, callback) { - if (callback == null) { - callback = function () {} - } - return V1SubscriptionManager._v1Request( - userId, - { - method: 'GET', - url(v1Id) { - return `/api/v1/sharelatex/users/${v1Id}/plan_code` - }, - }, - function (error, body, v1Id) { - if (error != null) { - return callback(error) - } - let planName = body != null ? body.plan_name : undefined - if (['pro', 'pro_plus', 'student', 'free'].includes(planName)) { - planName = `v1_${planName}` - } else { - // Throw away 'anonymous', etc as being equivalent to null - planName = null - } - if (planName != null && planName !== 'v1_free') { - logger.warn( - { v1Id, planName }, - 'got non expired personal subscription' - ) - } - return callback(null, planName, v1Id) - } - ) - }, - getSubscriptionsFromV1(userId, callback) { if (callback == null) { callback = function () {} @@ -229,7 +188,4 @@ function __guard__(value, transform) { module.exports.promises = promisifyAll(module.exports, { without: ['getGrandfatheredFeaturesForV1User'], - multiResult: { - getPlanCodeFromV1: ['planCode', 'v1Id'], - }, }) diff --git a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js index fe98564335..8cb497d7bc 100644 --- a/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/FeaturesUpdaterTests.js @@ -7,11 +7,12 @@ const MODULE_PATH = '../../../../app/src/Features/Subscription/FeaturesUpdater' describe('FeaturesUpdater', function () { beforeEach(function () { + this.v1UserId = 12345 this.user = { _id: new ObjectId(), features: {}, + overleaf: { id: this.v1UserId }, } - this.v1UserId = 12345 this.subscriptions = { individual: { planCode: 'individual-plan' }, group1: { planCode: 'group-plan-1' }, @@ -46,7 +47,6 @@ describe('FeaturesUpdater', function () { { planCode: 'individual-plan', features: { individual: 'features' } }, { planCode: 'group-plan-1', features: { group1: 'features' } }, { planCode: 'group-plan-2', features: { group2: 'features' } }, - { planCode: 'v1-plan', features: { v1: 'features' } }, { planCode: 'no-dropbox', features: { dropbox: false } }, ], features: { @@ -56,7 +56,6 @@ describe('FeaturesUpdater', function () { group1: 'features', group2: 'features', institutions: 'features', - v1: 'features', grandfathered: 'features', bonus: 'features', }, @@ -70,13 +69,7 @@ describe('FeaturesUpdater', function () { } this.V1SubscriptionManager = { getGrandfatheredFeaturesForV1User: sinon.stub(), - promises: { - getPlanCodeFromV1: sinon.stub(), - }, } - this.V1SubscriptionManager.promises.getPlanCodeFromV1 - .withArgs(this.user._id) - .resolves({ planCode: 'v1-plan', v1Id: this.v1UserId }) this.V1SubscriptionManager.getGrandfatheredFeaturesForV1User .withArgs(this.v1UserId) .returns({ grandfathered: 'features' }) diff --git a/services/web/test/unit/src/Subscription/V1SusbcriptionManagerTests.js b/services/web/test/unit/src/Subscription/V1SusbcriptionManagerTests.js index 0693acf0e3..844708874b 100644 --- a/services/web/test/unit/src/Subscription/V1SusbcriptionManagerTests.js +++ b/services/web/test/unit/src/Subscription/V1SusbcriptionManagerTests.js @@ -54,62 +54,6 @@ describe('V1SubscriptionManager', function () { }) }) - describe('getPlanCodeFromV1', function () { - beforeEach(function () { - this.responseBody = { - id: 32, - plan_name: 'pro', - } - this.V1SubscriptionManager._v1Request = sinon - .stub() - .yields(null, this.responseBody) - return (this.call = cb => { - return this.V1SubscriptionManager.getPlanCodeFromV1(this.userId, cb) - }) - }) - - describe('when all goes well', function () { - it('should call _v1Request', function (done) { - return this.call((err, planCode) => { - expect(this.V1SubscriptionManager._v1Request.callCount).to.equal(1) - expect( - this.V1SubscriptionManager._v1Request.calledWith(this.userId) - ).to.equal(true) - return done() - }) - }) - - it('should return the v1 user id', function (done) { - return this.call(function (err, planCode, v1Id) { - expect(v1Id).to.equal(this.v1UserId) - return done() - }) - }) - - it('should produce a plan-code without error', function (done) { - return this.call((err, planCode) => { - expect(err).to.not.exist - expect(planCode).to.equal('v1_pro') - return done() - }) - }) - - describe('when the plan_name from v1 is null', function () { - beforeEach(function () { - return (this.responseBody.plan_name = null) - }) - - it('should produce a null plan-code without error', function (done) { - return this.call((err, planCode) => { - expect(err).to.not.exist - expect(planCode).to.equal(null) - return done() - }) - }) - }) - }) - }) - describe('getGrandfatheredFeaturesForV1User', function () { describe('when the user ID is greater than the cutoff', function () { it('should return an empty feature set', function (done) {