Explorar el Código

Merge pull request #34870 from overleaf/ii-share-modal-group-sharing

[web] Make group project sharing available to Group Professional plans only

GitOrigin-RevId: ced66529818d573cfd20cef016ed4b3aafdd8d51
ilkin-overleaf hace 1 mes
padre
commit
13bc31cf25

+ 22 - 1
services/web/app/src/Features/Collaborators/CollaboratorsInviteController.mjs

@@ -22,6 +22,7 @@ import PrivilegeLevels, {
 } from '../Authorization/PrivilegeLevels.mjs'
 } from '../Authorization/PrivilegeLevels.mjs'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
 import SplitTestHandler from '../SplitTests/SplitTestHandler.mjs'
 import SubscriptionGroupHandler from '../Subscription/SubscriptionGroupHandler.mjs'
 import SubscriptionGroupHandler from '../Subscription/SubscriptionGroupHandler.mjs'
+import SubscriptionLocator from '../Subscription/SubscriptionLocator.mjs'
 
 
 // This rate limiter allows a different number of requests depending on the
 // This rate limiter allows a different number of requests depending on the
 // number of callaborators a user is allowed. This is implemented by providing
 // number of callaborators a user is allowed. This is implemented by providing
@@ -554,10 +555,30 @@ async function updateSharingLink(req, res) {
   const privileges = body.privileges
   const privileges = body.privileges
   const subscriptionId = body.subscriptionId
   const subscriptionId = body.subscriptionId
 
 
+  const currentUser = SessionManager.getSessionUser(req.session)
+
+  if (subscriptionId) {
+    const subscriptions =
+      await SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions(
+        currentUser._id,
+        { _id: 1 }
+      )
+    const canShareWithSubscription = subscriptions.some(
+      subscription => subscription._id.toString() === subscriptionId.toString()
+    )
+
+    if (!canShareWithSubscription) {
+      logger.debug(
+        { projectId, subscriptionId, userId: currentUser._id },
+        'cannot create a group sharing link for a non-professional or non-member subscription'
+      )
+      return res.status(403).json({ errorReason: 'subscription_not_eligible' })
+    }
+  }
+
   let invite =
   let invite =
     await CollaboratorsInviteGetter.promises.getSharingLinkInvite(projectId)
     await CollaboratorsInviteGetter.promises.getSharingLinkInvite(projectId)
 
 
-  const currentUser = SessionManager.getSessionUser(req.session)
   if (invite === null) {
   if (invite === null) {
     invite = await CollaboratorsInviteHandler.promises.createSharingLinkInvite(
     invite = await CollaboratorsInviteHandler.promises.createSharingLinkInvite(
       projectId,
       projectId,

+ 11 - 8
services/web/app/src/Features/Project/ProjectController.mjs

@@ -574,19 +574,22 @@ const _ProjectController = {
                 )
                 )
               ).isMember)()
               ).isMember)()
           : false,
           : false,
-        activeGroupSubscriptions:
-          SubscriptionLocator.promises.getUserActiveGroupSubscriptions(userId, {
-            _id: 1,
-            teamName: 1,
-            sharingPermissions: 1,
-          }),
+        activeProfessionalGroupSubscriptions:
+          SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions(
+            userId,
+            {
+              _id: 1,
+              teamName: 1,
+              sharingPermissions: 1,
+            }
+          ),
       })
       })
 
 
       const {
       const {
         project,
         project,
         userValues,
         userValues,
         userIsMemberOfGroupSubscription,
         userIsMemberOfGroupSubscription,
-        activeGroupSubscriptions,
+        activeProfessionalGroupSubscriptions,
       } = responses
       } = responses
 
 
       await Promise.all([
       await Promise.all([
@@ -976,7 +979,7 @@ const _ProjectController = {
           ),
           ),
           isMemberOfGroupSubscription: userIsMemberOfGroupSubscription,
           isMemberOfGroupSubscription: userIsMemberOfGroupSubscription,
           hasInstitutionLicence: userHasInstitutionLicence,
           hasInstitutionLicence: userHasInstitutionLicence,
-          activeGroupSubscriptions,
+          activeProfessionalGroupSubscriptions,
         },
         },
         initialLoadingScreenTheme,
         initialLoadingScreenTheme,
         userSettings,
         userSettings,

+ 15 - 3
services/web/app/src/Features/Subscription/SubscriptionLocator.mjs

@@ -7,6 +7,7 @@ import logger from '@overleaf/logger'
 import { AI_ADD_ON_CODE, isStandaloneAiAddOnPlanCode } from './AiHelper.mjs'
 import { AI_ADD_ON_CODE, isStandaloneAiAddOnPlanCode } from './AiHelper.mjs'
 import './GroupPlansData.mjs' // make sure dynamic group plans are loaded
 import './GroupPlansData.mjs' // make sure dynamic group plans are loaded
 import Features from '../../infrastructure/Features.mjs'
 import Features from '../../infrastructure/Features.mjs'
+import { isProfessionalGroupPlan } from './PlansHelper.mjs'
 
 
 const SubscriptionLocator = {
 const SubscriptionLocator = {
   async getUsersSubscription(userOrId) {
   async getUsersSubscription(userOrId) {
@@ -180,11 +181,14 @@ const SubscriptionLocator = {
     }
     }
   },
   },
 
 
-  async getUserActiveGroupSubscriptions(userOrId, projection = {}) {
+  async getUserActiveProfessionalGroupSubscriptions(userOrId, projection = {}) {
     if (!Features.hasFeature('saas')) return []
     if (!Features.hasFeature('saas')) return []
 
 
     const userId = SubscriptionLocator._getUserId(userOrId)
     const userId = SubscriptionLocator._getUserId(userOrId)
-    return await Subscription.find(
+
+    if (!userId) return []
+
+    const activeGroupSubscriptions = await Subscription.find(
       {
       {
         groupPlan: true,
         groupPlan: true,
         $and: [
         $and: [
@@ -197,8 +201,16 @@ const SubscriptionLocator = {
           },
           },
         ],
         ],
       },
       },
-      projection
+      {
+        ...projection,
+        groupPlan: 1,
+        planCode: 1,
+      }
     ).exec()
     ).exec()
+
+    return activeGroupSubscriptions.filter(subscription =>
+      isProfessionalGroupPlan(subscription)
+    )
   },
   },
 
 
   async getUserSubscriptionStatus(userId) {
   async getUserSubscriptionStatus(userId) {

+ 31 - 24
services/web/frontend/js/features/share-project-modal/components/project-access.tsx

@@ -31,6 +31,7 @@ import { ExcludeStrict } from '@ol-types/utils'
 import getMeta from '@/utils/meta'
 import getMeta from '@/utils/meta'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { sendMB } from '@/infrastructure/event-tracking'
 import { sendMB } from '@/infrastructure/event-tracking'
+import { debugConsole } from '@/utils/debugging'
 
 
 type ProjectAccessProps = {
 type ProjectAccessProps = {
   setIsInvitedPeopleScreen: React.Dispatch<React.SetStateAction<boolean>>
   setIsInvitedPeopleScreen: React.Dispatch<React.SetStateAction<boolean>>
@@ -51,7 +52,7 @@ function ProjectAccess({
     null
     null
   )
   )
   const { isProjectOwner } = useEditorContext()
   const { isProjectOwner } = useEditorContext()
-  const { activeGroupSubscriptions } = getMeta('ol-user')
+  const { activeProfessionalGroupSubscriptions } = getMeta('ol-user')
   const groupSharingEnabled = useFeatureFlag('group-link-sharing')
   const groupSharingEnabled = useFeatureFlag('group-link-sharing')
 
 
   const {
   const {
@@ -92,16 +93,18 @@ function ProjectAccess({
       }
       }
 
 
       return data
       return data
-    }).then(data => {
-      setSharingLinkData(data)
-      setProjectAccess(newAccess)
-      setSuccessActionMessage(t('access_updated'))
-      sendMB('sharing-link-set-permissions', {
-        project_id: projectId,
-        access_level: newAccess.split('.')[0],
-        ...reqBody,
-      })
     })
     })
+      .then(data => {
+        setSharingLinkData(data)
+        setProjectAccess(newAccess)
+        setSuccessActionMessage(t('access_updated'))
+        sendMB('sharing-link-set-permissions', {
+          project_id: projectId,
+          access_level: newAccess.split('.')[0],
+          ...reqBody,
+        })
+      })
+      .catch(debugConsole.error)
   }
   }
 
 
   const onAccessSelect = (eventKey: ProjectAccessType) => {
   const onAccessSelect = (eventKey: ProjectAccessType) => {
@@ -125,27 +128,31 @@ function ProjectAccess({
         privileges: eventKey,
         privileges: eventKey,
         subscriptionId: sharingLinkData?.subscriptionId,
         subscriptionId: sharingLinkData?.subscriptionId,
       })
       })
-    ).then(data => {
-      setSharingLinkData(data)
-      setSuccessActionMessage(t('access_updated'))
-      sendMB('sharing-link-set-permissions', {
-        project_id: projectId,
-        access_level: projectAccess?.split('.')[0],
-        privileges: eventKey,
-        subscriptionId: data.subscriptionId,
+    )
+      .then(data => {
+        setSharingLinkData(data)
+        setSuccessActionMessage(t('access_updated'))
+        sendMB('sharing-link-set-permissions', {
+          project_id: projectId,
+          access_level: projectAccess?.split('.')[0],
+          privileges: eventKey,
+          subscriptionId: data.subscriptionId,
+        })
       })
       })
-    })
+      .catch(debugConsole.error)
   }
   }
 
 
   const getGroupLinkText = (id?: string) => {
   const getGroupLinkText = (id?: string) => {
     if (
     if (
       !id ||
       !id ||
-      !activeGroupSubscriptions ||
-      activeGroupSubscriptions.length === 0
+      !activeProfessionalGroupSubscriptions ||
+      activeProfessionalGroupSubscriptions.length === 0
     ) {
     ) {
       return ''
       return ''
     }
     }
-    const subscription = activeGroupSubscriptions.find(sub => sub._id === id)
+    const subscription = activeProfessionalGroupSubscriptions.find(
+      sub => sub._id === id
+    )
     if (subscription?.teamName) {
     if (subscription?.teamName) {
       return t('anyone_in_x_with_the_link', {
       return t('anyone_in_x_with_the_link', {
         groupName: subscription.teamName,
         groupName: subscription.teamName,
@@ -252,8 +259,8 @@ function ProjectAccess({
                   </DropdownItem>
                   </DropdownItem>
                 </DropdownListItem>
                 </DropdownListItem>
                 {groupSharingEnabled &&
                 {groupSharingEnabled &&
-                  activeGroupSubscriptions &&
-                  activeGroupSubscriptions.map(subscription => (
+                  activeProfessionalGroupSubscriptions &&
+                  activeProfessionalGroupSubscriptions.map(subscription => (
                     <DropdownListItem
                     <DropdownListItem
                       className="d-flex align-items-center"
                       className="d-flex align-items-center"
                       key={subscription._id}
                       key={subscription._id}

+ 12 - 15
services/web/frontend/js/features/share-project-modal/components/share-project-modal.tsx

@@ -203,21 +203,18 @@ const ShareProjectModal = React.memo(function ShareProjectModal({
     setSuccessActionMessage(undefined)
     setSuccessActionMessage(undefined)
     setInFlight(true)
     setInFlight(true)
 
 
-    const promise = request()
-
-    promise.catch((error: { data?: Record<string, string> }) => {
-      setError(
-        error.data?.errorReason ||
-          error.data?.error ||
-          'generic_something_went_wrong'
-      )
-    })
-
-    promise.finally(() => {
-      setInFlight(false)
-    })
-
-    return promise
+    return request()
+      .catch((error: { data?: Record<string, string> }) => {
+        setError(
+          error.data?.errorReason ||
+            error.data?.error ||
+            'generic_something_went_wrong'
+        )
+        throw error
+      })
+      .finally(() => {
+        setInFlight(false)
+      })
   }, [])
   }, [])
 
 
   if (!project) {
   if (!project) {

+ 3 - 0
services/web/test/acceptance/src/ProjectInviteTests.mjs

@@ -1086,6 +1086,9 @@ describe('ProjectInviteTests', function () {
       const subscription = new Subscription({
       const subscription = new Subscription({
         adminId: this.sendingUser._id,
         adminId: this.sendingUser._id,
         memberIds: [this.sendingUser._id],
         memberIds: [this.sendingUser._id],
+        groupPlan: true,
+        planCode: 'group_professional',
+        paymentProvider: { state: 'active' },
       })
       })
       let sharingLink
       let sharingLink
       Async.series(
       Async.series(

+ 1 - 1
services/web/test/frontend/features/share-project-modal/components/share-project-modal.test.tsx

@@ -1102,7 +1102,7 @@ describe('<ShareProjectModal/>', function () {
         user: {
         user: {
           id: USER_ID,
           id: USER_ID,
           email: USER_EMAIL,
           email: USER_EMAIL,
-          activeGroupSubscriptions: [{ _id: 'sub-123' }],
+          activeProfessionalGroupSubscriptions: [{ _id: 'sub-123' }],
         },
         },
       })
       })
 
 

+ 1 - 1
services/web/test/frontend/helpers/editor-providers.tsx

@@ -75,7 +75,7 @@ export type EditorProvidersProps = {
     | 'id'
     | 'id'
     | 'email'
     | 'email'
     | 'signUpDate'
     | 'signUpDate'
-    | 'activeGroupSubscriptions'
+    | 'activeProfessionalGroupSubscriptions'
     | 'isProfessionalGroupPlan'
     | 'isProfessionalGroupPlan'
   >
   >
   projectId?: string
   projectId?: string

+ 60 - 0
services/web/test/unit/src/Collaborators/CollaboratorsInviteController.test.mjs

@@ -149,6 +149,12 @@ describe('CollaboratorsInviteController', function () {
       },
       },
     }
     }
 
 
+    ctx.SubscriptionLocator = {
+      promises: {
+        getUserActiveProfessionalGroupSubscriptions: sinon.stub().resolves([]),
+      },
+    }
+
     ctx.SplitTestHandler = {
     ctx.SplitTestHandler = {
       promises: {
       promises: {
         getAssignment: sinon.stub().resolves({ variant: 'default' }),
         getAssignment: sinon.stub().resolves({ variant: 'default' }),
@@ -257,6 +263,13 @@ describe('CollaboratorsInviteController', function () {
       })
       })
     )
     )
 
 
+    vi.doMock(
+      '../../../../app/src/Features/Subscription/SubscriptionLocator.mjs',
+      () => ({
+        default: ctx.SubscriptionLocator,
+      })
+    )
+
     ctx.CollaboratorsInviteController = (await import(MODULE_PATH)).default
     ctx.CollaboratorsInviteController = (await import(MODULE_PATH)).default
 
 
     ctx.res = new MockResponse(vi)
     ctx.res = new MockResponse(vi)
@@ -1916,6 +1929,53 @@ describe('CollaboratorsInviteController', function () {
       expect(ctx.invite.save).to.have.been.calledOnce
       expect(ctx.invite.save).to.have.been.calledOnce
       expect(ctx.res.json).toHaveBeenCalledTimes(1)
       expect(ctx.res.json).toHaveBeenCalledTimes(1)
     })
     })
+
+    describe('with a subscriptionId', function () {
+      beforeEach(function (ctx) {
+        ctx.subscriptionId = new ObjectId()
+        ctx.req.body.subscriptionId = ctx.subscriptionId.toString()
+      })
+
+      it('creates the invite when the user is in a matching professional group subscription', async function (ctx) {
+        await new Promise(resolve => {
+          ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions.resolves(
+            [{ _id: ctx.subscriptionId }]
+          )
+          ctx.CollaboratorsInviteGetter.promises.getSharingLinkInvite.resolves(
+            null
+          )
+          ctx.CollaboratorsInviteHandler.promises.createSharingLinkInvite.resolves(
+            ctx.invite
+          )
+          ctx.res.callback = () => resolve()
+          ctx.CollaboratorsInviteController.updateSharingLink(ctx.req, ctx.res)
+        })
+
+        ctx.CollaboratorsInviteHandler.promises.createSharingLinkInvite.should.have.been.calledWith(
+          ctx.projectId,
+          'readOnly',
+          ctx.subscriptionId.toString()
+        )
+      })
+
+      it('responds with a 403 JSON error when the subscription is not a professional group the user belongs to', async function (ctx) {
+        await new Promise(resolve => {
+          ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions.resolves(
+            []
+          )
+          ctx.res.callback = () => resolve()
+          ctx.CollaboratorsInviteController.updateSharingLink(ctx.req, ctx.res)
+        })
+
+        expect(ctx.res.statusCode).to.equal(403)
+        expect(ctx.res.json).toHaveBeenCalledWith({
+          errorReason: 'subscription_not_eligible',
+        })
+        ctx.CollaboratorsInviteHandler.promises.createSharingLinkInvite.called.should.equal(
+          false
+        )
+      })
+    })
   })
   })
 
 
   describe('validateSharingLink', function () {
   describe('validateSharingLink', function () {

+ 1 - 1
services/web/test/unit/src/Project/ProjectController.test.mjs

@@ -71,7 +71,7 @@ describe('ProjectController', function () {
     ctx.SubscriptionLocator = {
     ctx.SubscriptionLocator = {
       promises: {
       promises: {
         getUsersSubscription: sinon.stub().resolves(),
         getUsersSubscription: sinon.stub().resolves(),
-        getUserActiveGroupSubscriptions: sinon.stub().resolves(),
+        getUserActiveProfessionalGroupSubscriptions: sinon.stub().resolves(),
       },
       },
     }
     }
     ctx.SubscriptionController = {
     ctx.SubscriptionController = {

+ 60 - 0
services/web/test/unit/src/Subscription/SubscriptionLocator.test.mjs

@@ -48,6 +48,12 @@ describe('Subscription Locator Tests', function () {
       SSOConfig: ctx.SSOConfig,
       SSOConfig: ctx.SSOConfig,
     }))
     }))
 
 
+    ctx.isProfessionalGroupPlan = sinon.stub()
+    vi.doMock('../../../../app/src/Features/Subscription/PlansHelper', () => ({
+      isProfessionalGroupPlan: subscription =>
+        ctx.isProfessionalGroupPlan(subscription),
+    }))
+
     ctx.SubscriptionLocator = (await import(modulePath)).default
     ctx.SubscriptionLocator = (await import(modulePath)).default
   })
   })
 
 
@@ -193,4 +199,58 @@ describe('Subscription Locator Tests', function () {
       expect(subscriptionStatus).to.deep.equal({ personal: true, group: true })
       expect(subscriptionStatus).to.deep.equal({ personal: true, group: true })
     })
     })
   })
   })
+
+  describe('getUserActiveProfessionalGroupSubscriptions', function () {
+    beforeEach(function (ctx) {
+      ctx.standardSubscription = {
+        _id: 'standard-sub',
+        groupPlan: true,
+        planCode: 'group_standard',
+      }
+      ctx.professionalSubscription = {
+        _id: 'professional-sub',
+        groupPlan: true,
+        planCode: 'group_professional',
+      }
+      ctx.Subscription.find.returns({
+        exec: sinon
+          .stub()
+          .resolves([ctx.standardSubscription, ctx.professionalSubscription]),
+      })
+      ctx.isProfessionalGroupPlan.callsFake(
+        subscription => subscription === ctx.professionalSubscription
+      )
+    })
+
+    it('returns only the professional group subscriptions', async function (ctx) {
+      const result =
+        await ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions(
+          ctx.user._id
+        )
+      expect(result).to.deep.equal([ctx.professionalSubscription])
+    })
+
+    it('always requests `planCode` and `groupPlan` so professional status can be determined', async function (ctx) {
+      await ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions(
+        ctx.user._id,
+        { _id: 1, teamName: 1 }
+      )
+      const projection = ctx.Subscription.find.lastCall.args[1]
+      expect(projection).to.include({
+        _id: 1,
+        teamName: 1,
+        planCode: 1,
+        groupPlan: 1,
+      })
+    })
+
+    it('returns an empty list without querying when no userId is provided', async function (ctx) {
+      const result =
+        await ctx.SubscriptionLocator.promises.getUserActiveProfessionalGroupSubscriptions(
+          undefined
+        )
+      expect(result).to.deep.equal([])
+      expect(ctx.Subscription.find.called).to.equal(false)
+    })
+  })
 })
 })

+ 1 - 1
services/web/types/user.ts

@@ -65,7 +65,7 @@ export type User = {
   isMemberOfGroupSubscription?: boolean
   isMemberOfGroupSubscription?: boolean
   isProfessionalGroupPlan?: boolean
   isProfessionalGroupPlan?: boolean
   hasInstitutionLicence?: boolean
   hasInstitutionLicence?: boolean
-  activeGroupSubscriptions?: {
+  activeProfessionalGroupSubscriptions?: {
     _id: string
     _id: string
     teamName?: string
     teamName?: string
     sharingPermissions?: SharingPermissions
     sharingPermissions?: SharingPermissions