Remove comment federation by video owner

5 files changed, 49 insertions, 54 deletions

diff --git a/server/controllers/api/videos/comment.ts b/server/controllers/api/videos/comment.ts
index 39d521f5f..bc6d81a7c 100644
--- a/server/controllers/api/videos/comment.ts
+++ b/server/controllers/api/videos/comment.ts
@@ -27,6 +27,10 @@ import { auditLoggerFactory, CommentAuditView, getAuditIdFromRes } from '../../.
 import { AccountModel } from '../../../models/account/account'
 import { Notifier } from '../../../lib/notifier'
 import { Hooks } from '../../../lib/plugins/hooks'
+import { ActorModel } from '../../../models/activitypub/actor'
+import { VideoChannelModel } from '../../../models/video/video-channel'
+import { VideoModel } from '../../../models/video/video'
+import { sendDeleteVideoComment } from '../../../lib/activitypub/send'
 
 const auditLogger = auditLoggerFactory('comments')
 const videoCommentRouter = express.Router()
@@ -179,6 +183,10 @@ async function removeVideoComment (req: express.Request, res: express.Response)
 
   await sequelizeTypescript.transaction(async t => {
     await videoCommentInstance.destroy({ transaction: t })
+
+    if (videoCommentInstance.isOwned() || videoCommentInstance.Video.isOwned()) {
+      await sendDeleteVideoComment(videoCommentInstance, t)
+    }
   })
 
   auditLogger.delete(getAuditIdFromRes(res), new CommentAuditView(videoCommentInstance.toFormattedJSON()))
diff --git a/server/lib/activitypub/process/process-delete.ts b/server/lib/activitypub/process/process-delete.ts
index 845a7b249..9fcfd9e3a 100644
--- a/server/lib/activitypub/process/process-delete.ts
+++ b/server/lib/activitypub/process/process-delete.ts
@@ -34,7 +34,7 @@ async function processDeleteActivity (options: APProcessorOptions<ActivityDelete
   }
 
   {
-    const videoCommentInstance = await VideoCommentModel.loadByUrlAndPopulateAccount(objectUrl)
+    const videoCommentInstance = await VideoCommentModel.loadByUrlAndPopulateAccountAndVideo(objectUrl)
     if (videoCommentInstance) {
       return retryTransactionWrapper(processDeleteVideoComment, byActor, videoCommentInstance, activity)
     }
@@ -121,8 +121,8 @@ function processDeleteVideoComment (byActor: ActorModel, videoComment: VideoComm
   logger.debug('Removing remote video comment "%s".', videoComment.url)
 
   return sequelizeTypescript.transaction(async t => {
-    if (videoComment.Account.id !== byActor.Account.id) {
-      throw new Error('Account ' + byActor.url + ' does not own video comment ' + videoComment.url)
+    if (byActor.Account.id !== videoComment.Account.id && byActor.Account.id !== videoComment.Video.VideoChannel.accountId) {
+      throw new Error(`Account ${byActor.url} does not own video comment ${videoComment.url} or video ${videoComment.Video.url}`)
     }
 
     await videoComment.destroy({ transaction: t })
diff --git a/server/lib/activitypub/send/send-delete.ts b/server/lib/activitypub/send/send-delete.ts
index 7a1d6f0ba..6c7fb8449 100644
--- a/server/lib/activitypub/send/send-delete.ts
+++ b/server/lib/activitypub/send/send-delete.ts
@@ -48,7 +48,10 @@ async function sendDeleteVideoComment (videoComment: VideoCommentModel, t: Trans
   const isVideoOrigin = videoComment.Video.isOwned()
 
   const url = getDeleteActivityPubUrl(videoComment.url)
-  const byActor = videoComment.Account.Actor
+  const byActor = videoComment.isOwned()
+    ? videoComment.Account.Actor
+    : videoComment.Video.VideoChannel.Account.Actor
+
   const threadParentComments = await VideoCommentModel.listThreadParentComments(videoComment, t)
 
   const actorsInvolvedInComment = await getActorsInvolvedInVideo(videoComment.Video, t)
diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts
index 6eda32f05..58b75510d 100644
--- a/server/models/video/video-comment.ts
+++ b/server/models/video/video-comment.ts
@@ -106,6 +106,10 @@ enum ScopeNames {
             required: true,
             include: [
               {
+                model: ActorModel,
+                required: true
+              },
+              {
                 model: AccountModel,
                 required: true,
                 include: [
@@ -208,41 +212,6 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
   })
   Account: AccountModel
 
-  @BeforeDestroy
-  static async sendDeleteIfOwned (instance: VideoCommentModel, options) {
-    if (!instance.Account || !instance.Account.Actor) {
-      instance.Account = await instance.$get('Account', {
-        include: [ ActorModel ],
-        transaction: options.transaction
-      }) as AccountModel
-    }
-
-    if (!instance.Video) {
-      instance.Video = await instance.$get('Video', {
-        include: [
-          {
-            model: VideoChannelModel,
-            include: [
-              {
-                model: AccountModel,
-                include: [
-                  {
-                    model: ActorModel
-                  }
-                ]
-              }
-            ]
-          }
-        ],
-        transaction: options.transaction
-      }) as VideoModel
-    }
-
-    if (instance.isOwned()) {
-      await sendDeleteVideoComment(instance, options.transaction)
-    }
-  }
-
   static loadById (id: number, t?: Transaction) {
     const query: FindOptions = {
       where: {
@@ -269,7 +238,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
       .findOne(query)
   }
 
-  static loadByUrlAndPopulateAccount (url: string, t?: Transaction) {
+  static loadByUrlAndPopulateAccountAndVideo (url: string, t?: Transaction) {
     const query: FindOptions = {
       where: {
         url
@@ -278,7 +247,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
 
     if (t !== undefined) query.transaction = t
 
-    return VideoCommentModel.scope([ ScopeNames.WITH_ACCOUNT ]).findOne(query)
+    return VideoCommentModel.scope([ ScopeNames.WITH_ACCOUNT, ScopeNames.WITH_VIDEO ]).findOne(query)
   }
 
   static loadByUrlAndPopulateReplyAndVideoUrlAndAccount (url: string, t?: Transaction) {
diff --git a/server/tests/api/videos/multiple-servers.ts b/server/tests/api/videos/multiple-servers.ts
index e811ccd8e..651765776 100644
--- a/server/tests/api/videos/multiple-servers.ts
+++ b/server/tests/api/videos/multiple-servers.ts
@@ -500,20 +500,18 @@ describe('Test multiple servers', function () {
     it('Should view multiple videos on owned servers', async function () {
       this.timeout(30000)
 
-      const tasks: Promise<any>[] = []
-      await viewVideo(servers[2].url, localVideosServer3[0])
-      await viewVideo(servers[2].url, localVideosServer3[0])
       await viewVideo(servers[2].url, localVideosServer3[0])
-      await viewVideo(servers[2].url, localVideosServer3[1])
-
-      await Promise.all(tasks)
-      await waitJobs(servers)
+      await wait(1000)
 
       await viewVideo(servers[2].url, localVideosServer3[0])
+      await viewVideo(servers[2].url, localVideosServer3[1])
 
-      await waitJobs(servers)
+      await wait(1000)
 
-      await viewVideo(servers[2].url, localVideosServer3[0])
+      await Promise.all([
+        viewVideo(servers[2].url, localVideosServer3[0]),
+        viewVideo(servers[2].url, localVideosServer3[0])
+      ])
 
       await waitJobs(servers)
 
@@ -894,14 +892,14 @@ describe('Test multiple servers', function () {
     it('Should delete the thread comments', async function () {
       this.timeout(10000)
 
-      const res1 = await getVideoCommentThreads(servers[0].url, videoUUID, 0, 5)
-      const threadId = res1.body.data.find(c => c.text === 'my super first comment').id
-      await deleteVideoComment(servers[0].url, servers[0].accessToken, videoUUID, threadId)
+      const res = await getVideoCommentThreads(servers[ 0 ].url, videoUUID, 0, 5)
+      const threadId = res.body.data.find(c => c.text === 'my super first comment').id
+      await deleteVideoComment(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID, threadId)
 
       await waitJobs(servers)
     })
 
-    it('Should have the thread comments deleted on other servers too', async function () {
+    it('Should have the threads deleted on other servers too', async function () {
       for (const server of servers) {
         const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5)
 
@@ -922,6 +920,23 @@ describe('Test multiple servers', function () {
       }
     })
 
+    it('Should delete a remote thread by the origin server', async function () {
+      const res = await getVideoCommentThreads(servers[ 0 ].url, videoUUID, 0, 5)
+      const threadId = res.body.data.find(c => c.text === 'my super second comment').id
+      await deleteVideoComment(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID, threadId)
+
+      await waitJobs(servers)
+    })
+
+    it('Should have the threads deleted on other servers too', async function () {
+      for (const server of servers) {
+        const res = await getVideoCommentThreads(server.url, videoUUID, 0, 5)
+
+        expect(res.body.total).to.equal(0)
+        expect(res.body.data).to.have.lengthOf(0)
+      }
+    })
+
     it('Should disable comments and download', async function () {
       this.timeout(20000)