From 53d84efb3a571beaef0dd53730138e68f6ec9016 Mon Sep 17 00:00:00 2001 From: Miguel Serrano Date: Mon, 11 Nov 2024 12:16:55 +0100 Subject: [PATCH] =?UTF-8?q?Revert=20"[web]=20Send=20email=20to=20support?= =?UTF-8?q?=20when=20skipping=20deleting=20pro=20group=20subscrip=E2=80=A6?= =?UTF-8?q?"=20(#21780)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 8a4abe0742ab6a1c6b15bb17b5cc4863a11010b2. GitOrigin-RevId: 2b91b8a127d195f4297fe99653fb69664b2a91a7 --- .../Subscription/SubscriptionUpdater.js | 88 +++++-------------- .../Subscription/SubscriptionUpdaterTests.js | 58 ------------ 2 files changed, 20 insertions(+), 126 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js index fae13eec00..704335955e 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js +++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js @@ -11,8 +11,6 @@ const logger = require('@overleaf/logger') const Features = require('../../infrastructure/Features') const UserAuditLogHandler = require('../User/UserAuditLogHandler') const { SSOConfig } = require('../../models/SSOConfig') -const Modules = require('../../infrastructure/Modules') -const Settings = require('@overleaf/settings') /** * Change the admin of the given subscription. @@ -251,29 +249,6 @@ async function _deleteAndReplaceSubscriptionFromRecurly( ) } -async function _notifySupportSubscriptionDeletionSkipped( - subscription, - recurlySubscription, - hasGroupSSOEnabled -) { - const adminUrl = `${Settings.adminUrl + '/admin/user/' + subscription.admin_id}` - const groupUrl = `${Settings.adminUrl + '/admin/group/' + subscription._id}` - let message = `\n**Recurly account:** ${recurlySubscription?.account?.url}` - message += `\n**Group admin:** ${adminUrl}` - message += `\n**Group:** ${groupUrl}` - message += `\n**Managed users enabled:** ${subscription?.managedUsersEnabled}` - message += `\n**SSO enabled:** ${hasGroupSSOEnabled}` - - const data = { - subject: 'Skipped deleting pro group subscription', - inbox: 'support', - tags: 'Group subscription', - message, - } - - await Modules.promises.hooks.fire('sendSupportRequest', data) -} - async function updateSubscriptionFromRecurly( recurlySubscription, subscription, @@ -283,18 +258,6 @@ async function updateSubscriptionFromRecurly( const hasManagedUsersFeature = Features.hasFeature('saas') && subscription?.managedUsersEnabled - let hasGroupSSOEnabled = false - if (subscription?.ssoConfig) { - const ssoConfig = await SSOConfig.findOne({ - _id: subscription.ssoConfig._id || subscription.ssoConfig, - }) - .lean() - .exec() - if (ssoConfig.enabled) { - hasGroupSSOEnabled = true - } - } - // If a payment lapses and if the group is managed or has group SSO, as a temporary measure we need to // make sure that the group continues as-is and no destructive actions are taken. if (hasManagedUsersFeature) { @@ -302,39 +265,28 @@ async function updateSubscriptionFromRecurly( { subscriptionId: subscription._id }, 'expired subscription has managedUsers feature enabled, skipping deletion' ) - try { - await _notifySupportSubscriptionDeletionSkipped( - subscription, - recurlySubscription, - hasGroupSSOEnabled - ) - } catch (e) { - logger.warn( - { subscriptionId: subscription._id }, - 'unable to send notification to support that subscription deletion was skipped' - ) - } - } else if (hasGroupSSOEnabled) { - logger.warn( - { subscriptionId: subscription._id }, - 'expired subscription has groupSSO feature enabled, skipping deletion' - ) - try { - await _notifySupportSubscriptionDeletionSkipped( - subscription, - recurlySubscription, - hasGroupSSOEnabled - ) - } catch (e) { - logger.warn( - { subscriptionId: subscription._id }, - 'unable to send notification to support that subscription deletion was skipped' - ) - } } else { - await deleteSubscription(subscription, requesterData) - } + let hasGroupSSOEnabled = false + if (subscription?.ssoConfig) { + const ssoConfig = await SSOConfig.findOne({ + _id: subscription.ssoConfig._id || subscription.ssoConfig, + }) + .lean() + .exec() + if (ssoConfig.enabled) { + hasGroupSSOEnabled = true + } + } + if (hasGroupSSOEnabled) { + logger.warn( + { subscriptionId: subscription._id }, + 'expired subscription has groupSSO feature enabled, skipping deletion' + ) + } else { + await deleteSubscription(subscription, requesterData) + } + } return } const updatedPlanCode = recurlySubscription.plan.plan_code diff --git a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js index e0e8d666bf..3d8212201f 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js @@ -13,9 +13,6 @@ describe('SubscriptionUpdater', function () { plan: { plan_code: this.recurlyPlan.planCode, }, - account: { - url: 'test_url', - }, } this.adminUser = { _id: (this.adminuser_id = '5208dd34438843e2db000007') } @@ -29,7 +26,6 @@ describe('SubscriptionUpdater', function () { save: sinon.stub().resolves(), planCode: 'student_or_something', recurlySubscription_id: 'abc123def456fab789', - managedUsersEnabled: false, } this.user_id = this.adminuser_id @@ -124,7 +120,6 @@ describe('SubscriptionUpdater', function () { }, }, ], - adminUrl: 'test_admin_url', } this.UserFeaturesUpdater = { @@ -182,13 +177,6 @@ describe('SubscriptionUpdater', function () { '../Analytics/AnalyticsManager': this.AnalyticsManager, '../../infrastructure/Features': this.Features, '../User/UserAuditLogHandler': this.UserAuditLogHandler, - '../../infrastructure/Modules': (this.Modules = { - promises: { - hooks: { - fire: sinon.stub().resolves(), - }, - }, - }), }, }) }) @@ -328,52 +316,6 @@ describe('SubscriptionUpdater', function () { {} ) this.SubscriptionModel.deleteOne.should.not.have.been.called - const adminUrl = `${this.Settings.adminUrl + '/admin/user/' + this.subscription.admin_id}` - const groupUrl = `${this.Settings.adminUrl + '/admin/group/' + this.subscription._id}` - let message = `\n**Recurly account:** ${this.recurlySubscription.account?.url}` - message += `\n**Group admin:** ${adminUrl}` - message += `\n**Group:** ${groupUrl}` - message += `\n**Managed users enabled:** false` - message += `\n**SSO enabled:** true` - expect(this.Modules.promises.hooks.fire).to.have.been.calledOnce - expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( - 'sendSupportRequest', - { - subject: 'Skipped deleting pro group subscription', - inbox: 'support', - tags: 'Group subscription', - message, - } - ) - }) - - it('should not remove the subscription when expired if it has managed users is enabled', async function () { - this.Features.hasFeature.withArgs('saas').returns(true) - this.subscription.managedUsersEnabled = true - - this.recurlySubscription.state = 'expired' - await this.SubscriptionUpdater.promises.updateSubscriptionFromRecurly( - this.recurlySubscription, - this.subscription, - {} - ) - this.SubscriptionModel.deleteOne.should.not.have.been.called - const adminUrl = `${this.Settings.adminUrl + '/admin/user/' + this.subscription.admin_id}` - const groupUrl = `${this.Settings.adminUrl + '/admin/group/' + this.subscription._id}` - let message = `\n**Recurly account:** ${this.recurlySubscription.account?.url}` - message += `\n**Group admin:** ${adminUrl}` - message += `\n**Group:** ${groupUrl}` - message += `\n**Managed users enabled:** true` - message += `\n**SSO enabled:** false` - expect(this.Modules.promises.hooks.fire).to.have.been.calledWith( - 'sendSupportRequest', - { - subject: 'Skipped deleting pro group subscription', - inbox: 'support', - tags: 'Group subscription', - message, - } - ) }) it('should update all the users features', async function () {