Allow reviewers to delete their own comment threads (#23044)

GitOrigin-RevId: 2165e0f549c9df923fb1c124a7622a49d579c2e3

services/web/app/src/Features/Authorization/AuthorizationManager.js

@@ -254,7 +254,13 @@ async function isUserSiteAdmin(userId) {
   return hasAdminAccess(user)
 }
 
-async function canUserResolveThread(userId, projectId, docId, threadId, token) {
+async function canUserDeleteOrResolveThread(
+  userId,
+  projectId,
+  docId,
+  threadId,
+  token
+) {
   const privilegeLevel = await getPrivilegeLevelForProject(
     userId,
     projectId,
@@ -297,7 +303,7 @@ module.exports = {
   canUserReadProject: callbackify(canUserReadProject),
   canUserWriteProjectContent: callbackify(canUserWriteProjectContent),
   canUserReviewProjectContent: callbackify(canUserReviewProjectContent),
-  canUserResolveThread: callbackify(canUserResolveThread),
+  canUserDeleteOrResolveThread: callbackify(canUserDeleteOrResolveThread),
   canUserSendComment: callbackify(canUserSendComment),
   canUserWriteProjectSettings: callbackify(canUserWriteProjectSettings),
   canUserRenameProject: callbackify(canUserRenameProject),
@@ -310,7 +316,7 @@ module.exports = {
     canUserReadProject,
     canUserWriteProjectContent,
     canUserReviewProjectContent,
-    canUserResolveThread,
+    canUserDeleteOrResolveThread,
     canUserSendComment,
     canUserWriteProjectSettings,
     canUserRenameProject,

services/web/app/src/Features/Authorization/AuthorizationMiddleware.js

@@ -103,28 +103,31 @@ async function ensureUserCanWriteProjectSettings(req, res, next) {
   next()
 }
 
-async function ensureUserCanResolveThread(req, res, next) {
+async function ensureUserCanDeleteOrResolveThread(req, res, next) {
   const projectId = _getProjectId(req)
   const docId = _getDocId(req)
   const threadId = _getThreadId(req)
   const userId = _getUserId(req)
   const token = TokenAccessHandler.getRequestToken(req, projectId)
-  const canResolveThread =
-    await AuthorizationManager.promises.canUserResolveThread(
+  const canDeleteThread =
+    await AuthorizationManager.promises.canUserDeleteOrResolveThread(
       userId,
       projectId,
       docId,
       threadId,
       token
     )
-  if (canResolveThread) {
-    logger.debug({ userId, projectId }, 'allowing user resolve comment thread')
+  if (canDeleteThread) {
+    logger.debug(
+      { userId, projectId },
+      'allowing user to delete or resolve a comment thread'
+    )
     return next()
   }
 
   logger.debug(
     { userId, projectId, threadId },
-    'denying user to resolve comment thread'
+    'denying user to delete or resolve a comment thread'
   )
   return HttpErrorHandler.forbidden(req, res)
 }
@@ -267,7 +270,9 @@ module.exports = {
   ensureUserCanWriteProjectSettings: expressify(
     ensureUserCanWriteProjectSettings
   ),
-  ensureUserCanResolveThread: expressify(ensureUserCanResolveThread),
+  ensureUserCanDeleteOrResolveThread: expressify(
+    ensureUserCanDeleteOrResolveThread
+  ),
   ensureUserCanSendComment: expressify(ensureUserCanSendComment),
   ensureUserCanWriteProjectContent: expressify(
     ensureUserCanWriteProjectContent

services/web/test/unit/src/Authorization/AuthorizationManagerTests.js

@@ -536,14 +536,14 @@ describe('AuthorizationManager', function () {
     })
   })
 
-  describe('canUserReviewThread', function () {
+  describe('canUserDeleteOrResolveThread', function () {
     it('should return true when user has write permissions', async function () {
       this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel
         .withArgs(this.user._id, this.project._id)
         .resolves(PrivilegeLevels.READ_AND_WRITE)
 
       const canResolve =
-        await this.AuthorizationManager.promises.canUserResolveThread(
+        await this.AuthorizationManager.promises.canUserDeleteOrResolveThread(
           this.user._id,
           this.project._id,
           this.doc._id,
@@ -560,7 +560,7 @@ describe('AuthorizationManager', function () {
         .resolves(PrivilegeLevels.READ_ONLY)
 
       const canResolve =
-        await this.AuthorizationManager.promises.canUserResolveThread(
+        await this.AuthorizationManager.promises.canUserDeleteOrResolveThread(
           this.user._id,
           this.project._id,
           this.doc._id,
@@ -580,7 +580,7 @@ describe('AuthorizationManager', function () {
 
       it('should return false when user is not the comment author', async function () {
         const canResolve =
-          await this.AuthorizationManager.promises.canUserResolveThread(
+          await this.AuthorizationManager.promises.canUserDeleteOrResolveThread(
             this.user._id,
             this.project._id,
             this.doc._id,
@@ -597,7 +597,7 @@ describe('AuthorizationManager', function () {
           .resolves({ metadata: { user_id: this.user._id } })
 
         const canResolve =
-          await this.AuthorizationManager.promises.canUserResolveThread(
+          await this.AuthorizationManager.promises.canUserDeleteOrResolveThread(
             this.user._id,
             this.project._id,
             this.doc._id,

services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js

@@ -25,7 +25,7 @@ describe('AuthorizationMiddleware', function () {
         canUserReadProject: sinon.stub(),
         canUserWriteProjectSettings: sinon.stub(),
         canUserWriteProjectContent: sinon.stub(),
-        canUserResolveThread: sinon.stub(),
+        canUserDeleteOrResolveThread: sinon.stub(),
         canUserSendComment: sinon.stub(),
         canUserAdminProject: sinon.stub(),
         canUserRenameProject: sinon.stub(),
@@ -91,14 +91,14 @@ describe('AuthorizationMiddleware', function () {
     testMiddleware('ensureUserCanSendComment', 'canUserSendComment')
   })
 
-  describe('ensureUserCanResolveThread', function () {
+  describe('ensureUserCanDeleteOrResolveThread', function () {
     beforeEach(function () {
       this.req.params.doc_id = this.doc_id
       this.req.params.thread_id = this.thread_id
     })
     describe('when user has permission', function () {
       beforeEach(function () {
-        this.AuthorizationManager.promises.canUserResolveThread
+        this.AuthorizationManager.promises.canUserDeleteOrResolveThread
           .withArgs(
             this.userId,
             this.project_id,
@@ -109,13 +109,13 @@ describe('AuthorizationMiddleware', function () {
           .resolves(true)
       })
 
-      invokeMiddleware('ensureUserCanResolveThread')
+      invokeMiddleware('ensureUserCanDeleteOrResolveThread')
       expectNext()
     })
 
     describe("when user doesn't have permission", function () {
       beforeEach(function () {
-        this.AuthorizationManager.promises.canUserResolveThread
+        this.AuthorizationManager.promises.canUserDeleteOrResolveThread
           .withArgs(
             this.userId,
             this.project_id,
@@ -126,7 +126,7 @@ describe('AuthorizationMiddleware', function () {
           .resolves(false)
       })
 
-      invokeMiddleware('ensureUserCanResolveThread')
+      invokeMiddleware('ensureUserCanDeleteOrResolveThread')
       expectForbidden()
     })
   })