From cb7d75202b8f753553a6a28fa028e3dcadcd8c70 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 16 May 2025 09:00:58 +0100 Subject: [PATCH] [web] fetch users subscriptions once from project dashboard (#25652) * [web] fetch users subscriptions once from project dashboard * [web] fix types GitOrigin-RevId: 18de18f8d4237d97087ef92eaa5052f253a92813 --- .../Project/ProjectListController.mjs | 31 ++++++------ .../SubscriptionViewModelBuilder.js | 13 ++++- .../Project/ProjectListControllerTests.mjs | 48 +++++++++---------- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectListController.mjs b/services/web/app/src/Features/Project/ProjectListController.mjs index b3e5e81890..c62396e153 100644 --- a/services/web/app/src/Features/Project/ProjectListController.mjs +++ b/services/web/app/src/Features/Project/ProjectListController.mjs @@ -21,12 +21,10 @@ import { OError, V1ConnectionError } from '../Errors/Errors.js' import { User } from '../../models/User.js' import UserPrimaryEmailCheckHandler from '../User/UserPrimaryEmailCheckHandler.js' import UserController from '../User/UserController.js' -import LimitationsManager from '../Subscription/LimitationsManager.js' import NotificationsBuilder from '../Notifications/NotificationsBuilder.js' import GeoIpLookup from '../../infrastructure/GeoIpLookup.js' import SplitTestHandler from '../SplitTests/SplitTestHandler.js' import SplitTestSessionHandler from '../SplitTests/SplitTestSessionHandler.js' -import SubscriptionLocator from '../Subscription/SubscriptionLocator.js' import TutorialHandler from '../Tutorial/TutorialHandler.js' /** @@ -102,6 +100,8 @@ async function projectListPage(req, res, next) { // - undefined - when there's no "saas" feature or couldn't get subscription data // - object - the subscription data object let usersBestSubscription + let usersIndividualSubscription + let usersGroupSubscriptions = [] let survey let userIsMemberOfGroupSubscription = false let groupSubscriptionsPendingEnrollment = [] @@ -132,10 +132,13 @@ async function projectListPage(req, res, next) { await SplitTestSessionHandler.promises.sessionMaintenance(req, user) try { - usersBestSubscription = - await SubscriptionViewModelBuilder.promises.getBestSubscription({ - _id: userId, - }) + ;({ + bestSubscription: usersBestSubscription, + individualSubscription: usersIndividualSubscription, + memberGroupSubscriptions: usersGroupSubscriptions, + } = await SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails( + { _id: userId } + )) } catch (error) { logger.err( { err: error, userId }, @@ -143,14 +146,11 @@ async function projectListPage(req, res, next) { ) } try { - const { isMember, subscriptions } = - await LimitationsManager.promises.userIsMemberOfGroupSubscription(user) - - userIsMemberOfGroupSubscription = isMember + userIsMemberOfGroupSubscription = usersGroupSubscriptions?.length > 0 // TODO use helper function if (!user.enrollment?.managedBy) { - groupSubscriptionsPendingEnrollment = subscriptions.filter( + groupSubscriptionsPendingEnrollment = usersGroupSubscriptions.filter( subscription => subscription.groupPlan && subscription.managedUsersEnabled ) @@ -391,13 +391,10 @@ async function projectListPage(req, res, next) { let hasIndividualRecurlySubscription = false try { - const individualSubscription = - await SubscriptionLocator.promises.getUsersSubscription(userId) - hasIndividualRecurlySubscription = - individualSubscription?.groupPlan === false && - individualSubscription?.recurlyStatus?.state !== 'canceled' && - individualSubscription?.recurlySubscription_id !== '' + usersIndividualSubscription?.groupPlan === false && + usersIndividualSubscription?.recurlyStatus?.state !== 'canceled' && + usersIndividualSubscription?.recurlySubscription_id !== '' } catch (error) { logger.error({ err: error }, 'Failed to get individual subscription') } diff --git a/services/web/app/src/Features/Subscription/SubscriptionViewModelBuilder.js b/services/web/app/src/Features/Subscription/SubscriptionViewModelBuilder.js index 180a78f294..441d9c2c9b 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionViewModelBuilder.js +++ b/services/web/app/src/Features/Subscription/SubscriptionViewModelBuilder.js @@ -24,6 +24,7 @@ const Modules = require('../../infrastructure/Modules') /** * @import { Subscription } from "../../../../types/project/dashboard/subscription" + * @import { Subscription as DBSubscription } from "../../models/Subscription" */ function buildHostedLink(type) { @@ -378,6 +379,15 @@ async function buildUsersSubscriptionViewModel(user, locale = 'en') { * @returns {Promise} */ async function getBestSubscription(user) { + const { bestSubscription } = await getUsersSubscriptionDetails(user) + return bestSubscription +} + +/** + * @param {{_id: string}} user + * @returns {Promise<{bestSubscription:Subscription,individualSubscription:DBSubscription|null,memberGroupSubscriptions:DBSubscription[]}>} + */ +async function getUsersSubscriptionDetails(user) { let [ individualSubscription, memberGroupSubscriptions, @@ -464,7 +474,7 @@ async function getBestSubscription(user) { } } } - return bestSubscription + return { bestSubscription, individualSubscription, memberGroupSubscriptions } } function buildPlansList(currentPlan) { @@ -599,5 +609,6 @@ module.exports = { promises: { buildUsersSubscriptionViewModel, getBestSubscription, + getUsersSubscriptionDetails, }, } diff --git a/services/web/test/unit/src/Project/ProjectListControllerTests.mjs b/services/web/test/unit/src/Project/ProjectListControllerTests.mjs index 40c6dfaa54..827d16b737 100644 --- a/services/web/test/unit/src/Project/ProjectListControllerTests.mjs +++ b/services/web/test/unit/src/Project/ProjectListControllerTests.mjs @@ -53,11 +53,6 @@ describe('ProjectListController', function () { this.settings = { siteUrl: 'https://overleaf.com', } - this.LimitationsManager = { - promises: { - userIsMemberOfGroupSubscription: sinon.stub().resolves(false), - }, - } this.TagsHandler = { promises: { getAllTags: sinon.stub().resolves(this.tags), @@ -113,7 +108,11 @@ describe('ProjectListController', function () { } this.SubscriptionViewModelBuilder = { promises: { - getBestSubscription: sinon.stub().resolves({ type: 'free' }), + getUsersSubscriptionDetails: sinon.stub().resolves({ + bestSubscription: { type: 'free' }, + individualSubscription: null, + memberGroupSubscriptions: [], + }), }, } this.SurveyHandler = { @@ -126,11 +125,6 @@ describe('ProjectListController', function () { ipMatcherAffiliation: sinon.stub().returns({ create: sinon.stub() }), }, } - this.SubscriptionLocator = { - promises: { - getUserSubscription: sinon.stub().resolves({}), - }, - } this.GeoIpLookup = { promises: { getCurrencyCode: sinon.stub().resolves({ @@ -161,8 +155,6 @@ describe('ProjectListController', function () { this.SplitTestSessionHandler, '../../../../app/src/Features/User/UserController': this.UserController, '../../../../app/src/Features/Project/ProjectHelper': this.ProjectHelper, - '../../../../app/src/Features/Subscription/LimitationsManager': - this.LimitationsManager, '../../../../app/src/Features/Tags/TagsHandler': this.TagsHandler, '../../../../app/src/Features/Notifications/NotificationsHandler': this.NotificationsHandler, @@ -180,8 +172,6 @@ describe('ProjectListController', function () { this.UserPrimaryEmailCheckHandler, '../../../../app/src/Features/Notifications/NotificationsBuilder': this.NotificationBuilder, - '../../../../app/src/Features/Subscription/SubscriptionLocator': - this.SubscriptionLocator, '../../../../app/src/infrastructure/GeoIpLookup': this.GeoIpLookup, '../../../../app/src/Features/Tutorial/TutorialHandler': this.TutorialHandler, @@ -345,9 +335,13 @@ describe('ProjectListController', function () { it('should show INR Banner for Indian users with free account', function (done) { // usersBestSubscription is only available when saas feature is present this.Features.hasFeature.withArgs('saas').returns(true) - this.SubscriptionViewModelBuilder.promises.getBestSubscription.resolves({ - type: 'free', - }) + this.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves( + { + bestSubscription: { + type: 'free', + }, + } + ) this.GeoIpLookup.promises.getCurrencyCode.resolves({ countryCode: 'IN', }) @@ -361,9 +355,13 @@ describe('ProjectListController', function () { it('should not show INR Banner for Indian users with premium account', function (done) { // usersBestSubscription is only available when saas feature is present this.Features.hasFeature.withArgs('saas').returns(true) - this.SubscriptionViewModelBuilder.promises.getBestSubscription.resolves({ - type: 'individual', - }) + this.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves( + { + bestSubscription: { + type: 'individual', + }, + } + ) this.GeoIpLookup.promises.getCurrencyCode.resolves({ countryCode: 'IN', }) @@ -616,8 +614,8 @@ describe('ProjectListController', function () { describe('enterprise banner', function () { beforeEach(function (done) { this.Features.hasFeature.withArgs('saas').returns(true) - this.LimitationsManager.promises.userIsMemberOfGroupSubscription.resolves( - { isMember: false } + this.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves( + { memberGroupSubscriptions: [] } ) this.UserGetter.promises.getUserFullEmails.resolves([ { @@ -660,8 +658,8 @@ describe('ProjectListController', function () { }) it('does not show banner if user is part of any group subscription', function () { - this.LimitationsManager.promises.userIsMemberOfGroupSubscription.resolves( - { isMember: true } + this.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves( + { memberGroupSubscriptions: [{}] } ) this.res.render = (pageName, opts) => {