diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js index 5d729437a2..e37e18f443 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsGetter.js @@ -40,6 +40,7 @@ module.exports = { getMemberIdPrivilegeLevel, getInvitedCollaboratorCount, getInvitedEditCollaboratorCount, + getInvitedPendingEditorCount, getProjectsUserIsMemberOf, dangerouslyGetAllProjectsUserIsMemberOf, isUserInvitedMemberOfProject, @@ -59,6 +60,7 @@ async function getMemberIdsWithPrivilegeLevels(projectId) { tokenAccessReadOnly_refs: 1, tokenAccessReadAndWrite_refs: 1, publicAccesLevel: 1, + pendingEditor_refs: 1, }) if (!project) { throw new Errors.NotFoundError(`no project found with id ${projectId}`) @@ -69,7 +71,8 @@ async function getMemberIdsWithPrivilegeLevels(projectId) { project.readOnly_refs, project.tokenAccessReadAndWrite_refs, project.tokenAccessReadOnly_refs, - project.publicAccesLevel + project.publicAccesLevel, + project.pendingEditor_refs ) return memberIds } @@ -136,6 +139,17 @@ async function getInvitedEditCollaboratorCount(projectId) { ).length } +async function getInvitedPendingEditorCount(projectId) { + // Only counts invited members that are readonly pending editors + const members = await getMemberIdsWithPrivilegeLevels(projectId) + return members.filter( + m => + m.source === Sources.INVITE && + m.privilegeLevel === PrivilegeLevels.READ_ONLY && + m.pendingEditor === true + ).length +} + async function isUserInvitedMemberOfProject(userId, projectId) { if (!userId) { return false @@ -309,7 +323,8 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( readOnlyIds, tokenAccessIds, tokenAccessReadOnlyIds, - publicAccessLevel + publicAccessLevel, + pendingEditorIds ) { const members = [] members.push({ @@ -329,6 +344,9 @@ function _getMemberIdsWithPrivilegeLevelsFromFields( id: memberId.toString(), privilegeLevel: PrivilegeLevels.READ_ONLY, source: Sources.INVITE, + ...(pendingEditorIds?.some(pe => memberId.equals(pe)) && { + pendingEditor: true, + }), }) } if (publicAccessLevel === PublicAccessLevels.TOKEN_BASED) { @@ -364,7 +382,11 @@ async function _loadMembers(members) { signUpDate: 1, }) if (user != null) { - return { user, privilegeLevel: member.privilegeLevel } + return { + user, + privilegeLevel: member.privilegeLevel, + ...(member.pendingEditor && { pendingEditor: true }), + } } else { return null } diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js index 87e3755126..b23f981414 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js @@ -49,6 +49,7 @@ async function removeUserFromProject(projectId, userId) { $pull: { collaberator_refs: userId, readOnly_refs: userId, + pendingEditor_refs: userId, tokenAccessReadOnly_refs: userId, tokenAccessReadAndWrite_refs: userId, trashed: userId, @@ -62,6 +63,7 @@ async function removeUserFromProject(projectId, userId) { $pull: { collaberator_refs: userId, readOnly_refs: userId, + pendingEditor_refs: userId, tokenAccessReadOnly_refs: userId, tokenAccessReadAndWrite_refs: userId, archived: userId, @@ -196,6 +198,19 @@ async function transferProjects(fromUserId, toUserId) { } ).exec() + await Project.updateMany( + { pendingEditor_refs: fromUserId }, + { + $addToSet: { pendingEditor_refs: toUserId }, + } + ).exec() + await Project.updateMany( + { pendingEditor_refs: fromUserId }, + { + $pull: { pendingEditor_refs: fromUserId }, + } + ).exec() + // Flush in background, no need to block on this _flushProjects(projectIds).catch(err => { logger.err( @@ -220,14 +235,14 @@ async function setCollaboratorPrivilegeLevel( switch (privilegeLevel) { case PrivilegeLevels.READ_AND_WRITE: { update = { - $pull: { readOnly_refs: userId }, + $pull: { readOnly_refs: userId, pendingEditor_refs: userId }, $addToSet: { collaberator_refs: userId }, } break } case PrivilegeLevels.READ_ONLY: { update = { - $pull: { collaberator_refs: userId }, + $pull: { collaberator_refs: userId, pendingEditor_refs: userId }, $addToSet: { readOnly_refs: userId }, } break diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index 89353a502f..a61d530338 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -415,6 +415,7 @@ const _ProjectController = { tokens: 1, tokenAccessReadAndWrite_refs: 1, // used for link sharing analytics collaberator_refs: 1, // used for link sharing analytics + pendingEditor_refs: 1, // used for link sharing analytics }), userIsMemberOfGroupSubscription: sessionUser ? (async () => @@ -488,12 +489,24 @@ const _ProjectController = { anonRequestToken ) - const linkSharingChanges = - await SplitTestHandler.promises.getAssignmentForUser( + const [linkSharingChanges, linkSharingEnforcement] = await Promise.all([ + SplitTestHandler.promises.getAssignmentForUser( project.owner_ref, 'link-sharing-warning' - ) + ), + SplitTestHandler.promises.getAssignmentForUser( + project.owner_ref, + 'link-sharing-enforcement' + ), + ]) + if (linkSharingChanges?.variant === 'active') { + if (linkSharingEnforcement?.variant === 'active') { + await Modules.promises.hooks.fire( + 'enforceCollaboratorLimit', + projectId + ) + } if (isTokenMember) { // Check explicitly that the user is in read write token refs, while this could be inferred // from the privilege level, the privilege level of token members might later be restricted @@ -570,12 +583,14 @@ const _ProjectController = { ) const planLimit = ownerFeatures?.collaborators || 0 const namedEditors = project.collaberator_refs?.length || 0 + const pendingEditors = project.pendingEditor_refs?.length || 0 const exceedAtLimit = planLimit > -1 && namedEditors >= planLimit const projectOpenedSegmentation = { projectId: project._id, // temporary link sharing segmentation: linkSharingWarning: linkSharingChanges?.variant, namedEditors, + pendingEditors, tokenEditors: project.tokenAccessReadAndWrite_refs?.length || 0, planLimit, exceedAtLimit, @@ -1034,7 +1049,6 @@ const ProjectController = { ), updateProjectSettings: expressify(_ProjectController.updateProjectSettings), userProjectsJson: expressify(_ProjectController.userProjectsJson), - _buildProjectList: _ProjectController._buildProjectList, _buildProjectViewModel: _ProjectController._buildProjectViewModel, _injectProjectUsers: _ProjectController._injectProjectUsers, diff --git a/services/web/app/src/Features/Project/ProjectEditorHandler.js b/services/web/app/src/Features/Project/ProjectEditorHandler.js index 7d01708833..17c251357b 100644 --- a/services/web/app/src/Features/Project/ProjectEditorHandler.js +++ b/services/web/app/src/Features/Project/ProjectEditorHandler.js @@ -83,11 +83,9 @@ module.exports = ProjectEditorHandler = { for (const member of members || []) { if (member.privilegeLevel === 'owner') { ownerFeatures = member.user.features - owner = this.buildUserModelView(member.user, 'owner') + owner = this.buildUserModelView(member) } else { - filteredMembers.push( - this.buildUserModelView(member.user, member.privilegeLevel) - ) + filteredMembers.push(this.buildUserModelView(member)) } } return { @@ -97,14 +95,16 @@ module.exports = ProjectEditorHandler = { } }, - buildUserModelView(user, privileges) { + buildUserModelView(member) { + const user = member.user return { _id: user._id, first_name: user.first_name, last_name: user.last_name, email: user.email, - privileges, + privileges: member.privilegeLevel, signUpDate: user.signUpDate, + pendingEditor: member.pendingEditor, } }, diff --git a/services/web/app/src/Features/Subscription/LimitationsManager.js b/services/web/app/src/Features/Subscription/LimitationsManager.js index bd50f0f002..cc7e1a5a57 100644 --- a/services/web/app/src/Features/Subscription/LimitationsManager.js +++ b/services/web/app/src/Features/Subscription/LimitationsManager.js @@ -30,14 +30,14 @@ async function allowedNumberOfCollaboratorsForUser(userId) { async function canAddXCollaborators(projectId, numberOfNewCollaborators) { const allowedNumber = await allowedNumberOfCollaboratorsInProject(projectId) + if (allowedNumber < 0) { + return true // -1 means unlimited + } const currentNumber = await CollaboratorsGetter.promises.getInvitedCollaboratorCount(projectId) const inviteCount = await CollaboratorsInvitesHandler.promises.getInviteCount(projectId) - return ( - currentNumber + inviteCount + numberOfNewCollaborators <= allowedNumber || - allowedNumber < 0 // -1 means unlimited - ) + return currentNumber + inviteCount + numberOfNewCollaborators <= allowedNumber } async function canAddXEditCollaborators( @@ -45,6 +45,9 @@ async function canAddXEditCollaborators( numberOfNewEditCollaborators ) { const allowedNumber = await allowedNumberOfCollaboratorsInProject(projectId) + if (allowedNumber < 0) { + return true // -1 means unlimited + } const currentEditors = await CollaboratorsGetter.promises.getInvitedEditCollaboratorCount( projectId @@ -53,7 +56,7 @@ async function canAddXEditCollaborators( await CollaboratorsInvitesHandler.promises.getEditInviteCount(projectId) return ( currentEditors + editInviteCount + numberOfNewEditCollaborators <= - allowedNumber || allowedNumber < 0 // -1 means unlimited + allowedNumber ) } diff --git a/services/web/app/src/models/Project.js b/services/web/app/src/models/Project.js index aecb60c525..b78dadb116 100644 --- a/services/web/app/src/models/Project.js +++ b/services/web/app/src/models/Project.js @@ -39,6 +39,7 @@ const ProjectSchema = new Schema( owner_ref: { type: ObjectId, ref: 'User' }, collaberator_refs: [{ type: ObjectId, ref: 'User' }], readOnly_refs: [{ type: ObjectId, ref: 'User' }], + pendingEditor_refs: [{ type: ObjectId, ref: 'User' }], rootDoc_id: { type: ObjectId }, rootFolder: [FolderSchema], version: { type: Number }, // incremented for every change in the project structure (folders and filenames) diff --git a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx index 1bcd017d19..623b86b8a0 100644 --- a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx @@ -124,9 +124,17 @@ export default function EditMember({
- +
{member.email} + {member.pendingEditor && ( +
Pending editor
+ )} {shouldWarnMember() && (
{t('will_lose_edit_access_on_date', { diff --git a/services/web/frontend/js/shared/context/editor-context.tsx b/services/web/frontend/js/shared/context/editor-context.tsx index e66255b1b1..c060999fbc 100644 --- a/services/web/frontend/js/shared/context/editor-context.tsx +++ b/services/web/frontend/js/shared/context/editor-context.tsx @@ -44,6 +44,7 @@ export const EditorContext = createContext< insertSymbol?: (symbol: string) => void isProjectOwner: boolean isRestrictedTokenMember?: boolean + isPendingEditor: boolean permissionsLevel: 'readOnly' | 'readAndWrite' | 'owner' deactivateTutorial: (tutorial: string) => void inactiveTutorials: string[] @@ -62,7 +63,7 @@ export const EditorProvider: FC = ({ children }) => { const { role } = useDetachContext() const { showGenericMessageModal } = useModalsContext() - const { owner, features, _id: projectId } = useProjectContext() + const { owner, features, _id: projectId, members } = useProjectContext() const cobranding = useMemo(() => { const brandVariation = getMeta('ol-brandVariation') @@ -98,6 +99,17 @@ export const EditorProvider: FC = ({ children }) => { const [currentPopup, setCurrentPopup] = useState(null) + const isPendingEditor = useMemo( + () => + members?.some( + member => + member._id === userId && + member.pendingEditor && + member.privileges === 'readAndWrite' + ), + [members, userId] + ) + const deactivateTutorial = useCallback( tutorialKey => { setInactiveTutorials([...inactiveTutorials, tutorialKey]) @@ -174,6 +186,7 @@ export const EditorProvider: FC = ({ children }) => { setPermissionsLevel, isProjectOwner: owner?._id === userId, isRestrictedTokenMember: getMeta('ol-isRestrictedTokenMember'), + isPendingEditor, showSymbolPalette, toggleSymbolPalette, insertSymbol, @@ -194,6 +207,7 @@ export const EditorProvider: FC = ({ children }) => { renameProject, permissionsLevel, setPermissionsLevel, + isPendingEditor, showSymbolPalette, toggleSymbolPalette, insertSymbol, diff --git a/services/web/frontend/js/shared/context/types/project-context.tsx b/services/web/frontend/js/shared/context/types/project-context.tsx index f167dc060b..3369fc8fd6 100644 --- a/services/web/frontend/js/shared/context/types/project-context.tsx +++ b/services/web/frontend/js/shared/context/types/project-context.tsx @@ -7,6 +7,7 @@ export type ProjectContextMember = { email: string first_name: string last_name: string + pendingEditor?: boolean } export type ProjectContextValue = { diff --git a/services/web/test/acceptance/src/helpers/User.js b/services/web/test/acceptance/src/helpers/User.js index 2cedfb060a..f33b0e8eb3 100644 --- a/services/web/test/acceptance/src/helpers/User.js +++ b/services/web/test/acceptance/src/helpers/User.js @@ -946,6 +946,10 @@ class User { updateOp = { $addToSet: { collaberator_refs: user._id } } } else if (privileges === 'readOnly') { updateOp = { $addToSet: { readOnly_refs: user._id } } + } else if (privileges === 'pendingEditor') { + updateOp = { + $addToSet: { readOnly_refs: user._id, pendingEditor_refs: user._id }, + } } db.projects.updateOne({ _id: new ObjectId(projectId) }, updateOp, callback) } diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js index 47f78c7666..04e625458d 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsGetterTests.js @@ -17,6 +17,7 @@ describe('CollaboratorsGetter', function () { this.ownerRef = new ObjectId() this.readOnlyRef1 = new ObjectId() this.readOnlyRef2 = new ObjectId() + this.pendingEditorRef = new ObjectId() this.readWriteRef1 = new ObjectId() this.readWriteRef2 = new ObjectId() this.readOnlyTokenRef = new ObjectId() @@ -25,7 +26,12 @@ describe('CollaboratorsGetter', function () { this.project = { _id: new ObjectId(), owner_ref: [this.ownerRef], - readOnly_refs: [this.readOnlyRef1, this.readOnlyRef2], + readOnly_refs: [ + this.readOnlyRef1, + this.readOnlyRef2, + this.pendingEditorRef, + ], + pendingEditor_refs: [this.pendingEditorRef], collaberator_refs: [this.readWriteRef1, this.readWriteRef2], tokenAccessReadAndWrite_refs: [this.readWriteTokenRef], tokenAccessReadOnly_refs: [this.readOnlyTokenRef], @@ -99,6 +105,12 @@ describe('CollaboratorsGetter', function () { privilegeLevel: 'readOnly', source: 'invite', }, + { + id: this.pendingEditorRef.toString(), + privilegeLevel: 'readOnly', + source: 'invite', + pendingEditor: true, + }, { id: this.readOnlyTokenRef.toString(), privilegeLevel: 'readOnly', @@ -139,6 +151,7 @@ describe('CollaboratorsGetter', function () { this.readOnlyRef2.toString(), this.readWriteRef1.toString(), this.readWriteRef2.toString(), + this.pendingEditorRef.toString(), this.readWriteTokenRef.toString(), this.readOnlyTokenRef.toString(), ]) @@ -157,6 +170,7 @@ describe('CollaboratorsGetter', function () { this.readOnlyRef2.toString(), this.readWriteRef1.toString(), this.readWriteRef2.toString(), + this.pendingEditorRef.toString(), ]) }) }) @@ -484,4 +498,14 @@ describe('CollaboratorsGetter', function () { expect(count).to.equal(2) }) }) + + describe('getInvitedPendingEditorCount', function () { + it('should return the count of pending editors', async function () { + const count = + await this.CollaboratorsGetter.promises.getInvitedPendingEditorCount( + this.project._id + ) + expect(count).to.equal(1) + }) + }) }) diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js index 96ed8e52e2..7070595268 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js @@ -106,6 +106,7 @@ describe('CollaboratorsHandler', function () { $pull: { collaberator_refs: this.userId, readOnly_refs: this.userId, + pendingEditor_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, archived: this.userId, @@ -148,6 +149,7 @@ describe('CollaboratorsHandler', function () { $pull: { collaberator_refs: this.userId, readOnly_refs: this.userId, + pendingEditor_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, trashed: this.userId, @@ -182,6 +184,7 @@ describe('CollaboratorsHandler', function () { $pull: { collaberator_refs: this.userId, readOnly_refs: this.userId, + pendingEditor_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, archived: this.userId, @@ -377,6 +380,7 @@ describe('CollaboratorsHandler', function () { $pull: { collaberator_refs: this.userId, readOnly_refs: this.userId, + pendingEditor_refs: this.userId, tokenAccessReadOnly_refs: this.userId, tokenAccessReadAndWrite_refs: this.userId, archived: this.userId, @@ -457,6 +461,24 @@ describe('CollaboratorsHandler', function () { ) .chain('exec') .resolves() + this.ProjectMock.expects('updateMany') + .withArgs( + { pendingEditor_refs: this.fromUserId }, + { + $addToSet: { pendingEditor_refs: this.toUserId }, + } + ) + .chain('exec') + .resolves() + this.ProjectMock.expects('updateMany') + .withArgs( + { pendingEditor_refs: this.fromUserId }, + { + $pull: { pendingEditor_refs: this.fromUserId }, + } + ) + .chain('exec') + .resolves() }) describe('successfully', function () { @@ -501,7 +523,10 @@ describe('CollaboratorsHandler', function () { ], }, { - $pull: { collaberator_refs: this.userId }, + $pull: { + collaberator_refs: this.userId, + pendingEditor_refs: this.userId, + }, $addToSet: { readOnly_refs: this.userId }, } ) @@ -526,7 +551,10 @@ describe('CollaboratorsHandler', function () { }, { $addToSet: { collaberator_refs: this.userId }, - $pull: { readOnly_refs: this.userId }, + $pull: { + readOnly_refs: this.userId, + pendingEditor_refs: this.userId, + }, } ) .chain('exec') diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index c210da3b42..72153e25e3 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -131,6 +131,11 @@ describe('ProjectController', function () { isUserInvitedReadWriteMemberOfProject: sinon.stub().resolves(true), }, } + this.CollaboratorsHandler = { + promises: { + setCollaboratorPrivilegeLevel: sinon.stub().resolves(), + }, + } this.ProjectEntityHandler = {} this.UserGetter = { getUserFullEmails: sinon.stub().yields(null, []), @@ -200,12 +205,16 @@ describe('ProjectController', function () { this.OnboardingDataCollectionManager = { getOnboardingDataValue: sinon.stub().resolves(null), } + this.Modules = { + promises: { hooks: { fire: sinon.stub().resolves() } }, + } this.ProjectController = SandboxedModule.require(MODULE_PATH, { requires: { 'mongodb-legacy': { ObjectId }, '@overleaf/settings': this.settings, '@overleaf/metrics': this.Metrics, + '../Collaborators/CollaboratorsHandler': this.CollaboratorsHandler, '../SplitTests/SplitTestHandler': this.SplitTestHandler, '../SplitTests/SplitTestSessionHandler': this.SplitTestSessionHandler, './ProjectDeleter': this.ProjectDeleter, @@ -255,6 +264,7 @@ describe('ProjectController', function () { updateUser: sinon.stub().resolves(), }, }, + '../../infrastructure/Modules': this.Modules, }, }) @@ -1015,9 +1025,13 @@ describe('ProjectController', function () { describe('link sharing changes active', function () { beforeEach(function () { - this.SplitTestHandler.promises.getAssignmentForUser.resolves({ - variant: 'active', - }) + this.SplitTestHandler.promises.getAssignmentForUser.callsFake( + async (userId, test) => { + if (test === 'link-sharing-warning') { + return { variant: 'active' } + } + } + ) }) describe('when user is a read write token member (and not already a named editor)', function () { @@ -1059,6 +1073,57 @@ describe('ProjectController', function () { }) }) }) + + describe('link sharing enforcement', function () { + describe('when not active (default)', function () { + beforeEach(function () { + this.SplitTestHandler.promises.getAssignmentForUser.callsFake( + async (userId, test) => { + if (test === 'link-sharing-warning') { + return { variant: 'active' } + } else if (test === 'link-sharing-enforcement') { + return { variant: 'default' } + } + } + ) + }) + + it('should not call the collaborator limit enforcement check', function (done) { + this.res.render = (pageName, opts) => { + this.Modules.promises.hooks.fire.should.not.have.been.calledWith( + 'enforceCollaboratorLimit' + ) + done() + } + this.ProjectController.loadEditor(this.req, this.res) + }) + }) + + describe('when active', function () { + beforeEach(function () { + this.SplitTestHandler.promises.getAssignmentForUser.callsFake( + async (userId, test) => { + if (test === 'link-sharing-warning') { + return { variant: 'active' } + } else if (test === 'link-sharing-enforcement') { + return { variant: 'active' } + } + } + ) + }) + + it('should call the collaborator limit enforcement check', function (done) { + this.res.render = (pageName, opts) => { + this.Modules.promises.hooks.fire.should.have.been.calledWith( + 'enforceCollaboratorLimit', + this.project_id + ) + done() + } + this.ProjectController.loadEditor(this.req, this.res) + }) + }) + }) }) describe('userProjectsJson', function () {