Merge pull request #22386 from overleaf/em-remove-v1-subscription-checks

Remove v1 subscription checks

GitOrigin-RevId: 32962e8a88e98c120c5c58a061f293205c14dc3a

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,
   },
 }

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)

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

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,

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()
-        }
-      )
-    })
-  })
 })

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')

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,
         })