From 84996ea88c964ced971d3fed77e07da1b665ee58 Mon Sep 17 00:00:00 2001 From: Thomas Mees Date: Thu, 13 Mar 2025 13:04:39 +0100 Subject: [PATCH] Implement checks for user eligibility when switching plans (#24276) * Convert updateSubscription controller to async/await * Move updateSubscription to subscription module * Validate if user is eligible to change plan GitOrigin-RevId: ce538429cd5a3b93acabdc046f1a8b164ac02301 --- .../Subscription/SubscriptionController.js | 25 ------------- .../Subscription/SubscriptionRouter.mjs | 6 ---- .../web/frontend/extracted-translations.json | 1 + .../change-plan/change-to-group-plan.tsx | 36 ++++++++++++++++--- services/web/locales/en.json | 1 + .../SubscriptionControllerTests.js | 30 ++-------------- 6 files changed, 36 insertions(+), 63 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index 0b63a56ccb..cfd10a08ba 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -458,30 +458,6 @@ async function previewSubscription(req, res, next) { res.render('subscriptions/preview-change', { changePreview }) } -function updateSubscription(req, res, next) { - const origin = req && req.query ? req.query.origin : null - const user = SessionManager.getSessionUser(req.session) - const planCode = req.body.plan_code - if (planCode == null) { - const err = new Error('plan_code is not defined') - logger.warn( - { userId: user._id, err, planCode, origin, body: req.body }, - '[Subscription] error in updateSubscription form' - ) - return next(err) - } - logger.debug({ planCode, userId: user._id }, 'updating subscription') - SubscriptionHandler.updateSubscription(user, planCode, null, function (err) { - if (err) { - OError.tag(err, 'something went wrong updating subscription', { - user_id: user._id, - }) - return next(err) - } - res.redirect('/user/subscription') - }) -} - function cancelPendingSubscriptionChange(req, res, next) { const user = SessionManager.getSessionUser(req.session) logger.debug({ userId: user._id }, 'canceling pending subscription change') @@ -799,7 +775,6 @@ module.exports = { canceledSubscription: expressify(canceledSubscription), cancelV1Subscription, previewSubscription: expressify(previewSubscription), - updateSubscription, cancelPendingSubscriptionChange, updateAccountEmailAddress, reactivateSubscription, diff --git a/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs b/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs index 04b44aca1c..67b5538bf8 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionRouter.mjs @@ -183,12 +183,6 @@ export default { RateLimiterMiddleware.rateLimit(subscriptionRateLimiter), SubscriptionController.previewSubscription ) - webRouter.post( - '/user/subscription/update', - AuthenticationController.requireLogin(), - RateLimiterMiddleware.rateLimit(subscriptionRateLimiter), - SubscriptionController.updateSubscription - ) webRouter.get( '/user/subscription/addon/:addOnCode/add', AuthenticationController.requireLogin(), diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 3e7f70ac8e..431bf83252 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -1543,6 +1543,7 @@ "sorry_there_are_no_experiments": "", "sorry_there_was_an_issue_adding_x_users_to_your_subscription": "", "sorry_there_was_an_issue_upgrading_your_subscription": "", + "sorry_you_can_only_change_to_group_from_trial_via_support": "", "sorry_your_table_cant_be_displayed_at_the_moment": "", "sort_by": "", "sort_by_x": "", diff --git a/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/change-to-group-plan.tsx b/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/change-to-group-plan.tsx index ed7e929916..2b0b153cae 100644 --- a/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/change-to-group-plan.tsx +++ b/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/change-to-group-plan.tsx @@ -1,10 +1,20 @@ import { useTranslation } from 'react-i18next' import { useSubscriptionDashboardContext } from '../../../../../context/subscription-dashboard-context' import OLButton from '@/features/ui/components/ol/ol-button' +import OLTooltip from '@/features/ui/components/ol/ol-tooltip' +import isInFreeTrial from '../../../../../util/is-in-free-trial' +import { RecurlySubscription } from '../../../../../../../../../types/subscription/dashboard/subscription' export function ChangeToGroupPlan() { const { t } = useTranslation() - const { handleOpenModal } = useSubscriptionDashboardContext() + const { handleOpenModal, personalSubscription } = + useSubscriptionDashboardContext() + + // TODO: Better way to get RecurlySubscription/trial_ends_at + const subscription = + personalSubscription && 'recurly' in personalSubscription + ? (personalSubscription as RecurlySubscription) + : null const handleClick = () => { handleOpenModal('change-to-group') @@ -15,9 +25,27 @@ export function ChangeToGroupPlan() {

{t('looking_multiple_licenses')}

{t('reduce_costs_group_licenses')}


- - {t('change_to_group_plan')} - + {isInFreeTrial(subscription?.recurly?.trial_ends_at) ? ( + <> + +
+ + {t('change_to_group_plan')} + +
+ + + ) : ( + + {t('change_to_group_plan')} + + )} ) } diff --git a/services/web/locales/en.json b/services/web/locales/en.json index cf755629db..c293511896 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -2023,6 +2023,7 @@ "sorry_there_was_an_issue_adding_x_users_to_your_subscription": "Sorry, there was an issue adding __count__ users to your subscription. Please <0>contact our Support team for help.", "sorry_there_was_an_issue_upgrading_your_subscription": "Sorry, there was an issue upgrading your subscription. Please <0>contact our Support team for help.", "sorry_this_account_has_been_suspended": "Sorry, this account has been suspended.", + "sorry_you_can_only_change_to_group_from_trial_via_support": "Sorry, you can only change to a group plan during a free trial by contacting support.", "sorry_your_table_cant_be_displayed_at_the_moment": "Sorry, your table can’t be displayed at the moment.", "sorry_your_token_expired": "Sorry, your token expired", "sort_by": "Sort by", diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js index 602fb574ef..ef4b055dc3 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js @@ -175,9 +175,9 @@ describe('SubscriptionController', function () { recordEventForSession: sinon.stub(), setUserPropertyForUser: sinon.stub(), }), - '../../infrastructure/Modules': { + '../../infrastructure/Modules': (this.Modules = { promises: { hooks: { fire: sinon.stub().resolves() } }, - }, + }), '../../infrastructure/Features': this.Features, '../../util/currency': (this.currency = { formatCurrency: sinon.stub(), @@ -293,32 +293,6 @@ describe('SubscriptionController', function () { }) }) - describe('updateSubscription via post', function () { - beforeEach(function (done) { - this.res = { - redirect() { - done() - }, - } - sinon.spy(this.res, 'redirect') - this.plan_code = '1234' - this.req.body.plan_code = this.plan_code - this.SubscriptionController.updateSubscription(this.req, this.res) - }) - - it('should send the user and subscriptionId to the handler', function (done) { - this.SubscriptionHandler.updateSubscription - .calledWith(this.user, this.plan_code) - .should.equal(true) - done() - }) - - it('should redurect to the subscription page', function (done) { - this.res.redirect.calledWith('/user/subscription').should.equal(true) - done() - }) - }) - describe('updateAccountEmailAddress via put', function () { it('should send the user and subscriptionId to RecurlyWrapper', function () { this.res.sendStatus = sinon.spy()