From e2091156cce15fe07edae2ce0535ed9d344fdff2 Mon Sep 17 00:00:00 2001 From: Jimmy Domagala-Tang Date: Wed, 17 Sep 2025 10:30:22 -0400 Subject: [PATCH] feat: add tests for recurly plan revert feature (#25966) GitOrigin-RevId: 9bb0c198d237a9c86b63da8b4892c7867f79c71f --- .../PaymentProviderEntitiesTest.js | 69 +++++++ .../src/Subscription/RecurlyClientTests.js | 48 +++++ .../SubscriptionControllerTests.js | 168 ++++++++++++++++++ .../Subscription/SubscriptionHandlerTests.js | 154 +++++++++++++++- .../Subscription/SubscriptionUpdaterTests.js | 93 ++++++++++ 5 files changed, 529 insertions(+), 3 deletions(-) diff --git a/services/web/test/unit/src/Subscription/PaymentProviderEntitiesTest.js b/services/web/test/unit/src/Subscription/PaymentProviderEntitiesTest.js index da688c823e..964a99b6ca 100644 --- a/services/web/test/unit/src/Subscription/PaymentProviderEntitiesTest.js +++ b/services/web/test/unit/src/Subscription/PaymentProviderEntitiesTest.js @@ -493,6 +493,75 @@ describe('PaymentProviderEntities', function () { ).to.throw(Errors.AddOnNotPresentError) }) }) + + describe('getRequestForPlanRevert()', function () { + beforeEach(function () { + const { PaymentProviderSubscription } = this.PaymentProviderEntities + this.subscription = new PaymentProviderSubscription({ + id: 'subscription-id', + userId: 'user-id', + planCode: 'regular-plan', + planName: 'My Plan', + planPrice: 10, + addOns: [ + { + addOnCode: 'addon-1', + quantity: 2, + unitAmountInCents: 500, + }, + { + addOnCode: 'addon-2', + quantity: 1, + unitAmountInCents: 600, + }, + ], + subtotal: 10.99, + taxRate: 0.2, + taxAmount: 2.4, + total: 14.4, + currency: 'USD', + }) + }) + + it('throws if the plan to revert to doesnt exist', function () { + const invalidPlanCode = 'non-existent-plan' + expect(() => + this.subscription.getRequestForPlanRevert(invalidPlanCode, null) + ).to.throw('Unable to find plan in settings') + }) + + it('creates a change request with the restore point', function () { + const previousPlanCode = 'cheap-plan' + const previousAddOns = [ + { addOnCode: 'addon-1', quantity: 1, unitAmountInCents: 500 }, + ] + const changeRequest = this.subscription.getRequestForPlanRevert( + previousPlanCode, + previousAddOns + ) + expect(changeRequest).to.be.an.instanceOf( + this.PaymentProviderEntities + .PaymentProviderSubscriptionChangeRequest + ) + expect(changeRequest.planCode).to.equal(previousPlanCode) + expect(changeRequest.addOnUpdates).to.deep.equal([ + { + code: 'addon-1', + quantity: 1, + unitPrice: 5, + }, + ]) + }) + + it('defaults to addons to an empty array to clear the addon state', function () { + const previousPlanCode = 'cheap-plan' + const changeRequest = this.subscription.getRequestForPlanRevert( + previousPlanCode, + null + ) + expect(changeRequest.addOnUpdates).to.deep.equal([]) + }) + }) }) }) diff --git a/services/web/test/unit/src/Subscription/RecurlyClientTests.js b/services/web/test/unit/src/Subscription/RecurlyClientTests.js index 798da8d29f..82923ace96 100644 --- a/services/web/test/unit/src/Subscription/RecurlyClientTests.js +++ b/services/web/test/unit/src/Subscription/RecurlyClientTests.js @@ -116,6 +116,7 @@ describe('RecurlyClient', function () { listAccountSubscriptions: sinon.stub(), listActiveCouponRedemptions: sinon.stub(), previewSubscriptionChange: sinon.stub(), + listSubscriptionInvoices: sinon.stub(), } this.recurly = { errors: recurly.errors, @@ -732,4 +733,51 @@ describe('RecurlyClient', function () { ) }) }) + + describe('getPastDueInvoices', function () { + beforeEach(function () { + this.client.listSubscriptionInvoices = sinon.stub() + }) + + it('should return empty if no past due are found', async function () { + this.client.listSubscriptionInvoices.returns({ + each: async function* () {}, + }) + const invoices = await this.RecurlyClient.promises.getPastDueInvoices( + this.subscription.id + ) + expect(invoices).to.deep.equal([]) + }) + + it('should return past due invoice', async function () { + const pastDueInvoice = { id: 'invoice-1', state: 'past_due' } + this.client.listSubscriptionInvoices.returns({ + each: async function* () { + yield pastDueInvoice + }, + }) + const invoices = await this.RecurlyClient.promises.getPastDueInvoices( + this.subscription.id + ) + expect(invoices).to.deep.equal([pastDueInvoice]) + }) + + it('should return multiple invoices if multiple past due exist', async function () { + const pastDueInvoices = [ + { id: 'invoice-1', state: 'past_due' }, + { id: 'invoice-2', state: 'past_due' }, + ] + this.client.listSubscriptionInvoices.returns({ + each: async function* () { + for (const invoice of pastDueInvoices) { + yield invoice + } + }, + }) + const invoices = await this.RecurlyClient.promises.getPastDueInvoices( + this.subscription.id + ) + expect(invoices).to.deep.equal(pastDueInvoices) + }) + }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 910657305d..66a6a871f3 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -53,6 +53,7 @@ describe('SubscriptionController', function () { syncSubscription: sinon.stub().yields(), attemptPaypalInvoiceCollection: sinon.stub().yields(), startFreeTrial: sinon.stub(), + revertPlanChange: sinon.stub(), promises: { createSubscription: sinon.stub().resolves(), updateSubscription: sinon.stub().resolves(), @@ -80,6 +81,7 @@ describe('SubscriptionController', function () { tax: 0, total: 2000, }), + revertPlanChange: sinon.stub().resolves(), }, } @@ -127,6 +129,9 @@ describe('SubscriptionController', function () { subdomain: 'sl', }, }, + planReverts: { + enabled: false, + }, siteUrl: 'http://de.overleaf.dev:3000', } this.AuthorizationManager = { @@ -688,6 +693,136 @@ describe('SubscriptionController', function () { this.res.sendStatus.calledWith(200) }) }) + + describe('with a failed payment notification', function () { + describe('with planReverts disabled in settings', function () { + beforeEach(function (done) { + this.settings.planReverts = { enabled: false } + this.SubscriptionHandler.revertPlanChange = sinon.stub() + + this.req.body = { + failed_payment_notification: { + transaction: { + subscription_id: 'subscription-123', + }, + }, + } + + this.res = { + sendStatus() { + done() + }, + } + sinon.spy(this.res, 'sendStatus') + this.SubscriptionController.recurlyCallback(this.req, this.res) + }) + it('should not call revertPlanChange', function () { + expect(this.SubscriptionHandler.revertPlanChange.called).to.be.false + }) + + it('should respond with 200', function (done) { + this.res.sendStatus.calledWith(200) + done() + }) + }) + + describe('with planReverts enabled in settings', function () { + beforeEach(function () { + this.settings.planReverts = { enabled: true } + }) + + describe('with no valid restore point', function () { + beforeEach(function (done) { + this.SubscriptionHandler.getSubscriptionRestorePoint = sinon + .stub() + .yields(null, null) + this.SubscriptionHandler.revertPlanChange = sinon.stub() + + this.req.body = { + failed_payment_notification: { + transaction: { + subscription_id: 'subscription-123', + }, + }, + } + this.res = { + sendStatus() { + done() + }, + } + sinon.spy(this.res, 'sendStatus') + this.SubscriptionController.recurlyCallback(this.req, this.res) + }) + it('should not call revertPlanChange()', function () { + expect(this.SubscriptionHandler.revertPlanChange.called).to.be.false + }) + + it('should respond with 200', function () { + this.res.sendStatus.calledWith(200) + }) + }) + + describe('with a valid restore point', function () { + beforeEach(function (done) { + this.addOns = [ + { + addOnCode: 'addon-1', + quantity: 2, + unitAmountInCents: 500, + }, + { + addOnCode: 'addon-2', + quantity: 1, + unitAmountInCents: 600, + }, + ] + this.lastSubscription = { + planCode: 'gold', + addOns: this.addOns, + } + this.SubscriptionHandler.getSubscriptionRestorePoint = sinon + .stub() + .yields(null, this.lastSubscription) + this.SubscriptionHandler.revertPlanChange = sinon.stub().yields() + this.req.body = { + failed_payment_notification: { + transaction: { + subscription_id: 'subscription-123', + }, + }, + } + this.res = { + sendStatus() { + done() + }, + } + sinon.spy(this.res, 'sendStatus') + this.SubscriptionController.recurlyCallback(this.req, this.res) + }) + + it('should get the subscription restore point', function () { + expect( + this.SubscriptionHandler.getSubscriptionRestorePoint.calledWith( + 'subscription-123' + ) + ).to.be.true + }) + + it('should call revertPlanChange()', function () { + expect( + this.SubscriptionHandler.revertPlanChange.calledWith( + 'subscription-123', + this.lastSubscription + ) + ).to.be.true + }) + + it('should respond with 200', function () { + this.res.sendStatus.calledWith(200) + }) + }) + }) + }) }) describe('purchaseAddon', function () { @@ -793,6 +928,39 @@ describe('SubscriptionController', function () { expect(this.FeaturesUpdater.promises.refreshFeatures).to.not.have.been .called }) + + it('should refresh features', async function () { + this.req.params.addOnCode = 'assistant' + this.SubscriptionHandler.promises.purchaseAddon = sinon.stub().resolves() + this.FeaturesUpdater.promises.refreshFeatures = sinon.stub().resolves() + + await this.SubscriptionController.purchaseAddon(this.req, this.res) + + expect( + this.FeaturesUpdater.promises.refreshFeatures.calledWith( + this.user._id, + 'add-on-purchase' + ) + ).to.be.true + }) + + it('should respond with a bad request if the subscription already includes the addOn', async function () { + this.req.params.addOnCode = 'assistant' + this.SubscriptionHandler.promises.purchaseAddon = sinon + .stub() + .rejects(new SubscriptionErrors.DuplicateAddOnError()) + + await this.SubscriptionController.purchaseAddon(this.req, this.res) + + expect( + this.HttpErrorHandler.badRequest.calledWith( + this.req, + this.res, + 'Your subscription already includes this add-on', + { addon: 'assistant' } + ) + ).to.be.true + }) }) describe('checkSubscriptionPauseStatus', function () { diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js index 12ca8c6246..c7b6b3f0a0 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js @@ -14,8 +14,8 @@ const mockRecurlySubscriptions = { 'subscription-123-active': { uuid: 'subscription-123-active', plan: { - name: 'Gold', - plan_code: 'gold', + name: 'Collaborator', + plan_code: 'collaborator', }, current_period_ends_at: new Date(), state: 'active', @@ -107,15 +107,17 @@ describe('SubscriptionHandler', function () { .resolves(this.activeRecurlyClientSubscription), pauseSubscriptionByUuid: sinon.stub().resolves(), resumeSubscriptionByUuid: sinon.stub().resolves(), + failInvoice: sinon.stub(), + getPastDueInvoices: sinon.stub(), }, } - this.SubscriptionUpdater = { promises: { updateSubscriptionFromRecurly: sinon.stub().resolves(), syncSubscription: sinon.stub().resolves(), syncStripeSubscription: sinon.stub().resolves(), startFreeTrial: sinon.stub().resolves(), + setSubscriptionWasReverted: sinon.stub().resolves(), }, } @@ -760,4 +762,150 @@ describe('SubscriptionHandler', function () { }) }) }) + + describe('revertPlanChange', function () { + describe('with correct invoices', function () { + beforeEach(async function () { + this.subscriptionRestorePoint = { + planCode: 'collaborator', + addOns: [ + { addOnCode: 'addon-1', quantity: 1, unitAmountInCents: 500 }, + ], + _id: 'restore-point-id', + } + this.pastDueInvoice = { + id: 'invoice-123', + dueAt: new Date(), + collectionMethod: 'automatic', + } + this.user.id = this.activeRecurlySubscription.account.account_code + this.User.findById = (userId, projection) => ({ + exec: () => { + userId.should.equal(this.user.id) + return Promise.resolve(this.user) + }, + }) + this.RecurlyClient.promises.getSubscription.resolves( + this.activeRecurlyClientSubscription + ) + this.RecurlyClient.promises.getPastDueInvoices.resolves([ + this.pastDueInvoice, + ]) + this.RecurlyClient.promises.failInvoice.resolves() + this.SubscriptionUpdater.promises.setSubscriptionWasReverted.resolves() + this.RecurlyClient.promises.applySubscriptionChangeRequest.resolves() + + await this.SubscriptionHandler.promises.revertPlanChange( + this.activeRecurlyClientSubscription.id, + this.subscriptionRestorePoint + ) + }) + + it('should fetch the subscription from recurly', async function () { + expect( + this.RecurlyClient.promises.getSubscription.calledWith( + this.activeRecurlyClientSubscription.id + ) + ).to.be.true + }) + + it('should fail the invoice', async function () { + expect( + this.RecurlyClient.promises.failInvoice.calledWith( + this.pastDueInvoice.id + ) + ).to.be.true + }) + + it('should call setSubscriptionWasReverted', async function () { + expect( + this.SubscriptionUpdater.promises.setSubscriptionWasReverted.calledWith( + this.subscriptionRestorePoint._id + ) + ).to.be.true + }) + + it('should sync the subscription', async function () { + this.SubscriptionUpdater.promises.syncSubscription.calledOnce.should.equal( + true + ) + this.SubscriptionUpdater.promises.syncSubscription.args[0][0].should.deep.equal( + this.activeRecurlySubscription + ) + this.SubscriptionUpdater.promises.syncSubscription.args[0][1].should.deep.equal( + this.user._id + ) + }) + }) + + describe('should throw an IndeterminateInvoiceError when', function () { + beforeEach(function () { + this.subscriptionRestorePoint = { + planCode: 'collaborator', + addOns: [ + { addOnCode: 'addon-1', quantity: 1, unitAmountInCents: 500 }, + ], + _id: 'restore-point-id', + } + this.RecurlyClient.promises.getSubscription.resolves( + this.activeRecurlyClientSubscription + ) + }) + + it('finds a past due invoice older than 24 hours', async function () { + const oldInvoice = { + id: 'invoice-123', + dueAt: new Date(Date.now() - 25 * 60 * 60 * 1000), // 25 hours ago + collectionMethod: 'automatic', + } + this.RecurlyClient.promises.getPastDueInvoices.resolves([oldInvoice]) + + await expect( + this.SubscriptionHandler.promises.revertPlanChange( + this.activeRecurlyClientSubscription.id, + this.subscriptionRestorePoint + ) + ).to.be.rejectedWith('cant determine invoice to fail for plan revert') + }) + + it('finds more than one past due invoice', async function () { + const invoices = [ + { + id: 'invoice-123', + dueAt: new Date(), + collectionMethod: 'automatic', + }, + { + id: 'invoice-456', + dueAt: new Date(), + collectionMethod: 'automatic', + }, + ] + this.RecurlyClient.promises.getPastDueInvoices.resolves(invoices) + + await expect( + this.SubscriptionHandler.promises.revertPlanChange( + this.activeRecurlyClientSubscription.id, + this.subscriptionRestorePoint + ) + ).to.be.rejectedWith('cant determine invoice to fail for plan revert') + }) + + it('finds an invoice with a collectionMethod other than automatic', async function () { + const manualInvoice = { + id: 'invoice-123', + dueAt: new Date(), + collectionMethod: 'manual', + } + this.RecurlyClient.promises.getPastDueInvoices.resolves([manualInvoice]) + + await expect( + this.SubscriptionHandler.promises.revertPlanChange( + this.activeRecurlyClientSubscription.id, + this.subscriptionRestorePoint + ) + ).to.be.rejectedWith('cant determine invoice to fail for plan revert') + }) + }) + }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js index d272ad51e4..d9ce693837 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js @@ -906,4 +906,97 @@ describe('SubscriptionUpdater', function () { ).to.equal(4) }) }) + describe('setRestorePoint', function () { + it('should set the restore point with the given plan code and add-ons', async function () { + const subscriptionId = new ObjectId() + const planCode = 'gold-plan' + const addOns = [ + { addOnCode: 'addon-1', quantity: 2, unitAmountInCents: 500 }, + { addOnCode: 'addon-2', quantity: 1, unitAmountInCents: 1000 }, + ] + const consumed = false + + await this.SubscriptionUpdater.promises.setRestorePoint( + subscriptionId, + planCode, + addOns, + consumed + ) + + sinon.assert.calledWith( + this.SubscriptionModel.updateOne, + { _id: subscriptionId }, + { + $set: { + 'lastSuccesfulSubscription.planCode': planCode, + 'lastSuccesfulSubscription.addOns': addOns, + }, + } + ) + }) + + it('should increment revertedDueToFailedPayment if consumed is true', async function () { + const consumed = true + const subscriptionId = new ObjectId() + + await this.SubscriptionUpdater.promises.setRestorePoint( + subscriptionId, + null, + null, + consumed + ) + + sinon.assert.calledWith( + this.SubscriptionModel.updateOne, + { _id: subscriptionId }, + { + $set: { + 'lastSuccesfulSubscription.planCode': null, + 'lastSuccesfulSubscription.addOns': null, + }, + $inc: { timesRevertedDueToFailedPayment: 1 }, + } + ) + }) + }) + + describe('setSubscriptionWasReverted', function () { + it('should clear the restore point and mark the subscription as reverted', async function () { + const subscriptionId = new ObjectId().toString() + + await this.SubscriptionUpdater.promises.setSubscriptionWasReverted( + subscriptionId + ) + + this.SubscriptionModel.updateOne.should.have.been.calledWith( + { _id: subscriptionId }, + { + $set: { + 'lastSuccesfulSubscription.planCode': null, + 'lastSuccesfulSubscription.addOns': null, + }, + $inc: { timesRevertedDueToFailedPayment: 1 }, + } + ) + }) + }) + + describe('voidRestorePoint', function () { + it('should clear the restore point without marking the subscription as reverted', async function () { + const subscriptionId = new ObjectId().toString() + + await this.SubscriptionUpdater.promises.voidRestorePoint(subscriptionId) + + sinon.assert.calledWith( + this.SubscriptionModel.updateOne, + { _id: subscriptionId }, + { + $set: { + 'lastSuccesfulSubscription.planCode': null, + 'lastSuccesfulSubscription.addOns': null, + }, + } + ) + }) + }) })