diff --git a/services/web/app/src/Features/Subscription/LimitationsManager.js b/services/web/app/src/Features/Subscription/LimitationsManager.js index a2dba7c02d..717d641389 100644 --- a/services/web/app/src/Features/Subscription/LimitationsManager.js +++ b/services/web/app/src/Features/Subscription/LimitationsManager.js @@ -5,8 +5,6 @@ const SubscriptionLocator = require('./SubscriptionLocator') const Settings = require('@overleaf/settings') const CollaboratorsGetter = require('../Collaborators/CollaboratorsGetter') const CollaboratorsInvitesGetter = require('../Collaborators/CollaboratorsInviteGetter') -const V1SubscriptionManager = require('./V1SubscriptionManager') -const { V1ConnectionError } = require('../Errors/Errors') const { callbackify, callbackifyMultiResult, @@ -73,18 +71,11 @@ async function canAddXEditCollaborators( } async function hasPaidSubscription(user) { - const { hasSubscription, subscription } = await userHasV2Subscription(user) + const { hasSubscription, subscription } = await userHasSubscription(user) const { isMember } = await userIsMemberOfGroupSubscription(user) - try { - const hasV1Subscription = await userHasV1Subscription(user) - return { - hasPaidSubscription: hasSubscription || isMember || hasV1Subscription, - subscription, - } - } catch (err) { - throw new V1ConnectionError('error getting subscription from v1').withCause( - err - ) + return { + hasPaidSubscription: hasSubscription || isMember, + subscription, } } @@ -93,7 +84,7 @@ async function userHasSubscriptionOrIsGroupMember(user) { return await hasPaidSubscription(user) } -async function userHasV2Subscription(user) { +async function userHasSubscription(user) { const subscription = await SubscriptionLocator.promises.getUsersSubscription( user._id ) @@ -109,31 +100,12 @@ async function userHasV2Subscription(user) { } } -async function userHasV1OrV2Subscription(user) { - const { hasSubscription: hasV2Subscription } = - await userHasV2Subscription(user) - if (hasV2Subscription) { - return true - } - const hasV1Subscription = await userHasV1Subscription(user) - if (hasV1Subscription) { - return true - } - return false -} - async function userIsMemberOfGroupSubscription(user) { const subscriptions = (await SubscriptionLocator.promises.getMemberSubscriptions(user._id)) || [] return { isMember: subscriptions.length > 0, subscriptions } } -async function userHasV1Subscription(user, callback) { - const v1Subscription = - await V1SubscriptionManager.promises.getSubscriptionsFromV1(user._id) - return !!(v1Subscription ? v1Subscription.has_subscription : undefined) -} - function teamHasReachedMemberLimit(subscription) { const currentTotal = (subscription.member_ids || []).length + @@ -171,16 +143,14 @@ const LimitationsManager = { userHasSubscriptionOrIsGroupMember, ['hasPaidSubscription', 'subscription'] ), - userHasV2Subscription: callbackifyMultiResult(userHasV2Subscription, [ + userHasSubscription: callbackifyMultiResult(userHasSubscription, [ 'hasSubscription', 'subscription', ]), - userHasV1OrV2Subscription: callbackify(userHasV1OrV2Subscription), userIsMemberOfGroupSubscription: callbackifyMultiResult( userIsMemberOfGroupSubscription, ['isMember', 'subscriptions'] ), - userHasV1Subscription: callbackify(userHasV1Subscription), hasGroupMembersLimitReached: callbackifyMultiResult( hasGroupMembersLimitReached, ['limitReached', 'subscription'] @@ -196,10 +166,8 @@ const LimitationsManager = { canAddXEditCollaborators, hasPaidSubscription, userHasSubscriptionOrIsGroupMember, - userHasV2Subscription, - userHasV1OrV2Subscription, + userHasSubscription, userIsMemberOfGroupSubscription, - userHasV1Subscription, hasGroupMembersLimitReached, }, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 825befc344..9175a520bd 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -253,8 +253,8 @@ async function userSubscriptionPage(req, res) { managedInstitutions, managedPublishers, } = results - const hasSubscription = - await LimitationsManager.promises.userHasV1OrV2Subscription(user) + const { hasSubscription } = + await LimitationsManager.promises.userHasSubscription(user) const userCanExtendTrial = ( await Modules.promises.hooks.fire('userCanExtendTrial', user) @@ -353,8 +353,8 @@ async function interstitialPaymentPage(req, res) { const latamCountryBannerDetails = await getLatamCountryBannerDetails(req, res) - const hasSubscription = - await LimitationsManager.promises.userHasV1OrV2Subscription(user) + const { hasSubscription } = + await LimitationsManager.promises.userHasSubscription(user) const showSkipLink = req.query?.skipLink === 'true' if (hasSubscription) { @@ -749,7 +749,7 @@ function recurlyCallback(req, res, next) { async function extendTrial(req, res) { const user = SessionManager.getSessionUser(req.session) const { subscription } = - await LimitationsManager.promises.userHasV2Subscription(user) + await LimitationsManager.promises.userHasSubscription(user) const allowed = ( await Modules.promises.hooks.fire('userCanExtendTrial', user) diff --git a/services/web/app/src/Features/Subscription/SubscriptionHandler.js b/services/web/app/src/Features/Subscription/SubscriptionHandler.js index 63d69ac460..fe565e1bbb 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionHandler.js @@ -90,7 +90,7 @@ async function updateSubscription(user, planCode, couponCode) { try { ;({ hasSubscription, subscription } = - await LimitationsManager.promises.userHasV2Subscription(user)) + await LimitationsManager.promises.userHasSubscription(user)) } catch (err) { logger.warn( { err, userId: user._id }, @@ -132,7 +132,7 @@ async function updateSubscription(user, planCode, couponCode) { */ async function cancelPendingSubscriptionChange(user) { const { hasSubscription, subscription } = - await LimitationsManager.promises.userHasV2Subscription(user) + await LimitationsManager.promises.userHasSubscription(user) if (hasSubscription && subscription != null) { await RecurlyClient.promises.removeSubscriptionChangeByUuid( @@ -147,7 +147,7 @@ async function cancelPendingSubscriptionChange(user) { async function cancelSubscription(user) { try { const { hasSubscription, subscription } = - await LimitationsManager.promises.userHasV2Subscription(user) + await LimitationsManager.promises.userHasSubscription(user) if (hasSubscription && subscription != null) { await RecurlyClient.promises.cancelSubscriptionByUuid( subscription.recurlySubscription_id @@ -178,7 +178,7 @@ async function cancelSubscription(user) { async function reactivateSubscription(user) { try { const { hasSubscription, subscription } = - await LimitationsManager.promises.userHasV2Subscription(user) + await LimitationsManager.promises.userHasSubscription(user) if (hasSubscription && subscription != null) { await RecurlyClient.promises.reactivateSubscriptionByUuid( subscription.recurlySubscription_id diff --git a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js index 4408079b7c..6a852eb974 100644 --- a/services/web/app/src/Features/Subscription/V1SubscriptionManager.js +++ b/services/web/app/src/Features/Subscription/V1SubscriptionManager.js @@ -6,19 +6,6 @@ const { V1ConnectionError, NotFoundError } = require('../Errors/Errors') const { promisifyAll } = require('@overleaf/promise-utils') module.exports = V1SubscriptionManager = { - getSubscriptionsFromV1(userId, callback) { - V1SubscriptionManager._v1Request( - userId, - { - method: 'GET', - url(v1Id) { - return `/api/v1/overleaf/users/${v1Id}/subscriptions` - }, - }, - callback - ) - }, - cancelV1Subscription(userId, callback) { V1SubscriptionManager._v1Request( userId, diff --git a/services/web/test/unit/src/Subscription/LimitationsManagerTests.js b/services/web/test/unit/src/Subscription/LimitationsManagerTests.js index b4d39756c2..694183547a 100644 --- a/services/web/test/unit/src/Subscription/LimitationsManagerTests.js +++ b/services/web/test/unit/src/Subscription/LimitationsManagerTests.js @@ -48,12 +48,6 @@ describe('LimitationsManager', function () { }, } - this.V1SubscriptionManager = { - promises: { - getSubscriptionsFromV1: sinon.stub().resolves(), - }, - } - this.CollaboratorsGetter = { promises: { getInvitedCollaboratorCount: sinon.stub().resolves(), @@ -560,7 +554,7 @@ describe('LimitationsManager', function () { }) }) - describe('userHasV2Subscription', function () { + describe('userHasSubscription', function () { beforeEach(function () { this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() @@ -573,7 +567,7 @@ describe('LimitationsManager', function () { .resolves({ recurlySubscription_id: '1234', }) - this.LimitationsManager.userHasV2Subscription( + this.LimitationsManager.userHasSubscription( this.user, (err, hasSubscription) => { assert.equal(err, null) @@ -586,7 +580,7 @@ describe('LimitationsManager', function () { it('should return false if the recurly token is not set', function (done) { this.SubscriptionLocator.promises.getUsersSubscription.resolves({}) this.subscription = {} - this.LimitationsManager.userHasV2Subscription( + this.LimitationsManager.userHasSubscription( this.user, (err, hasSubscription) => { assert.equal(err, null) @@ -598,7 +592,7 @@ describe('LimitationsManager', function () { it('should return false if the subscription is undefined', function (done) { this.SubscriptionLocator.promises.getUsersSubscription.resolves() - this.LimitationsManager.userHasV2Subscription( + this.LimitationsManager.userHasSubscription( this.user, (err, hasSubscription) => { assert.equal(err, null) @@ -613,7 +607,7 @@ describe('LimitationsManager', function () { this.SubscriptionLocator.promises.getUsersSubscription.resolves( stubbedSubscription ) - this.LimitationsManager.userHasV2Subscription( + this.LimitationsManager.userHasSubscription( this.user, (err, hasSubOrIsGroupMember, subscription) => { assert.equal(err, null) @@ -632,7 +626,7 @@ describe('LimitationsManager', function () { }) it('should return true', function (done) { - this.LimitationsManager.userHasV2Subscription( + this.LimitationsManager.userHasSubscription( this.user, (err, hasSubscription, subscription) => { assert.equal(err, null) @@ -643,7 +637,7 @@ describe('LimitationsManager', function () { }) it('should return the subscription', function (done) { - this.LimitationsManager.userHasV2Subscription( + this.LimitationsManager.userHasSubscription( this.user, (err, hasSubscription, subscription) => { assert.equal(err, null) @@ -699,9 +693,6 @@ describe('LimitationsManager', function () { this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() .resolves(null) - this.V1SubscriptionManager.promises.userHasV1Subscription = sinon - .stub() - .yields(null, null) }) it('should return true if userIsMemberOfGroupSubscription', function (done) { @@ -718,7 +709,7 @@ describe('LimitationsManager', function () { ) }) - it('should return true if userHasV2Subscription', function (done) { + it('should return true if userHasSubscription', function (done) { this.SubscriptionLocator.promises.getUsersSubscription = sinon .stub() .resolves({ recurlySubscription_id: '123' }) @@ -732,20 +723,6 @@ describe('LimitationsManager', function () { ) }) - it('should return true if userHasV1Subscription', function (done) { - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 = sinon - .stub() - .resolves({ has_subscription: true }) - this.LimitationsManager.hasPaidSubscription( - this.user, - (err, hasSubOrIsGroupMember) => { - assert.equal(err, null) - hasSubOrIsGroupMember.should.equal(true) - done() - } - ) - }) - it('should return false if none are true', function (done) { this.LimitationsManager.hasPaidSubscription( this.user, @@ -769,56 +746,6 @@ describe('LimitationsManager', function () { }) }) - describe('userHasV1OrV2Subscription', function () { - beforeEach(function () { - this.SubscriptionLocator.promises.getUsersSubscription = sinon - .stub() - .resolves() - this.V1SubscriptionManager.promises.userHasV1Subscription = sinon - .stub() - .resolves() - }) - - it('should return true if userHasV2Subscription', function (done) { - this.SubscriptionLocator.promises.getUsersSubscription = sinon - .stub() - .resolves({ recurlySubscription_id: '123' }) - this.LimitationsManager.userHasV1OrV2Subscription( - this.user, - (err, hasSub) => { - assert.equal(err, null) - hasSub.should.equal(true) - done() - } - ) - }) - - it('should return true if userHasV1Subscription', function (done) { - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 = sinon - .stub() - .resolves({ has_subscription: true }) - this.LimitationsManager.userHasV1OrV2Subscription( - this.user, - (err, hasSub) => { - assert.equal(err, null) - hasSub.should.equal(true) - done() - } - ) - }) - - it('should return false if none are true', function (done) { - this.LimitationsManager.userHasV1OrV2Subscription( - this.user, - (err, hasSub) => { - assert.equal(err, null) - hasSub.should.equal(false) - done() - } - ) - }) - }) - describe('hasGroupMembersLimitReached', function () { beforeEach(function () { this.subscriptionId = '12312' @@ -875,57 +802,4 @@ describe('LimitationsManager', function () { ) }) }) - - describe('userHasV1Subscription', function () { - it('should return true if v1 returns has_subscription = true', function (done) { - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 = sinon - .stub() - .resolves({ has_subscription: true }) - this.LimitationsManager.userHasV1Subscription( - this.user, - (error, result) => { - assert.equal(error, null) - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 - .calledWith(this.userId) - .should.equal(true) - result.should.equal(true) - done() - } - ) - }) - - it('should return false if v1 returns has_subscription = false', function (done) { - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 = sinon - .stub() - .resolves({ has_subscription: false }) - this.LimitationsManager.userHasV1Subscription( - this.user, - (error, result) => { - assert.equal(error, null) - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 - .calledWith(this.userId) - .should.equal(true) - result.should.equal(false) - done() - } - ) - }) - - it('should return false if v1 returns nothing', function (done) { - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 = sinon - .stub() - .resolves({ has_subscription: false }) - this.LimitationsManager.userHasV1Subscription( - this.user, - (error, result) => { - assert.equal(error, null) - this.V1SubscriptionManager.promises.getSubscriptionsFromV1 - .calledWith(this.userId) - .should.equal(true) - result.should.equal(false) - done() - } - ) - }) - }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 51f16b16c9..79dcc3234b 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -61,12 +61,12 @@ describe('SubscriptionController', function () { this.LimitationsManager = { hasPaidSubscription: sinon.stub(), - userHasV1OrV2Subscription: sinon.stub(), - userHasV2Subscription: sinon.stub(), + userHasSubscription: sinon + .stub() + .yields(null, { hasSubscription: false }), promises: { hasPaidSubscription: sinon.stub().resolves(), - userHasV1OrV2Subscription: sinon.stub().resolves(), - userHasV2Subscription: sinon.stub().resolves(), + userHasSubscription: sinon.stub().resolves({ hasSubscription: false }), }, } @@ -756,9 +756,9 @@ describe('SubscriptionController', function () { describe('with a user with subscription', function () { it('should redirect to the subscription dashboard', function (done) { - this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves( - true - ) + this.LimitationsManager.promises.userHasSubscription.resolves({ + hasSubscription: true, + }) this.res.redirect = url => { url.should.equal('/user/subscription?hasSubscription=true') done() @@ -954,7 +954,9 @@ describe('SubscriptionController', function () { planCodesChangingAtTermEnd: [], } ) - this.LimitationsManager.promises.userHasV1OrV2Subscription.resolves(false) + this.LimitationsManager.promises.userHasSubscription.resolves({ + hasSubscription: false, + }) this.res.render = (view, data) => { this.data = data expect(view).to.equal('subscriptions/dashboard-react') diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js index 87fde5700b..1a86b0215f 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js @@ -118,7 +118,7 @@ describe('SubscriptionHandler', function () { this.LimitationsManager = { promises: { - userHasV2Subscription: sinon.stub().resolves(), + userHasSubscription: sinon.stub().resolves(), }, } @@ -258,7 +258,7 @@ describe('SubscriptionHandler', function () { }, }) this.plan_code = 'professional' - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: true, subscription: this.subscription, }) @@ -304,7 +304,7 @@ describe('SubscriptionHandler', function () { }, }) - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: true, subscription: this.subscription, }) @@ -326,7 +326,7 @@ describe('SubscriptionHandler', function () { describe('with a user without a subscription', function () { beforeEach(async function () { - this.LimitationsManager.promises.userHasV2Subscription.resolves(false) + this.LimitationsManager.promises.userHasSubscription.resolves(false) await this.SubscriptionHandler.promises.updateSubscription( this.user, this.plan_code, @@ -356,7 +356,7 @@ describe('SubscriptionHandler', function () { }) this.plan_code = 'collaborator' this.coupon_code = '1231312' - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: true, subscription: this.subscription, }) @@ -399,7 +399,7 @@ describe('SubscriptionHandler', function () { describe('cancelSubscription', function () { describe('with a user without a subscription', function () { beforeEach(async function () { - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: false, subscription: this.subscription, }) @@ -415,7 +415,7 @@ describe('SubscriptionHandler', function () { describe('with a user with a subscription', function () { beforeEach(async function () { - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: true, subscription: this.subscription, }) @@ -445,7 +445,7 @@ describe('SubscriptionHandler', function () { describe('reactivateSubscription', function () { describe('with a user without a subscription', function () { beforeEach(async function () { - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: false, subscription: this.subscription, }) @@ -467,7 +467,7 @@ describe('SubscriptionHandler', function () { describe('with a user with a subscription', function () { beforeEach(async function () { - this.LimitationsManager.promises.userHasV2Subscription.resolves({ + this.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: true, subscription: this.subscription, })