agw/overleaf-cep
1
0
Fork 0
mirror of https://github.com/yu-i-i/overleaf-cep.git synced 2026-05-30 12:24:25 +02:00
Code Issues Packages Projects Releases Wiki Activity

Set track changes state permissions for reviewer role (#22345) (#22436)

Browse Source 
* Support for adding reviewer role

* added collaboratorsGetter tests

* emit toggle-track-changes when reviewer is added

* Add reviewer in change privilege level handler

GitOrigin-RevId: 88ec39f2b760b5d1ca6dc3a363df31c087268972
This commit is contained in:
Domagoj Kriskovic
2024-12-11 10:51:39 +01:00
committed by Copybot
parent 0490a4bab2
commit 71a0b48a68
9 changed files with 226 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
services/web/app/src/Features/Collaborators/CollaboratorsHandler.js
View File

@@ -284,20 +284,39 @@ async function setCollaboratorPrivilegeLevel(
// collaborator
const query = {
_id: projectId,
$or: [{ collaberator_refs: userId }, { readOnly_refs: userId }],
$or: [
{ collaberator_refs: userId },
{ readOnly_refs: userId },
{ reviewer_refs: userId },
],
}
let update
switch (privilegeLevel) {
case PrivilegeLevels.READ_AND_WRITE: {
update = {
$pull: { readOnly_refs: userId, pendingEditor_refs: userId },
$pull: {
readOnly_refs: userId,
pendingEditor_refs: userId,
reviewer_refs: userId,
},
$addToSet: { collaberator_refs: userId },
}
break
}
case PrivilegeLevels.REVIEW: {
update = {
$pull: {
readOnly_refs: userId,
pendingEditor_refs: userId,
collaberator_refs: userId,
},
$addToSet: { reviewer_refs: userId },
}
break
}
case PrivilegeLevels.READ_ONLY: {
update = {
$pull: { collaberator_refs: userId },
$pull: { collaberator_refs: userId, reviewer_refs: userId },
$addToSet: { readOnly_refs: userId },
}
if (pendingEditor) {

6
services/web/app/src/Features/Collaborators/CollaboratorsRouter.mjs
View File

@@ -50,7 +50,11 @@ export default {
}),
body: Joi.object({
privilegeLevel: Joi.string()
.valid(PrivilegeLevels.READ_ONLY, PrivilegeLevels.READ_AND_WRITE)
.valid(
PrivilegeLevels.READ_ONLY,
PrivilegeLevels.READ_AND_WRITE,
PrivilegeLevels.REVIEW
)
.required(),
}),
}),

32
services/web/app/src/Features/Helpers/AuthorizationHelper.js
View File

@@ -1,7 +1,14 @@
const { UserSchema } = require('../../models/User')
const SplitTestHandler = require('../SplitTests/SplitTestHandler')
const ProjectGetter = require('../Project/ProjectGetter')
const { callbackify } = require('@overleaf/promise-utils')
module.exports = {
hasAnyStaffAccess,
isReviewerRoleEnabled: callbackify(isReviewerRoleEnabled),
promises: {
isReviewerRoleEnabled,
},
}
function hasAnyStaffAccess(user) {
@@ -14,3 +21,28 @@ function hasAnyStaffAccess(user) {
}
return false
}
async function isReviewerRoleEnabled(userId, projectId) {
const project = await ProjectGetter.promises.getProject(projectId, {
reviewer_refs: 1,
owner_ref: 1,
})
// if there are reviewers, it means the role is enabled
if (Object.keys(project.reviewer_refs || {}).length > 0) {
return true
}
// if there are no reviewers, check split test from project owner
if (project.owner_ref === userId) {
const reviewerRoleAssigment =
await SplitTestHandler.promises.getAssignmentForUser(
userId,
'reviewer-role'
)
return reviewerRoleAssigment.variant === 'enabled'
}
return false
}

1
services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/edit-member.tsx
View File

@@ -92,6 +92,7 @@ export default function EditMember({
)
} else if (
newPrivileges === 'readAndWrite' ||
newPrivileges === 'review' ||
newPrivileges === 'readOnly'
) {
monitorRequest(() =>

1
services/web/locales/en.json
View File

@@ -1824,6 +1824,7 @@
"revert_pending_plan_change": "Revert scheduled plan change",
"review": "Review",
"review_your_peers_work": "Review your peers’ work",
"reviewer": "Reviewer",
"revoke": "Revoke",
"revoke_invite": "Revoke Invite",
"right": "Right",

50
services/web/test/acceptance/src/SharingTests.mjs
View File

@@ -8,12 +8,15 @@ describe('Sharing', function () {
this.ownerSession = new User()
this.collaboratorSession = new User()
this.strangerSession = new User()
this.reviewerSession = new User()
await this.ownerSession.login()
await this.collaboratorSession.login()
await this.strangerSession.login()
await this.reviewerSession.login()
this.owner = await this.ownerSession.get()
this.collaborator = await this.collaboratorSession.get()
this.stranger = await this.strangerSession.get()
this.reviewer = await this.reviewerSession.get()
this.projectId = await this.ownerSession.createProject('Test project')
})
@@ -35,6 +38,19 @@ describe('Sharing', function () {
const project = await this.ownerSession.getProject(this.projectId)
expect(project.collaberator_refs).to.deep.equal([this.collaborator._id])
expect(project.readOnly_refs).to.deep.equal([])
expect(project.reviewer_refs).to.deep.equal([])
})
it('sets the privilege level to review', async function () {
await this.ownerSession.setCollaboratorInfo(
this.projectId,
this.collaborator._id,
{ privilegeLevel: 'review' }
)
const project = await this.ownerSession.getProject(this.projectId)
expect(project.reviewer_refs).to.deep.equal([this.collaborator._id])
expect(project.collaberator_refs).to.deep.equal([])
expect(project.readOnly_refs).to.deep.equal([])
})
it('treats setting the privilege to read-only as a noop', async function () {
@@ -45,6 +61,7 @@ describe('Sharing', function () {
)
const project = await this.ownerSession.getProject(this.projectId)
expect(project.collaberator_refs).to.deep.equal([])
expect(project.reviewer_refs).to.deep.equal([])
expect(project.readOnly_refs).to.deep.equal([this.collaborator._id])
})
@@ -96,7 +113,40 @@ describe('Sharing', function () {
)
const project = await this.ownerSession.getProject(this.projectId)
expect(project.collaberator_refs).to.deep.equal([])
expect(project.reviewer_refs).to.deep.equal([])
expect(project.readOnly_refs).to.deep.equal([this.collaborator._id])
})
})
describe('with reviewer collaborator', function () {
beforeEach(async function () {
await this.ownerSession.addUserToProject(
this.projectId,
this.reviewer,
'review'
)
})
it('prevents non-owners to set the privilege level', async function () {
await expect(
this.collaboratorSession.setCollaboratorInfo(
this.projectId,
this.reviewer._id,
{ privilegeLevel: 'review' }
)
).to.be.rejectedWith(/failed: status=403 /)
})
it('sets the privilege level to read-only', async function () {
await this.ownerSession.setCollaboratorInfo(
this.projectId,
this.reviewer._id,
{ privilegeLevel: 'readOnly' }
)
const project = await this.ownerSession.getProject(this.projectId)
expect(project.collaberator_refs).to.deep.equal([])
expect(project.reviewer_refs).to.deep.equal([])
expect(project.readOnly_refs).to.deep.equal([this.reviewer._id])
})
})
})

4
services/web/test/acceptance/src/helpers/User.js
View File

@@ -968,6 +968,10 @@ class User {
updateOp = {
$addToSet: { readOnly_refs: user._id, pendingEditor_refs: user._id },
}
} else if (privileges === 'review') {
updateOp = {
$addToSet: { reviewer_refs: user._id },
}
}
db.projects.updateOne({ _id: new ObjectId(projectId) }, updateOp, callback)
}

35
services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js
View File

@@ -590,12 +590,14 @@ describe('CollaboratorsHandler', function () {
$or: [
{ collaberator_refs: this.userId },
{ readOnly_refs: this.userId },
{ reviewer_refs: this.userId },
],
},
{
$pull: {
collaberator_refs: this.userId,
pendingEditor_refs: this.userId,
reviewer_refs: this.userId,
},
$addToSet: { readOnly_refs: this.userId },
}
@@ -617,12 +619,14 @@ describe('CollaboratorsHandler', function () {
$or: [
{ collaberator_refs: this.userId },
{ readOnly_refs: this.userId },
{ reviewer_refs: this.userId },
],
},
{
$addToSet: { collaberator_refs: this.userId },
$pull: {
readOnly_refs: this.userId,
reviewer_refs: this.userId,
pendingEditor_refs: this.userId,
},
}
@@ -636,6 +640,35 @@ describe('CollaboratorsHandler', function () {
)
})
it('sets a collaborator to reviewer', async function () {
this.ProjectMock.expects('updateOne')
.withArgs(
{
_id: this.projectId,
$or: [
{ collaberator_refs: this.userId },
{ readOnly_refs: this.userId },
{ reviewer_refs: this.userId },
],
},
{
$addToSet: { reviewer_refs: this.userId },
$pull: {
readOnly_refs: this.userId,
collaberator_refs: this.userId,
pendingEditor_refs: this.userId,
},
}
)
.chain('exec')
.resolves({ matchedCount: 1 })
await this.CollaboratorsHandler.promises.setCollaboratorPrivilegeLevel(
this.projectId,
this.userId,
'review'
)
})
it('sets a collaborator to read-only as a pendingEditor', async function () {
this.ProjectMock.expects('updateOne')
.withArgs(
@@ -644,6 +677,7 @@ describe('CollaboratorsHandler', function () {
$or: [
{ collaberator_refs: this.userId },
{ readOnly_refs: this.userId },
{ reviewer_refs: this.userId },
],
},
{
@@ -653,6 +687,7 @@ describe('CollaboratorsHandler', function () {
},
$pull: {
collaberator_refs: this.userId,
reviewer_refs: this.userId,
},
}
)

76
services/web/test/unit/src/HelperFiles/AuthorizationHelperTests.js
View File

@@ -25,6 +25,10 @@ describe('AuthorizationHelper', function () {
},
},
},
'../Project/ProjectGetter': (this.ProjectGetter = { promises: {} }),
'../SplitTests/SplitTestHandler': (this.SplitTestHandler = {
promises: {},
}),
},
})
})
@@ -59,4 +63,76 @@ describe('AuthorizationHelper', function () {
expect(this.AuthorizationHelper.hasAnyStaffAccess(user)).to.be.false
})
})
describe('isReviewerRoleEnabled', function () {
it('with no reviewers and no split test', async function () {
this.ProjectGetter.promises.getProject = sinon.stub().resolves({
reviewer_refs: {},
owner_ref: 'ownerId',
})
this.SplitTestHandler.promises.getAssignmentForUser = sinon
.stub()
.resolves({
variant: 'disabled',
})
expect(
await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
'userId',
'projectId'
)
).to.be.false
})
it('with no reviewers and enabled split test', async function () {
this.ProjectGetter.promises.getProject = sinon.stub().resolves({
reviewer_refs: {},
owner_ref: 'userId',
})
this.SplitTestHandler.promises.getAssignmentForUser = sinon
.stub()
.resolves({
variant: 'enabled',
})
expect(
await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
'userId',
'projectId'
)
).to.be.true
})
it('with reviewers and disabled split test', async function () {
this.ProjectGetter.promises.getProject = sinon.stub().resolves({
reviewer_refs: [{ $oid: 'userId' }],
})
this.SplitTestHandler.promises.getAssignmentForUser = sinon
.stub()
.resolves({
variant: 'default',
})
expect(
await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
'userId',
'projectId'
)
).to.be.true
})
it('with reviewers and enabled split test', async function () {
this.ProjectGetter.promises.getProject = sinon.stub().resolves({
reviewer_refs: [{ $oid: 'userId' }],
})
this.SplitTestHandler.promises.getAssignmentForUser = sinon
.stub()
.resolves({
variant: 'enabled',
})
expect(
await this.AuthorizationHelper.promises.isReviewerRoleEnabled(
'userId',
'projectId'
)
).to.be.true
})
})
})