Remove comment federation by video owner

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()))

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) {
+    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)
+      throw new Error(`Account ${byActor.url} does not own video comment ${videoComment.url} or video ${videoComment.Video.url}`)
     }
 
     await videoComment.destroy({ transaction: t })

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)

server/models/video/video-comment.ts

@@ -105,6 +105,10 @@ enum ScopeNames {
             model: VideoChannelModel.unscoped(),
             required: true,
             include: [
+              {
+                model: ActorModel,
+                required: true
+              },
               {
                 model: AccountModel,
                 required: true,
@@ -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) {

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 wait(1000)
+
       await viewVideo(servers[2].url, localVideosServer3[0])
       await viewVideo(servers[2].url, localVideosServer3[1])
 
-      await Promise.all(tasks)
+      await wait(1000)
-      await waitJobs(servers)
 
-      await viewVideo(servers[2].url, localVideosServer3[0])
+      await Promise.all([
-
+        viewVideo(servers[2].url, localVideosServer3[0]),
-      await waitJobs(servers)
+        viewVideo(servers[2].url, localVideosServer3[0])
-
+      ])
-      await 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 res = await getVideoCommentThreads(servers[ 0 ].url, videoUUID, 0, 5)
-      const threadId = res1.body.data.find(c => c.text === 'my super first comment').id
+      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)