ソースを参照

Merge pull request #33345 from overleaf/rh-split-group-role

Update group_role in customer.io when changed

GitOrigin-RevId: d21866a9fe324a0468de74a45b6932dda27de8a1
roo hutton 2 ヶ月 前
コミット
4f192564f2

+ 18 - 6
services/web/app/src/Features/Project/ProjectListController.mjs

@@ -129,6 +129,7 @@ async function projectListPage(req, res, next) {
   let usersIndividualSubscription
   /** @type {any[]} */
   let usersGroupSubscriptions = []
+  /** @type {any[]} */
   let usersManagedGroupSubscriptions = []
   let survey
   let userIsMemberOfGroupSubscription = false
@@ -546,12 +547,23 @@ async function projectListPage(req, res, next) {
     user
   )
 
-  const groupRole = userIsMemberOfGroupSubscription
-    ? usersManagedGroupSubscriptions?.length > 0 ||
-      usersGroupSubscriptions.some(sub => sub.userIsGroupManager)
-      ? 'admin'
-      : 'member'
-    : undefined
+  let groupRole
+  if (userIsMemberOfGroupSubscription) {
+    const userIdStr = userId.toString()
+    const isGroupAdmin = usersManagedGroupSubscriptions?.some(
+      sub => sub.admin_id?._id?.toString() === userIdStr
+    )
+    const isGroupManager =
+      usersManagedGroupSubscriptions?.length > 0 ||
+      usersGroupSubscriptions?.some(sub => sub.userIsGroupManager)
+    if (isGroupAdmin) {
+      groupRole = 'admin'
+    } else if (isGroupManager) {
+      groupRole = 'manager'
+    } else {
+      groupRole = 'member'
+    }
+  }
 
   Modules.promises.hooks
     .fire('setUserProperties', userId, {

+ 36 - 0
services/web/app/src/Features/Subscription/CustomerIoPlanHelpers.mjs

@@ -512,6 +512,34 @@ function shouldUseCommonsBestSubscription(
   )
 }
 
+/**
+ * Determine the user's role in any group subscription they participate in.
+ *
+ * @param {MongoSubscription[]} memberGroupSubscriptions
+ * @param {MongoSubscription[]} managedGroupSubscriptions
+ * @param {string|object} userId
+ * @returns {''|'admin'|'manager'|'member'}
+ */
+function getGroupRole(
+  memberGroupSubscriptions = [],
+  managedGroupSubscriptions = [],
+  userId
+) {
+  if (
+    managedGroupSubscriptions.length === 0 &&
+    memberGroupSubscriptions.length === 0
+  ) {
+    return ''
+  }
+  const userIdStr = userId.toString()
+  const isAdmin = managedGroupSubscriptions.some(
+    sub => sub.admin_id?._id?.toString() === userIdStr
+  )
+  if (isAdmin) return 'admin'
+  if (managedGroupSubscriptions.length > 0) return 'manager'
+  return 'member'
+}
+
 /**
  * Compute plan-related user properties for sending to customer.io.
  *
@@ -525,6 +553,7 @@ function shouldUseCommonsBestSubscription(
  * @param {boolean} options.hasCommons
  * @param {Nullable<{ isPremium?: boolean }>} [options.writefullData]
  * @param {Map<string, boolean>} [options.aiBlockedByPolicyId]
+ * @param {string|object} options.userId
  */
 function getPlanProperties({
   bestSubscription,
@@ -536,6 +565,7 @@ function getPlanProperties({
   hasCommons,
   writefullData,
   aiBlockedByPolicyId,
+  userId,
 }) {
   const planType = normalizePlanType(bestSubscription)
   const displayPlanType = getFriendlyPlanName(planType)
@@ -581,6 +611,11 @@ function getPlanProperties({
   const properties = {
     ai_plan: aiPlan,
     group: userIsMemberOfGroupSubscription,
+    group_role: getGroupRole(
+      memberGroupSubscriptions,
+      managedGroupSubscriptions,
+      userId
+    ),
     commons: Boolean(hasCommons),
     individual_subscription: Boolean(
       individualSubscription && !individualSubscription.groupPlan
@@ -620,5 +655,6 @@ export default {
   getAiPlanCadence,
   hasPlanAiEnabled,
   shouldUseCommonsBestSubscription,
+  getGroupRole,
   getPlanProperties,
 }

+ 1 - 0
services/web/app/src/Features/Subscription/FeaturesUpdater.mjs

@@ -185,6 +185,7 @@ async function _updateCustomerIoSubscriptionProperties(user, features) {
     hasCommons,
     writefullData,
     aiBlockedByPolicyId,
+    userId,
   })
 
   await Modules.promises.hooks.fire('setUserProperties', userId, {

+ 40 - 0
services/web/app/src/Features/Subscription/SubscriptionUpdater.mjs

@@ -15,6 +15,7 @@ import AccountMappingHelper from '../Analytics/AccountMappingHelper.mjs'
 import { SSOConfig } from '../../models/SSOConfig.mjs'
 import mongoose from '../../infrastructure/Mongoose.mjs'
 import Modules from '../../infrastructure/Modules.mjs'
+import CustomerIoPlanHelpers from './CustomerIoPlanHelpers.mjs'
 
 /**
  * @typedef {import('../../../../types/subscription/dashboard/subscription').Subscription} Subscription
@@ -73,6 +74,13 @@ async function updateAdmin(subscription, adminId) {
     update.$set.manager_ids = [new ObjectId(adminId)]
   }
   await Subscription.updateOne(query, update).exec()
+  if (subscription.groupPlan) {
+    const previousAdminId = subscription.admin_id?.toString()
+    if (previousAdminId && previousAdminId !== adminId.toString()) {
+      await sendGroupRoleUserProperty(previousAdminId)
+    }
+    await sendGroupRoleUserProperty(adminId)
+  }
 }
 
 async function syncSubscription(
@@ -445,6 +453,30 @@ async function _sendUserGroupPlanCodeUserProperty(userId) {
   }
 }
 
+async function sendGroupRoleUserProperty(userId) {
+  try {
+    const [memberSubscriptions, managedSubscriptions] = await Promise.all([
+      SubscriptionLocator.promises.getMemberSubscriptions(userId),
+      SubscriptionLocator.promises.getManagedGroupSubscriptions(userId),
+    ])
+
+    const groupRole = CustomerIoPlanHelpers.getGroupRole(
+      memberSubscriptions,
+      managedSubscriptions,
+      userId
+    )
+
+    await Modules.promises.hooks.fire('setUserProperties', userId, {
+      group_role: groupRole,
+    })
+  } catch (error) {
+    logger.error(
+      { err: error, userId },
+      'Failed to update group_role user property in customer.io'
+    )
+  }
+}
+
 async function handleExpiredSubscription(subscription, requesterData) {
   const hasManagedUsersFeature =
     Features.hasFeature('saas') && subscription?.managedUsersEnabled
@@ -547,6 +579,13 @@ async function transferSubscriptionOwnership(
     update.$set.previousPaymentProvider = subscription.paymentProvider
   }
   await Subscription.updateOne(query, update).exec()
+  if (subscription.groupPlan) {
+    const previousAdminId = subscription.admin_id?.toString()
+    if (previousAdminId && previousAdminId !== adminId.toString()) {
+      await sendGroupRoleUserProperty(previousAdminId)
+    }
+    await sendGroupRoleUserProperty(adminId)
+  }
 }
 
 export default {
@@ -579,5 +618,6 @@ export default {
     scheduleRefreshFeatures,
     handleExpiredSubscription,
     transferSubscriptionOwnership,
+    sendGroupRoleUserProperty,
   },
 }

+ 19 - 1
services/web/app/src/Features/UserMembership/UserMembershipHandler.mjs

@@ -8,6 +8,7 @@ import UserGetter from '../User/UserGetter.mjs'
 import UserMembershipErrors from './UserMembershipErrors.mjs'
 import Modules from '../../infrastructure/Modules.mjs'
 import mongoose from '../../infrastructure/Mongoose.mjs'
+import SubscriptionUpdater from '../Subscription/SubscriptionUpdater.mjs'
 
 const { ObjectId } = mongodb
 
@@ -71,6 +72,14 @@ const UserMembershipHandler = {
     }
 
     await addUserToEntity(entity, attribute, user)
+
+    if (
+      entityConfig.modelName === 'Subscription' &&
+      attribute === 'manager_ids'
+    ) {
+      await SubscriptionUpdater.promises.sendGroupRoleUserProperty(user._id)
+    }
+
     return UserMembershipViewModel.build(user)
   },
 
@@ -98,7 +107,16 @@ const UserMembershipHandler = {
       await Modules.promises.hooks.fire('addGroupAuditLogEntry', auditLog)
     }
 
-    return await removeUserFromEntity(entity, attribute, userId)
+    const result = await removeUserFromEntity(entity, attribute, userId)
+
+    if (
+      entityConfig.modelName === 'Subscription' &&
+      attribute === 'manager_ids'
+    ) {
+      await SubscriptionUpdater.promises.sendGroupRoleUserProperty(userId)
+    }
+
+    return result
   },
 }
 

+ 14 - 7
services/web/scripts/export_active_subscription_users_csv.mjs

@@ -211,17 +211,24 @@ function getGroupRole(candidates, userId) {
     return ''
   }
 
-  const isGroupAdminOrManager = groupCandidates.some(candidate => {
-    const subscription = candidate.subscription
-    const adminId = subscription?.admin_id?.toString()
-    const managerIds = (subscription?.manager_ids || []).map(id =>
+  const isGroupAdmin = groupCandidates.some(
+    candidate => candidate.subscription?.admin_id?.toString() === userId
+  )
+  if (isGroupAdmin) {
+    return 'admin'
+  }
+
+  const isGroupManager = groupCandidates.some(candidate => {
+    const managerIds = (candidate.subscription?.manager_ids || []).map(id =>
       id?.toString()
     )
-
-    return adminId === userId || managerIds.includes(userId)
+    return managerIds.includes(userId)
   })
+  if (isGroupManager) {
+    return 'manager'
+  }
 
-  return isGroupAdminOrManager ? 'admin' : 'member'
+  return 'member'
 }
 
 function chooseBestCandidate(candidates) {

+ 58 - 0
services/web/test/unit/src/Project/ProjectListController.test.mjs

@@ -478,6 +478,7 @@ describe('ProjectListController', function () {
             {
               planCode: 'group_professional',
               membersLimit: 12,
+              admin_id: { _id: ctx.user._id },
             },
           ],
         }
@@ -495,6 +496,63 @@ describe('ProjectListController', function () {
       )
     })
 
+    it('should send groupRole to customer.io for group managers', async function (ctx) {
+      ctx.Features.hasFeature.withArgs('saas').returns(true)
+      ctx.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves(
+        {
+          bestSubscription: { type: 'free' },
+          individualSubscription: null,
+          memberGroupSubscriptions: [],
+          managedGroupSubscriptions: [
+            {
+              planCode: 'group_professional',
+              membersLimit: 12,
+              admin_id: { _id: new ObjectId() },
+            },
+          ],
+        }
+      )
+      ctx.res.render = () => {}
+
+      await ctx.ProjectListController.projectListPage(ctx.req, ctx.res)
+
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.user._id,
+        sinon.match({
+          group_role: 'manager',
+        })
+      )
+    })
+
+    it('should send groupRole to customer.io for group members', async function (ctx) {
+      ctx.Features.hasFeature.withArgs('saas').returns(true)
+      ctx.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves(
+        {
+          bestSubscription: { type: 'free' },
+          individualSubscription: null,
+          memberGroupSubscriptions: [
+            {
+              planCode: 'group_professional',
+              userIsGroupManager: false,
+            },
+          ],
+          managedGroupSubscriptions: [],
+        }
+      )
+      ctx.res.render = () => {}
+
+      await ctx.ProjectListController.projectListPage(ctx.req, ctx.res)
+
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.user._id,
+        sinon.match({
+          group_role: 'member',
+        })
+      )
+    })
+
     it('should show INR Banner for Indian users with free account', async function (ctx) {
       // usersBestSubscription is only available when saas feature is present
       ctx.Features.hasFeature.withArgs('saas').returns(true)

+ 70 - 0
services/web/test/unit/src/Subscription/FeaturesUpdater.test.mjs

@@ -839,6 +839,76 @@ describe('FeaturesUpdater', function () {
       })
     })
 
+    describe('group_role property', function () {
+      it("should set group_role to '' when the user is not in any group", async function (ctx) {
+        ctx.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves(
+          {
+            bestSubscription: { type: 'free' },
+            individualSubscription: null,
+            memberGroupSubscriptions: [],
+            managedGroupSubscriptions: [],
+          }
+        )
+        await ctx.FeaturesUpdater.promises.refreshFeatures(ctx.user._id, 'test')
+        expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+          'setUserProperties',
+          ctx.user._id,
+          sinon.match({ group_role: '' })
+        )
+      })
+
+      it("should set group_role to 'member' when the user only belongs to a group as a member", async function (ctx) {
+        ctx.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves(
+          {
+            bestSubscription: { type: 'free' },
+            individualSubscription: null,
+            memberGroupSubscriptions: [{ _id: new ObjectId() }],
+            managedGroupSubscriptions: [],
+          }
+        )
+        await ctx.FeaturesUpdater.promises.refreshFeatures(ctx.user._id, 'test')
+        expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+          'setUserProperties',
+          ctx.user._id,
+          sinon.match({ group_role: 'member' })
+        )
+      })
+
+      it("should set group_role to 'manager' when the user manages a group they don't own", async function (ctx) {
+        ctx.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves(
+          {
+            bestSubscription: { type: 'free' },
+            individualSubscription: null,
+            memberGroupSubscriptions: [],
+            managedGroupSubscriptions: [{ admin_id: { _id: new ObjectId() } }],
+          }
+        )
+        await ctx.FeaturesUpdater.promises.refreshFeatures(ctx.user._id, 'test')
+        expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+          'setUserProperties',
+          ctx.user._id,
+          sinon.match({ group_role: 'manager' })
+        )
+      })
+
+      it("should set group_role to 'admin' when the user owns a group", async function (ctx) {
+        ctx.SubscriptionViewModelBuilder.promises.getUsersSubscriptionDetails.resolves(
+          {
+            bestSubscription: { type: 'free' },
+            individualSubscription: null,
+            memberGroupSubscriptions: [],
+            managedGroupSubscriptions: [{ admin_id: { _id: ctx.user._id } }],
+          }
+        )
+        await ctx.FeaturesUpdater.promises.refreshFeatures(ctx.user._id, 'test')
+        expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+          'setUserProperties',
+          ctx.user._id,
+          sinon.match({ group_role: 'admin' })
+        )
+      })
+    })
+
     describe('with a non-standard feature set', async function () {
       beforeEach(async function (ctx) {
         ctx.SubscriptionLocator.promises.getGroupSubscriptionsMemberOf

+ 79 - 0
services/web/test/unit/src/Subscription/SubscriptionUpdater.test.mjs

@@ -91,6 +91,7 @@ describe('SubscriptionUpdater', function () {
         getUsersSubscription: sinon.stub(),
         getGroupSubscriptionMemberOf: sinon.stub(),
         getMemberSubscriptions: sinon.stub().resolves([]),
+        getManagedGroupSubscriptions: sinon.stub().resolves([]),
         getSubscription: sinon.stub(),
       },
     }
@@ -283,6 +284,44 @@ describe('SubscriptionUpdater', function () {
       ctx.SubscriptionModel.updateOne.should.have.been.calledWith(query, update)
     })
 
+    it("should fire setUserProperties for both old and new admin's group_role on group subscriptions", async function (ctx) {
+      ctx.subscription.groupPlan = true
+      ctx.SubscriptionLocator.promises.getManagedGroupSubscriptions
+        .withArgs(ctx.otherUserId)
+        .resolves([{ admin_id: { _id: ctx.otherUserId } }])
+      ctx.SubscriptionLocator.promises.getManagedGroupSubscriptions
+        .withArgs(ctx.adminUser._id)
+        .resolves([{ admin_id: { _id: ctx.otherUserId } }])
+
+      await ctx.SubscriptionUpdater.promises.updateAdmin(
+        ctx.subscription,
+        ctx.otherUserId
+      )
+
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.adminUser._id,
+        { group_role: 'manager' }
+      )
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.otherUserId,
+        { group_role: 'admin' }
+      )
+    })
+
+    it('should not fire setUserProperties for group_role on non-group subscriptions', async function (ctx) {
+      await ctx.SubscriptionUpdater.promises.updateAdmin(
+        ctx.subscription,
+        ctx.otherUserId
+      )
+      expect(ctx.Modules.promises.hooks.fire).to.not.have.been.calledWith(
+        'setUserProperties',
+        sinon.match.any,
+        sinon.match.has('group_role')
+      )
+    })
+
     it('should remove the manager for non-group subscriptions', async function (ctx) {
       await ctx.SubscriptionUpdater.promises.updateAdmin(
         ctx.subscription,
@@ -353,6 +392,46 @@ describe('SubscriptionUpdater', function () {
       ctx.SubscriptionModel.updateOne.should.have.been.calledWith(query, update)
     })
 
+    it("should fire setUserProperties for both old and new admin's group_role on group subscriptions", async function (ctx) {
+      ctx.subscription.groupPlan = true
+      ctx.SubscriptionLocator.promises.getManagedGroupSubscriptions
+        .withArgs(ctx.otherUserId)
+        .resolves([{ admin_id: { _id: ctx.otherUserId } }])
+      ctx.SubscriptionLocator.promises.getManagedGroupSubscriptions
+        .withArgs(ctx.adminUser._id)
+        .resolves([{ admin_id: { _id: ctx.otherUserId } }])
+
+      await ctx.SubscriptionUpdater.promises.transferSubscriptionOwnership(
+        ctx.subscription,
+        ctx.otherUserId,
+        false
+      )
+
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.adminUser._id,
+        { group_role: 'manager' }
+      )
+      expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith(
+        'setUserProperties',
+        ctx.otherUserId,
+        { group_role: 'admin' }
+      )
+    })
+
+    it('should not fire setUserProperties for group_role on non-group subscriptions', async function (ctx) {
+      await ctx.SubscriptionUpdater.promises.transferSubscriptionOwnership(
+        ctx.subscription,
+        ctx.otherUserId,
+        false
+      )
+      expect(ctx.Modules.promises.hooks.fire).to.not.have.been.calledWith(
+        'setUserProperties',
+        sinon.match.any,
+        sinon.match.has('group_role')
+      )
+    })
+
     it('should clear previousPaymentProvider when clearPreviousPaymentProvider is true', async function (ctx) {
       ctx.subscription.paymentProvider = {
         id: 'stripe-123',

+ 13 - 0
services/web/test/unit/src/UserMembership/UserMembershipHandler.test.mjs

@@ -101,6 +101,12 @@ describe('UserMembershipHandler', function () {
       }),
     }
 
+    ctx.SubscriptionUpdater = {
+      promises: {
+        sendGroupRoleUserProperty: vi.fn().mockResolvedValue(),
+      },
+    }
+
     vi.doMock('mongodb-legacy', () => ({
       default: { ObjectId },
     }))
@@ -136,6 +142,13 @@ describe('UserMembershipHandler', function () {
       default: ctx.mongoose,
     }))
 
+    vi.doMock(
+      '../../../../app/src/Features/Subscription/SubscriptionUpdater',
+      () => ({
+        default: ctx.SubscriptionUpdater,
+      })
+    )
+
     ctx.UserMembershipHandler = (await import(modulePath)).default
   })