6 files changed, 131 insertions, 99 deletions

diff --git a/server/helpers/custom-validators/activitypub/video-comments.ts b/server/helpers/custom-validators/activitypub/video-comments.ts
index 96655c3f8..ea780ca46 100644
--- a/server/helpers/custom-validators/activitypub/video-comments.ts
+++ b/server/helpers/custom-validators/activitypub/video-comments.ts
@@ -3,18 +3,10 @@ import { ACTIVITY_PUB } from '../../../initializers/constants'
 import { exists, isArray, isDateValid } from '../misc'
 import { isActivityPubUrlValid } from './misc'
 
-function isTypeValid (comment: any): boolean {
-  if (comment.type === 'Note') return true
-
-  if (comment.type === 'Tombstone' && comment.formerType === 'Note') return true
-
-  return false
-}
-
 function sanitizeAndCheckVideoCommentObject (comment: any) {
   if (!comment) return false
 
-  if (!isTypeValid(comment)) return false
+  if (!isCommentTypeValid(comment)) return false
 
   normalizeComment(comment)
 
@@ -59,3 +51,11 @@ function normalizeComment (comment: any) {
 
   return
 }
+
+function isCommentTypeValid (comment: any): boolean {
+  if (comment.type === 'Note') return true
+
+  if (comment.type === 'Tombstone' && comment.formerType === 'Note') return true
+
+  return false
+}
diff --git a/server/lib/activitypub/send/send-delete.ts b/server/lib/activitypub/send/send-delete.ts
index a91756ff4..3225ebf32 100644
--- a/server/lib/activitypub/send/send-delete.ts
+++ b/server/lib/activitypub/send/send-delete.ts
@@ -53,16 +53,17 @@ async function sendDeleteVideoComment (videoComment: MCommentOwnerVideoReply, t:
     : videoComment.Video.VideoChannel.Account.Actor
 
   const threadParentComments = await VideoCommentModel.listThreadParentComments(videoComment, t)
+  const threadParentCommentsFiltered = threadParentComments.filter(c => !c.isDeleted())
 
   const actorsInvolvedInComment = await getActorsInvolvedInVideo(videoComment.Video, t)
   actorsInvolvedInComment.push(byActor) // Add the actor that commented the video
 
-  const audience = getVideoCommentAudience(videoComment, threadParentComments, actorsInvolvedInComment, isVideoOrigin)
+  const audience = getVideoCommentAudience(videoComment, threadParentCommentsFiltered, actorsInvolvedInComment, isVideoOrigin)
   const activity = buildDeleteActivity(url, videoComment.url, byActor, audience)
 
   // This was a reply, send it to the parent actors
   const actorsException = [ byActor ]
-  await broadcastToActors(activity, byActor, threadParentComments.map(c => c.Account.Actor), t, actorsException)
+  await broadcastToActors(activity, byActor, threadParentCommentsFiltered.map(c => c.Account.Actor), t, actorsException)
 
   // Broadcast to our followers
   await broadcastToFollowers(activity, byActor, [ byActor ], t)
diff --git a/server/lib/activitypub/video-comments.ts b/server/lib/activitypub/video-comments.ts
index 1a15842cf..d5c078a29 100644
--- a/server/lib/activitypub/video-comments.ts
+++ b/server/lib/activitypub/video-comments.ts
@@ -141,7 +141,10 @@ async function resolveParentComment (params: ResolveThreadParams) {
     throw new Error(`Comment url ${url} host is different from the AP object id ${body.id}`)
   }
 
-  const actor = actorUrl ? await getOrCreateActorAndServerAndModel(actorUrl, 'all') : null
+  const actor = actorUrl
+    ? await getOrCreateActorAndServerAndModel(actorUrl, 'all')
+    : null
+
   const comment = new VideoCommentModel({
     url: body.id,
     text: body.content ? body.content : '',
diff --git a/server/middlewares/validators/videos/video-comments.ts b/server/middlewares/validators/videos/video-comments.ts
index 1d81eb5d8..eb07d9430 100644
--- a/server/middlewares/validators/videos/video-comments.ts
+++ b/server/middlewares/validators/videos/video-comments.ts
@@ -189,6 +189,13 @@ function isVideoCommentsEnabled (video: MVideo, res: express.Response) {
 }
 
 function checkUserCanDeleteVideoComment (user: MUser, videoComment: MCommentOwner, res: express.Response) {
+  if (videoComment.isDeleted()) {
+    res.status(409)
+      .json({ error: 'This comment is already deleted' })
+      .end()
+    return false
+  }
+
   const account = videoComment.Account
   if (user.hasRight(UserRight.REMOVE_ANY_VIDEO_COMMENT) === false && account.userId !== user.id) {
     res.status(403)
diff --git a/server/tests/api/server/follows.ts b/server/tests/api/server/follows.ts
index dd85722a0..60dbe2e6d 100644
--- a/server/tests/api/server/follows.ts
+++ b/server/tests/api/server/follows.ts
@@ -4,7 +4,7 @@ import * as chai from 'chai'
 import 'mocha'
 import { Video, VideoPrivacy } from '../../../../shared/models/videos'
 import { VideoComment, VideoCommentThreadTree } from '../../../../shared/models/videos/video-comment.model'
-import { cleanupTests, completeVideoCheck } from '../../../../shared/extra-utils'
+import { cleanupTests, completeVideoCheck, deleteVideoComment } from '../../../../shared/extra-utils'
 import {
   flushAndRunMultipleServers,
   getVideosList,
@@ -356,19 +356,40 @@ describe('Test follows', function () {
         }
 
         {
-          const text = 'my super first comment'
-          const res = await addVideoCommentThread(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, text)
-          const threadId = res.body.comment.id
+          {
+            const text = 'my super first comment'
+            const res = await addVideoCommentThread(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, text)
+            const threadId = res.body.comment.id
+
+            const text1 = 'my super answer to thread 1'
+            const childCommentRes = await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId, text1)
+            const childCommentId = childCommentRes.body.comment.id
+
+            const text2 = 'my super answer to answer of thread 1'
+            await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, childCommentId, text2)
+
+            const text3 = 'my second answer to thread 1'
+            await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId, text3)
+          }
+
+          {
+            const text = 'will be deleted'
+            const res = await addVideoCommentThread(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, text)
+            const threadId = res.body.comment.id
+
+            const text1 = 'answer to deleted'
+            await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId, text1)
 
-          const text1 = 'my super answer to thread 1'
-          const childCommentRes = await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId, text1)
-          const childCommentId = childCommentRes.body.comment.id
+            const text2 = 'will also be deleted'
+            const childCommentRes = await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId, text2)
+            const childCommentId = childCommentRes.body.comment.id
 
-          const text2 = 'my super answer to answer of thread 1'
-          await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, childCommentId, text2)
+            const text3 = 'my second answer to deleted'
+            await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, childCommentId, text3)
 
-          const text3 = 'my second answer to thread 1'
-          await addVideoCommentReply(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId, text3)
+            await deleteVideoComment(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, threadId)
+            await deleteVideoComment(servers[ 2 ].url, servers[ 2 ].accessToken, video4.id, childCommentId)
+          }
         }
 
         {
@@ -453,42 +474,80 @@ describe('Test follows', function () {
     })
 
     it('Should have propagated comments', async function () {
-      const res1 = await getVideoCommentThreads(servers[0].url, video4.id, 0, 5)
+      const res1 = await getVideoCommentThreads(servers[0].url, video4.id, 0, 5, 'createdAt')
 
-      expect(res1.body.total).to.equal(1)
+      expect(res1.body.total).to.equal(2)
       expect(res1.body.data).to.be.an('array')
-      expect(res1.body.data).to.have.lengthOf(1)
-
-      const comment: VideoComment = res1.body.data[0]
-      expect(comment.inReplyToCommentId).to.be.null
-      expect(comment.text).equal('my super first comment')
-      expect(comment.videoId).to.equal(video4.id)
-      expect(comment.id).to.equal(comment.threadId)
-      expect(comment.account.name).to.equal('root')
-      expect(comment.account.host).to.equal('localhost:' + servers[2].port)
-      expect(comment.totalReplies).to.equal(3)
-      expect(dateIsValid(comment.createdAt as string)).to.be.true
-      expect(dateIsValid(comment.updatedAt as string)).to.be.true
-
-      const threadId = comment.threadId
-
-      const res2 = await getVideoThreadComments(servers[0].url, video4.id, threadId)
-
-      const tree: VideoCommentThreadTree = res2.body
-      expect(tree.comment.text).equal('my super first comment')
-      expect(tree.children).to.have.lengthOf(2)
-
-      const firstChild = tree.children[0]
-      expect(firstChild.comment.text).to.equal('my super answer to thread 1')
-      expect(firstChild.children).to.have.lengthOf(1)
-
-      const childOfFirstChild = firstChild.children[0]
-      expect(childOfFirstChild.comment.text).to.equal('my super answer to answer of thread 1')
-      expect(childOfFirstChild.children).to.have.lengthOf(0)
-
-      const secondChild = tree.children[1]
-      expect(secondChild.comment.text).to.equal('my second answer to thread 1')
-      expect(secondChild.children).to.have.lengthOf(0)
+      expect(res1.body.data).to.have.lengthOf(2)
+
+      {
+        const comment: VideoComment = res1.body.data[ 0 ]
+        expect(comment.inReplyToCommentId).to.be.null
+        expect(comment.text).equal('my super first comment')
+        expect(comment.videoId).to.equal(video4.id)
+        expect(comment.id).to.equal(comment.threadId)
+        expect(comment.account.name).to.equal('root')
+        expect(comment.account.host).to.equal('localhost:' + servers[ 2 ].port)
+        expect(comment.totalReplies).to.equal(3)
+        expect(dateIsValid(comment.createdAt as string)).to.be.true
+        expect(dateIsValid(comment.updatedAt as string)).to.be.true
+
+        const threadId = comment.threadId
+
+        const res2 = await getVideoThreadComments(servers[ 0 ].url, video4.id, threadId)
+
+        const tree: VideoCommentThreadTree = res2.body
+        expect(tree.comment.text).equal('my super first comment')
+        expect(tree.children).to.have.lengthOf(2)
+
+        const firstChild = tree.children[ 0 ]
+        expect(firstChild.comment.text).to.equal('my super answer to thread 1')
+        expect(firstChild.children).to.have.lengthOf(1)
+
+        const childOfFirstChild = firstChild.children[ 0 ]
+        expect(childOfFirstChild.comment.text).to.equal('my super answer to answer of thread 1')
+        expect(childOfFirstChild.children).to.have.lengthOf(0)
+
+        const secondChild = tree.children[ 1 ]
+        expect(secondChild.comment.text).to.equal('my second answer to thread 1')
+        expect(secondChild.children).to.have.lengthOf(0)
+      }
+
+      {
+        const deletedComment: VideoComment = res1.body.data[1]
+        expect(deletedComment).to.not.be.undefined
+        expect(deletedComment.isDeleted).to.be.true
+        expect(deletedComment.deletedAt).to.not.be.null
+        expect(deletedComment.text).to.equal('')
+        expect(deletedComment.inReplyToCommentId).to.be.null
+        expect(deletedComment.account).to.be.null
+        expect(deletedComment.totalReplies).to.equal(3)
+        expect(dateIsValid(deletedComment.deletedAt as string)).to.be.true
+
+        const res2 = await getVideoThreadComments(servers[0].url, video4.id, deletedComment.threadId)
+
+        const tree: VideoCommentThreadTree = res2.body
+        const [ commentRoot, deletedChildRoot ] = tree.children
+
+        expect(deletedChildRoot).to.not.be.undefined
+        expect(deletedChildRoot.comment.isDeleted).to.be.true
+        expect(deletedChildRoot.comment.deletedAt).to.not.be.null
+        expect(deletedChildRoot.comment.text).to.equal('')
+        expect(deletedChildRoot.comment.inReplyToCommentId).to.equal(deletedComment.id)
+        expect(deletedChildRoot.comment.account).to.be.null
+        expect(deletedChildRoot.children).to.have.lengthOf(1)
+
+        const answerToDeletedChild = deletedChildRoot.children[0]
+        expect(answerToDeletedChild.comment).to.not.be.undefined
+        expect(answerToDeletedChild.comment.inReplyToCommentId).to.equal(deletedChildRoot.comment.id)
+        expect(answerToDeletedChild.comment.text).to.equal('my second answer to deleted')
+        expect(answerToDeletedChild.comment.account.name).to.equal('root')
+
+        expect(commentRoot.comment).to.not.be.undefined
+        expect(commentRoot.comment.inReplyToCommentId).to.equal(deletedComment.id)
+        expect(commentRoot.comment.text).to.equal('answer to deleted')
+        expect(commentRoot.comment.account.name).to.equal('root')
+      }
     })
 
     it('Should have propagated captions', async function () {
diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts
index 836bdc658..22d87b88c 100644
--- a/server/tests/api/videos/multiple-servers.ts
+++ b/server/tests/api/videos/multiple-servers.ts
@@ -517,6 +517,8 @@ describe('Test multiple servers', function () {
       // Wait the repeatable job
       await wait(6000)
 
+      await waitJobs(servers)
+
       for (const server of servers) {
         const res = await getVideosList(server.url)
 
@@ -551,6 +553,8 @@ describe('Test multiple servers', function () {
       // Wait the repeatable job
       await wait(16000)
 
+      await waitJobs(servers)
+
       let baseVideos = null
 
       for (const server of servers) {
@@ -939,48 +943,6 @@ describe('Test multiple servers', function () {
       }
     })
 
-    it('Should retrieve all comments when subscribing to a new server', async function () {
-      this.timeout(120000)
-
-      const newServer = await flushAndRunServer(4)
-
-      await setAccessTokensToServers([newServer])
-      await doubleFollow(newServer, servers[0])
-      await doubleFollow(newServer, servers[2])
-      await waitJobs([newServer, ...servers])
-
-      const res = await getVideoCommentThreads(newServer.url, videoUUID, 0, 5)
-
-      expect(res.body.total).to.equal(2)
-      expect(res.body.data).to.be.an('array')
-      expect(res.body.data).to.have.lengthOf(2)
-
-      {
-        const comment: VideoComment = res.body.data[0]
-        expect(comment).to.not.be.undefined
-        expect(comment.inReplyToCommentId).to.be.null
-        expect(comment.account.name).to.equal('root')
-        expect(comment.account.host).to.equal('localhost:' + servers[2].port)
-        expect(comment.totalReplies).to.equal(0)
-        expect(dateIsValid(comment.createdAt as string)).to.be.true
-        expect(dateIsValid(comment.updatedAt as string)).to.be.true
-      }
-
-      {
-        const deletedComment: VideoComment = res.body.data[1]
-        expect(deletedComment).to.not.be.undefined
-        expect(deletedComment.isDeleted).to.be.true
-        expect(deletedComment.deletedAt).to.not.be.null
-        expect(deletedComment.text).to.equal('')
-        expect(deletedComment.inReplyToCommentId).to.be.null
-        expect(deletedComment.account).to.be.null
-        expect(deletedComment.totalReplies).to.equal(3)
-        expect(dateIsValid(deletedComment.createdAt as string)).to.be.true
-        expect(dateIsValid(deletedComment.updatedAt as string)).to.be.true
-        expect(dateIsValid(deletedComment.deletedAt as string)).to.be.true
-      }
-    })
-
     it('Should delete a remote thread by the origin server', async function () {
       this.timeout(5000)