Block comments from muted accounts/servers

Add better control for users of comments displayed on their videos:

 * Do not forward comments from muted remote accounts/servers (muted by the current server or by the video owner)
 * Do not list threads and hide replies (with their children) of accounts/servers muted by the video owner
 * Hide from RSS comments of muted accounts/servers by video owners

Use case:
  * Try to limit spam propagation in the federation
  * Add ability for users to automatically hide comments on their videos from undesirable accounts/servers (the comment section belongs to videomakers, so they choose what's posted there)

server/controllers/activitypub/client.ts

@@ -285,7 +285,7 @@ async function videoCommentsController (req: express.Request, res: express.Respo
   const video = res.locals.onlyImmutableVideo
 
   const handler = async (start: number, count: number) => {
-    const result = await VideoCommentModel.listAndCountByVideoId(video.id, start, count)
+    const result = await VideoCommentModel.listAndCountByVideoForAP(video, start, count)
     return {
       total: result.count,
       data: result.rows.map(r => r.url)

server/controllers/api/videos/comment.ts

@@ -78,6 +78,7 @@ async function listVideoThreads (req: express.Request, res: express.Response) {
   if (video.commentsEnabled === true) {
     const apiOptions = await Hooks.wrapObject({
       videoId: video.id,
+      isVideoOwned: video.isOwned(),
       start: req.query.start,
       count: req.query.count,
       sort: req.query.sort,
@@ -108,6 +109,7 @@ async function listVideoThreadComments (req: express.Request, res: express.Respo
   if (video.commentsEnabled === true) {
     const apiOptions = await Hooks.wrapObject({
       videoId: video.id,
+      isVideoOwned: video.isOwned(),
       threadId: res.locals.videoCommentThread.id,
       user
     }, 'filter:api.video-thread-comments.list.params')

server/lib/activitypub/process/process-create.ts

@@ -1,18 +1,19 @@
+import { isRedundancyAccepted } from '@server/lib/redundancy'
 import { ActivityCreate, CacheFileObject, VideoTorrentObject } from '../../../../shared'
+import { PlaylistObject } from '../../../../shared/models/activitypub/objects/playlist-object'
 import { VideoCommentObject } from '../../../../shared/models/activitypub/objects/video-comment-object'
 import { retryTransactionWrapper } from '../../../helpers/database-utils'
 import { logger } from '../../../helpers/logger'
 import { sequelizeTypescript } from '../../../initializers/database'
-import { resolveThread } from '../video-comments'
-import { getOrCreateVideoAndAccountAndChannel } from '../videos'
-import { forwardVideoRelatedActivity } from '../send/utils'
-import { createOrUpdateCacheFile } from '../cache-file'
-import { Notifier } from '../../notifier'
-import { PlaylistObject } from '../../../../shared/models/activitypub/objects/playlist-object'
-import { createOrUpdateVideoPlaylist } from '../playlist'
 import { APProcessorOptions } from '../../../typings/activitypub-processor.model'
 import { MActorSignature, MCommentOwnerVideo, MVideoAccountLightBlacklistAllFiles } from '../../../typings/models'
-import { isRedundancyAccepted } from '@server/lib/redundancy'
+import { Notifier } from '../../notifier'
+import { createOrUpdateCacheFile } from '../cache-file'
+import { createOrUpdateVideoPlaylist } from '../playlist'
+import { forwardVideoRelatedActivity } from '../send/utils'
+import { resolveThread } from '../video-comments'
+import { getOrCreateVideoAndAccountAndChannel } from '../videos'
+import { isBlockedByServerOrAccount } from '@server/lib/blocklist'
 
 async function processCreateActivity (options: APProcessorOptions<ActivityCreate>) {
   const { activity, byActor } = options
@@ -101,6 +102,12 @@ async function processCreateVideoComment (activity: ActivityCreate, byActor: MAc
     return
   }
 
+  // Try to not forward unwanted commments on our videos
+  if (video.isOwned() && await isBlockedByServerOrAccount(comment.Account, video.VideoChannel.Account)) {
+    logger.info('Skip comment forward from blocked account or server %s.', comment.Account.Actor.url)
+    return
+  }
+
   if (video.isOwned() && created === true) {
     // Don't resend the activity to the sender
     const exceptions = [ byActor ]

server/lib/blocklist.ts

@@ -1,5 +1,6 @@
 import { sequelizeTypescript } from '@server/initializers/database'
-import { MAccountBlocklist, MServerBlocklist } from '@server/typings/models'
+import { getServerActor } from '@server/models/application/application'
+import { MAccountBlocklist, MAccountId, MAccountServer, MServerBlocklist } from '@server/typings/models'
 import { AccountBlocklistModel } from '../models/account/account-blocklist'
 import { ServerBlocklistModel } from '../models/server/server-blocklist'
 
@@ -33,9 +34,29 @@ function removeServerFromBlocklist (serverBlock: MServerBlocklist) {
   })
 }
 
+async function isBlockedByServerOrAccount (targetAccount: MAccountServer, userAccount?: MAccountId) {
+  const serverAccountId = (await getServerActor()).Account.id
+  const sourceAccounts = [ serverAccountId ]
+
+  if (userAccount) sourceAccounts.push(userAccount.id)
+
+  const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, targetAccount.id)
+  if (accountMutedHash[serverAccountId] || (userAccount && accountMutedHash[userAccount.id])) {
+    return true
+  }
+
+  const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, targetAccount.Actor.serverId)
+  if (instanceMutedHash[serverAccountId] || (userAccount && instanceMutedHash[userAccount.id])) {
+    return true
+  }
+
+  return false
+}
+
 export {
   addAccountInBlocklist,
   addServerInBlocklist,
   removeAccountFromBlocklist,
-  removeServerFromBlocklist
+  removeServerFromBlocklist,
+  isBlockedByServerOrAccount
 }

server/lib/notifier.ts

@@ -1,20 +1,5 @@
-import { UserNotificationSettingValue, UserNotificationType, UserRight } from '../../shared/models/users'
+import { getServerActor } from '@server/models/application/application'
-import { logger } from '../helpers/logger'
+import { ServerBlocklistModel } from '@server/models/server/server-blocklist'
-import { Emailer } from './emailer'
-import { UserNotificationModel } from '../models/account/user-notification'
-import { UserModel } from '../models/account/user'
-import { PeerTubeSocket } from './peertube-socket'
-import { CONFIG } from '../initializers/config'
-import { VideoPrivacy, VideoState, VideoAbuse } from '../../shared/models/videos'
-import { AccountBlocklistModel } from '../models/account/account-blocklist'
-import {
-  MCommentOwnerVideo,
-  MVideoAbuseVideo,
-  MVideoAccountLight,
-  MVideoBlacklistLightVideo,
-  MVideoBlacklistVideo,
-  MVideoFullLight
-} from '../typings/models/video'
 import {
   MUser,
   MUserAccount,
@@ -23,10 +8,26 @@ import {
   MUserWithNotificationSetting,
   UserNotificationModelForApi
 } from '@server/typings/models/user'
-import { MAccountDefault, MActorFollowFull } from '../typings/models'
 import { MVideoImportVideo } from '@server/typings/models/video/video-import'
-import { ServerBlocklistModel } from '@server/models/server/server-blocklist'
+import { UserNotificationSettingValue, UserNotificationType, UserRight } from '../../shared/models/users'
-import { getServerActor } from '@server/models/application/application'
+import { VideoAbuse, VideoPrivacy, VideoState } from '../../shared/models/videos'
+import { logger } from '../helpers/logger'
+import { CONFIG } from '../initializers/config'
+import { AccountBlocklistModel } from '../models/account/account-blocklist'
+import { UserModel } from '../models/account/user'
+import { UserNotificationModel } from '../models/account/user-notification'
+import { MAccountServer, MActorFollowFull } from '../typings/models'
+import {
+  MCommentOwnerVideo,
+  MVideoAbuseVideo,
+  MVideoAccountLight,
+  MVideoBlacklistLightVideo,
+  MVideoBlacklistVideo,
+  MVideoFullLight
+} from '../typings/models/video'
+import { isBlockedByServerOrAccount } from './blocklist'
+import { Emailer } from './emailer'
+import { PeerTubeSocket } from './peertube-socket'
 
 class Notifier {
 
@@ -169,7 +170,7 @@ class Notifier {
     // Not our user or user comments its own video
     if (!user || comment.Account.userId === user.id) return
 
-    if (await this.isBlockedByServerOrAccount(user, comment.Account)) return
+    if (await this.isBlockedByServerOrUser(comment.Account, user)) return
 
     logger.info('Notifying user %s of new comment %s.', user.username, comment.url)
 
@@ -270,7 +271,7 @@ class Notifier {
     const followerAccount = actorFollow.ActorFollower.Account
     const followerAccountWithActor = Object.assign(followerAccount, { Actor: actorFollow.ActorFollower })
 
-    if (await this.isBlockedByServerOrAccount(user, followerAccountWithActor)) return
+    if (await this.isBlockedByServerOrUser(followerAccountWithActor, user)) return
 
     logger.info('Notifying user %s of new follower: %s.', user.username, followerAccount.getDisplayName())
 
@@ -299,6 +300,9 @@ class Notifier {
   private async notifyAdminsOfNewInstanceFollow (actorFollow: MActorFollowFull) {
     const admins = await UserModel.listWithRight(UserRight.MANAGE_SERVER_FOLLOW)
 
+    const follower = Object.assign(actorFollow.ActorFollower.Account, { Actor: actorFollow.ActorFollower })
+    if (await this.isBlockedByServerOrUser(follower)) return
+
     logger.info('Notifying %d administrators of new instance follower: %s.', admins.length, actorFollow.ActorFollower.url)
 
     function settingGetter (user: MUserWithNotificationSetting) {
@@ -590,17 +594,8 @@ class Notifier {
     return value & UserNotificationSettingValue.WEB
   }
 
-  private async isBlockedByServerOrAccount (user: MUserAccount, targetAccount: MAccountDefault) {
+  private isBlockedByServerOrUser (targetAccount: MAccountServer, user?: MUserAccount) {
-    const serverAccountId = (await getServerActor()).Account.id
+    return isBlockedByServerOrAccount(targetAccount, user?.Account)
-    const sourceAccounts = [ serverAccountId, user.Account.id ]
-
-    const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, targetAccount.id)
-    if (accountMutedHash[serverAccountId] || accountMutedHash[user.Account.id]) return true
-
-    const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, targetAccount.Actor.serverId)
-    if (instanceMutedHash[serverAccountId] || instanceMutedHash[user.Account.id]) return true
-
-    return false
   }
 
   static get Instance () {

server/models/account/account.ts

@@ -32,9 +32,10 @@ import { FindOptions, IncludeOptions, Op, Transaction, WhereOptions } from 'sequ
 import { AccountBlocklistModel } from './account-blocklist'
 import { ServerBlocklistModel } from '../server/server-blocklist'
 import { ActorFollowModel } from '../activitypub/actor-follow'
-import { MAccountActor, MAccountAP, MAccountDefault, MAccountFormattable, MAccountSummaryFormattable } from '../../typings/models'
+import { MAccountActor, MAccountAP, MAccountDefault, MAccountFormattable, MAccountSummaryFormattable, MAccount } from '../../typings/models'
 import * as Bluebird from 'bluebird'
 import { ModelCache } from '@server/models/model-cache'
+import { VideoModel } from '../video/video'
 
 export enum ScopeNames {
   SUMMARY = 'SUMMARY'
@@ -343,6 +344,29 @@ export class AccountModel extends Model<AccountModel> {
       })
   }
 
+  static loadAccountIdFromVideo (videoId: number): Bluebird<MAccount> {
+    const query = {
+      include: [
+        {
+          attributes: [ 'id', 'accountId' ],
+          model: VideoChannelModel.unscoped(),
+          required: true,
+          include: [
+            {
+              attributes: [ 'id', 'channelId' ],
+              model: VideoModel.unscoped(),
+              where: {
+                id: videoId
+              }
+            }
+          ]
+        }
+      ]
+    }
+
+    return AccountModel.findOne(query)
+  }
+
   static listLocalsForSitemap (sort: string): Bluebird<MAccountActor[]> {
     const query = {
       attributes: [ ],

server/models/utils.ts

@@ -136,10 +136,7 @@ function createSimilarityAttribute (col: string, value: string) {
   )
 }
 
-function buildBlockedAccountSQL (serverAccountId: number, userAccountId?: number) {
+function buildBlockedAccountSQL (blockerIds: number[]) {
-  const blockerIds = [ serverAccountId ]
-  if (userAccountId) blockerIds.push(userAccountId)
-
   const blockerIdsString = blockerIds.join(', ')
 
   return 'SELECT "targetAccountId" AS "id" FROM "accountBlocklist" WHERE "accountId" IN (' + blockerIdsString + ')' +

server/models/video/video-abuse.ts

@@ -57,7 +57,7 @@ export enum ScopeNames {
   }) => {
     const where = {
       reporterAccountId: {
-        [Op.notIn]: literal('(' + buildBlockedAccountSQL(options.serverAccountId, options.userAccountId) + ')')
+        [Op.notIn]: literal('(' + buildBlockedAccountSQL([ options.serverAccountId, options.userAccountId ]) + ')')
       }
     }
 

server/models/video/video-comment.ts

@@ -21,7 +21,8 @@ import {
   MCommentOwnerReplyVideoLight,
   MCommentOwnerVideo,
   MCommentOwnerVideoFeed,
-  MCommentOwnerVideoReply
+  MCommentOwnerVideoReply,
+  MVideoImmutable
 } from '../../typings/models/video'
 import { AccountModel } from '../account/account'
 import { ActorModel, unusedActorAttributesForAPI } from '../activitypub/actor'
@@ -38,14 +39,14 @@ enum ScopeNames {
 }
 
 @Scopes(() => ({
-  [ScopeNames.ATTRIBUTES_FOR_API]: (serverAccountId: number, userAccountId?: number) => {
+  [ScopeNames.ATTRIBUTES_FOR_API]: (blockerAccountIds: number[]) => {
     return {
       attributes: {
         include: [
           [
             Sequelize.literal(
               '(' +
-                'WITH "blocklist" AS (' + buildBlockedAccountSQL(serverAccountId, userAccountId) + ')' +
+                'WITH "blocklist" AS (' + buildBlockedAccountSQL(blockerAccountIds) + ')' +
                 'SELECT COUNT("replies"."id") - (' +
                   'SELECT COUNT("replies"."id") ' +
                   'FROM "videoComment" AS "replies" ' +
@@ -276,16 +277,15 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
 
   static async listThreadsForApi (parameters: {
     videoId: number
+    isVideoOwned: boolean
     start: number
     count: number
     sort: string
     user?: MUserAccountId
   }) {
-    const { videoId, start, count, sort, user } = parameters
+    const { videoId, isVideoOwned, start, count, sort, user } = parameters
 
-    const serverActor = await getServerActor()
+    const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned })
-    const serverAccountId = serverActor.Account.id
-    const userAccountId = user ? user.Account.id : undefined
 
     const query = {
       offset: start,
@@ -304,7 +304,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
               {
                 accountId: {
                   [Op.notIn]: Sequelize.literal(
-                    '(' + buildBlockedAccountSQL(serverAccountId, userAccountId) + ')'
+                    '(' + buildBlockedAccountSQL(blockerAccountIds) + ')'
                   )
                 }
               },
@@ -320,7 +320,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
     const scopes: (string | ScopeOptions)[] = [
       ScopeNames.WITH_ACCOUNT_FOR_API,
       {
-        method: [ ScopeNames.ATTRIBUTES_FOR_API, serverAccountId, userAccountId ]
+        method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ]
       }
     ]
 
@@ -334,14 +334,13 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
 
   static async listThreadCommentsForApi (parameters: {
     videoId: number
+    isVideoOwned: boolean
     threadId: number
     user?: MUserAccountId
   }) {
-    const { videoId, threadId, user } = parameters
+    const { videoId, threadId, user, isVideoOwned } = parameters
 
-    const serverActor = await getServerActor()
+    const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({ videoId, user, isVideoOwned })
-    const serverAccountId = serverActor.Account.id
-    const userAccountId = user ? user.Account.id : undefined
 
     const query = {
       order: [ [ 'createdAt', 'ASC' ], [ 'updatedAt', 'ASC' ] ] as Order,
@@ -353,7 +352,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
         ],
         accountId: {
           [Op.notIn]: Sequelize.literal(
-            '(' + buildBlockedAccountSQL(serverAccountId, userAccountId) + ')'
+            '(' + buildBlockedAccountSQL(blockerAccountIds) + ')'
           )
         }
       }
@@ -362,7 +361,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
     const scopes: any[] = [
       ScopeNames.WITH_ACCOUNT_FOR_API,
       {
-        method: [ ScopeNames.ATTRIBUTES_FOR_API, serverAccountId, userAccountId ]
+        method: [ ScopeNames.ATTRIBUTES_FOR_API, blockerAccountIds ]
       }
     ]
 
@@ -399,13 +398,23 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
       .findAll(query)
   }
 
-  static listAndCountByVideoId (videoId: number, start: number, count: number, t?: Transaction, order: 'ASC' | 'DESC' = 'ASC') {
+  static async listAndCountByVideoForAP (video: MVideoImmutable, start: number, count: number, t?: Transaction) {
+    const blockerAccountIds = await VideoCommentModel.buildBlockerAccountIds({
+      videoId: video.id,
+      isVideoOwned: video.isOwned()
+    })
+
     const query = {
-      order: [ [ 'createdAt', order ] ] as Order,
+      order: [ [ 'createdAt', 'ASC' ] ] as Order,
       offset: start,
       limit: count,
       where: {
-        videoId
+        videoId: video.id,
+        accountId: {
+          [Op.notIn]: Sequelize.literal(
+            '(' + buildBlockedAccountSQL(blockerAccountIds) + ')'
+          )
+        }
       },
       transaction: t
     }
@@ -424,7 +433,7 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
         deletedAt: null,
         accountId: {
           [Op.notIn]: Sequelize.literal(
-            '(' + buildBlockedAccountSQL(serverActor.Account.id) + ')'
+            '(' + buildBlockedAccountSQL([ serverActor.Account.id, '"Video->VideoChannel"."accountId"' ]) + ')'
           )
         }
       },
@@ -435,7 +444,14 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
           required: true,
           where: {
             privacy: VideoPrivacy.PUBLIC
+          },
+          include: [
+            {
+              attributes: [ 'accountId' ],
+              model: VideoChannelModel.unscoped(),
+              required: true
             }
+          ]
         }
       ]
     }
@@ -650,4 +666,24 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
       tag
     }
   }
+
+  private static async buildBlockerAccountIds (options: {
+    videoId: number
+    isVideoOwned: boolean
+    user?: MUserAccountId
+  }) {
+    const { videoId, user, isVideoOwned } = options
+
+    const serverActor = await getServerActor()
+    const blockerAccountIds = [ serverActor.Account.id ]
+
+    if (user) blockerAccountIds.push(user.Account.id)
+
+    if (isVideoOwned) {
+      const videoOwnerAccount = await AccountModel.loadAccountIdFromVideo(videoId)
+      blockerAccountIds.push(videoOwnerAccount.id)
+    }
+
+    return blockerAccountIds
+  }
 }

server/tests/api/users/blocklist.ts

@@ -2,7 +2,7 @@
 
 import * as chai from 'chai'
 import 'mocha'
-import { AccountBlock, ServerBlock, Video } from '../../../../shared/index'
+import { AccountBlock, ServerBlock, Video, UserNotification, UserNotificationType } from '../../../../shared/index'
 import {
   cleanupTests,
   createUser,
@@ -11,7 +11,9 @@ import {
   flushAndRunMultipleServers,
   ServerInfo,
   uploadVideo,
-  userLogin
+  userLogin,
+  follow,
+  unfollow
 } from '../../../../shared/extra-utils/index'
 import { setAccessTokensToServers } from '../../../../shared/extra-utils/users/login'
 import { getVideosList, getVideosListWithToken } from '../../../../shared/extra-utils/videos/videos'
@@ -19,7 +21,8 @@ import {
   addVideoCommentReply,
   addVideoCommentThread,
   getVideoCommentThreads,
-  getVideoThreadComments
+  getVideoThreadComments,
+  findCommentId
 } from '../../../../shared/extra-utils/videos/video-comments'
 import { waitJobs } from '../../../../shared/extra-utils/server/jobs'
 import { VideoComment, VideoCommentThreadTree } from '../../../../shared/models/videos/video-comment.model'
@@ -45,13 +48,13 @@ async function checkAllVideos (url: string, token: string) {
   {
     const res = await getVideosListWithToken(url, token)
 
-    expect(res.body.data).to.have.lengthOf(4)
+    expect(res.body.data).to.have.lengthOf(5)
   }
 
   {
     const res = await getVideosList(url)
 
-    expect(res.body.data).to.have.lengthOf(4)
+    expect(res.body.data).to.have.lengthOf(5)
   }
 }
 
@@ -76,13 +79,15 @@ async function checkCommentNotification (
   check: 'presence' | 'absence'
 ) {
   const resComment = await addVideoCommentThread(comment.server.url, comment.token, comment.videoUUID, comment.text)
-  const threadId = resComment.body.comment.id
+  const created = resComment.body.comment as VideoComment
+  const threadId = created.id
+  const createdAt = created.createdAt
 
   await waitJobs([ mainServer, comment.server ])
 
   const res = await getUserNotifications(mainServer.url, mainServer.accessToken, 0, 30)
-  const commentNotifications = res.body.data
+  const commentNotifications = (res.body.data as UserNotification[])
-                                  .filter(n => n.comment && n.comment.id === threadId)
+                                  .filter(n => n.comment && n.comment.video.uuid === comment.videoUUID && n.createdAt >= createdAt)
 
   if (check === 'presence') expect(commentNotifications).to.have.lengthOf(1)
   else expect(commentNotifications).to.have.lengthOf(0)
@@ -96,6 +101,7 @@ describe('Test blocklist', function () {
   let servers: ServerInfo[]
   let videoUUID1: string
   let videoUUID2: string
+  let videoUUID3: string
   let userToken1: string
   let userModeratorToken: string
   let userToken2: string
@@ -103,7 +109,7 @@ describe('Test blocklist', function () {
   before(async function () {
     this.timeout(60000)
 
-    servers = await flushAndRunMultipleServers(2)
+    servers = await flushAndRunMultipleServers(3)
     await setAccessTokensToServers(servers)
 
     {
@@ -139,7 +145,13 @@ describe('Test blocklist', function () {
       videoUUID2 = res.body.video.uuid
     }
 
+    {
+      const res = await uploadVideo(servers[0].url, servers[0].accessToken, { name: 'video 2 server 1' })
+      videoUUID3 = res.body.video.uuid
+    }
+
     await doubleFollow(servers[0], servers[1])
+    await doubleFollow(servers[0], servers[2])
 
     {
       const resComment = await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID1, 'comment root 1')
@@ -174,7 +186,7 @@ describe('Test blocklist', function () {
         const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken)
 
         const videos: Video[] = res.body.data
-        expect(videos).to.have.lengthOf(3)
+        expect(videos).to.have.lengthOf(4)
 
         const v = videos.find(v => v.name === 'video user 2')
         expect(v).to.be.undefined
@@ -188,7 +200,7 @@ describe('Test blocklist', function () {
         const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken)
 
         const videos: Video[] = res.body.data
-        expect(videos).to.have.lengthOf(2)
+        expect(videos).to.have.lengthOf(3)
 
         const v = videos.find(v => v.name === 'video user 1')
         expect(v).to.be.undefined
@@ -235,10 +247,6 @@ describe('Test blocklist', function () {
         return checkAllVideos(servers[0].url, userToken1)
       })
 
-      it('Should list all the comments with another user', async function () {
-        return checkAllComments(servers[0].url, userToken1, videoUUID1)
-      })
-
       it('Should list blocked accounts', async function () {
         {
           const res = await getAccountBlocklistByAccount(servers[0].url, servers[0].accessToken, 0, 1, 'createdAt')
@@ -269,6 +277,61 @@ describe('Test blocklist', function () {
         }
       })
 
+      it('Should not allow a remote blocked user to comment my videos', async function () {
+        this.timeout(60000)
+
+        {
+          await addVideoCommentThread(servers[1].url, userToken2, videoUUID3, 'comment user 2')
+          await waitJobs(servers)
+
+          await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID3, 'uploader')
+          await waitJobs(servers)
+
+          const commentId = await findCommentId(servers[1].url, videoUUID3, 'uploader')
+          const message = 'reply by user 2'
+          const resReply = await addVideoCommentReply(servers[1].url, userToken2, videoUUID3, commentId, message)
+          await addVideoCommentReply(servers[1].url, servers[1].accessToken, videoUUID3, resReply.body.comment.id, 'another reply')
+
+          await waitJobs(servers)
+        }
+
+        // Server 2 has all the comments
+        {
+          const resThreads = await getVideoCommentThreads(servers[1].url, videoUUID3, 0, 25, '-createdAt')
+          const threads: VideoComment[] = resThreads.body.data
+
+          expect(threads).to.have.lengthOf(2)
+          expect(threads[0].text).to.equal('uploader')
+          expect(threads[1].text).to.equal('comment user 2')
+
+          const resReplies = await getVideoThreadComments(servers[1].url, videoUUID3, threads[0].id)
+
+          const tree: VideoCommentThreadTree = resReplies.body
+          expect(tree.children).to.have.lengthOf(1)
+          expect(tree.children[0].comment.text).to.equal('reply by user 2')
+          expect(tree.children[0].children).to.have.lengthOf(1)
+          expect(tree.children[0].children[0].comment.text).to.equal('another reply')
+        }
+
+        // Server 1 and 3 should only have uploader comments
+        for (const server of [ servers[0], servers[2] ]) {
+          const resThreads = await getVideoCommentThreads(server.url, videoUUID3, 0, 25, '-createdAt')
+          const threads: VideoComment[] = resThreads.body.data
+
+          expect(threads).to.have.lengthOf(1)
+          expect(threads[0].text).to.equal('uploader')
+
+          const resReplies = await getVideoThreadComments(server.url, videoUUID3, threads[0].id)
+
+          const tree: VideoCommentThreadTree = resReplies.body
+          if (server.serverNumber === 1) {
+            expect(tree.children).to.have.lengthOf(0)
+          } else {
+            expect(tree.children).to.have.lengthOf(1)
+          }
+        }
+      })
+
       it('Should unblock the remote account', async function () {
         await removeAccountFromAccountBlocklist(servers[0].url, servers[0].accessToken, 'user2@localhost:' + servers[1].port)
       })
@@ -277,12 +340,37 @@ describe('Test blocklist', function () {
         const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken)
 
         const videos: Video[] = res.body.data
-        expect(videos).to.have.lengthOf(3)
+        expect(videos).to.have.lengthOf(4)
 
         const v = videos.find(v => v.name === 'video user 2')
         expect(v).not.to.be.undefined
       })
 
+      it('Should display its comments on my video', async function () {
+        for (const server of servers) {
+          const resThreads = await getVideoCommentThreads(server.url, videoUUID3, 0, 25, '-createdAt')
+          const threads: VideoComment[] = resThreads.body.data
+
+          // Server 3 should not have 2 comment threads, because server 1 did not forward the server 2 comment
+          if (server.serverNumber === 3) {
+            expect(threads).to.have.lengthOf(1)
+            continue
+          }
+
+          expect(threads).to.have.lengthOf(2)
+          expect(threads[0].text).to.equal('uploader')
+          expect(threads[1].text).to.equal('comment user 2')
+
+          const resReplies = await getVideoThreadComments(server.url, videoUUID3, threads[0].id)
+
+          const tree: VideoCommentThreadTree = resReplies.body
+          expect(tree.children).to.have.lengthOf(1)
+          expect(tree.children[0].comment.text).to.equal('reply by user 2')
+          expect(tree.children[0].children).to.have.lengthOf(1)
+          expect(tree.children[0].children[0].comment.text).to.equal('another reply')
+        }
+      })
+
       it('Should unblock the local account', async function () {
         await removeAccountFromAccountBlocklist(servers[0].url, servers[0].accessToken, 'user1')
       })
@@ -328,7 +416,7 @@ describe('Test blocklist', function () {
         const res = await getVideosListWithToken(servers[0].url, servers[0].accessToken)
 
         const videos: Video[] = res.body.data
-        expect(videos).to.have.lengthOf(2)
+        expect(videos).to.have.lengthOf(3)
 
         const v1 = videos.find(v => v.name === 'video user 2')
         const v2 = videos.find(v => v.name === 'video server 2')
@@ -442,7 +530,7 @@ describe('Test blocklist', function () {
           const res = await getVideosListWithToken(servers[0].url, token)
 
           const videos: Video[] = res.body.data
-          expect(videos).to.have.lengthOf(3)
+          expect(videos).to.have.lengthOf(4)
 
           const v = videos.find(v => v.name === 'video user 2')
           expect(v).to.be.undefined
@@ -458,7 +546,7 @@ describe('Test blocklist', function () {
           const res = await getVideosListWithToken(servers[0].url, token)
 
           const videos: Video[] = res.body.data
-          expect(videos).to.have.lengthOf(2)
+          expect(videos).to.have.lengthOf(3)
 
           const v = videos.find(v => v.name === 'video user 1')
           expect(v).to.be.undefined
@@ -545,7 +633,7 @@ describe('Test blocklist', function () {
           const res = await getVideosListWithToken(servers[0].url, token)
 
           const videos: Video[] = res.body.data
-          expect(videos).to.have.lengthOf(3)
+          expect(videos).to.have.lengthOf(4)
 
           const v = videos.find(v => v.name === 'video user 2')
           expect(v).not.to.be.undefined
@@ -606,7 +694,7 @@ describe('Test blocklist', function () {
 
           for (const res of [ res1, res2 ]) {
             const videos: Video[] = res.body.data
-            expect(videos).to.have.lengthOf(2)
+            expect(videos).to.have.lengthOf(3)
 
             const v1 = videos.find(v => v.name === 'video user 2')
             const v2 = videos.find(v => v.name === 'video server 2')
@@ -631,7 +719,7 @@ describe('Test blocklist', function () {
       })
 
       it('Should not have notification from blocked instances by instance', async function () {
-        this.timeout(20000)
+        this.timeout(50000)
 
         {
           const comment = { server: servers[1], token: userToken2, videoUUID: videoUUID1, text: 'hidden comment' }
@@ -647,6 +735,24 @@ describe('Test blocklist', function () {
           }
           await checkCommentNotification(servers[0], comment, 'absence')
         }
+
+        {
+          const now = new Date()
+          await unfollow(servers[1].url, servers[1].accessToken, servers[0])
+          await waitJobs(servers)
+          await follow(servers[1].url, [ servers[0].host ], servers[1].accessToken)
+
+          await waitJobs(servers)
+
+          const res = await getUserNotifications(servers[0].url, servers[0].accessToken, 0, 30)
+          const commentNotifications = (res.body.data as UserNotification[])
+                                          .filter(n => {
+                                            return n.type === UserNotificationType.NEW_INSTANCE_FOLLOWER &&
+                                            n.createdAt >= now.toISOString()
+                                          })
+
+          expect(commentNotifications).to.have.lengthOf(0)
+        }
       })
 
       it('Should list blocked servers', async function () {
@@ -678,7 +784,7 @@ describe('Test blocklist', function () {
       })
 
       it('Should have notification from unblocked instances', async function () {
-        this.timeout(20000)
+        this.timeout(50000)
 
         {
           const comment = { server: servers[1], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' }
@@ -694,6 +800,24 @@ describe('Test blocklist', function () {
           }
           await checkCommentNotification(servers[0], comment, 'presence')
         }
+
+        {
+          const now = new Date()
+          await unfollow(servers[1].url, servers[1].accessToken, servers[0])
+          await waitJobs(servers)
+          await follow(servers[1].url, [ servers[0].host ], servers[1].accessToken)
+
+          await waitJobs(servers)
+
+          const res = await getUserNotifications(servers[0].url, servers[0].accessToken, 0, 30)
+          const commentNotifications = (res.body.data as UserNotification[])
+                                          .filter(n => {
+                                            return n.type === UserNotificationType.NEW_INSTANCE_FOLLOWER &&
+                                            n.createdAt >= now.toISOString()
+                                          })
+
+          expect(commentNotifications).to.have.lengthOf(1)
+        }
       })
     })
   })

server/tests/api/videos/multiple-servers.ts

@@ -37,7 +37,8 @@ import {
   addVideoCommentThread,
   deleteVideoComment,
   getVideoCommentThreads,
-  getVideoThreadComments
+  getVideoThreadComments,
+  findCommentId
 } from '../../../../shared/extra-utils/videos/video-comments'
 import { waitJobs } from '../../../../shared/extra-utils/server/jobs'
 
@@ -773,8 +774,7 @@ describe('Test multiple servers', function () {
       await waitJobs(servers)
 
       {
-        const res = await getVideoCommentThreads(servers[1].url, videoUUID, 0, 5)
+        const threadId = await findCommentId(servers[1].url, videoUUID, 'my super first comment')
-        const threadId = res.body.data.find(c => c.text === 'my super first comment').id
 
         const text = 'my super answer to thread 1'
         await addVideoCommentReply(servers[1].url, servers[1].accessToken, videoUUID, threadId, text)
@@ -783,8 +783,7 @@ describe('Test multiple servers', function () {
       await waitJobs(servers)
 
       {
-        const res1 = await getVideoCommentThreads(servers[2].url, videoUUID, 0, 5)
+        const threadId = await findCommentId(servers[2].url, videoUUID, 'my super first comment')
-        const threadId = res1.body.data.find(c => c.text === 'my super first comment').id
 
         const res2 = await getVideoThreadComments(servers[2].url, videoUUID, threadId)
         const childCommentId = res2.body.children[0].comment.id

server/tests/feeds/feeds.ts

@@ -1,7 +1,14 @@
 /* eslint-disable @typescript-eslint/no-unused-expressions,@typescript-eslint/require-await */
 
-import * as chai from 'chai'
 import 'mocha'
+import * as chai from 'chai'
+import * as libxmljs from 'libxmljs'
+import {
+  addAccountToAccountBlocklist,
+  addAccountToServerBlocklist,
+  removeAccountFromServerBlocklist
+} from '@shared/extra-utils/users/blocklist'
+import { VideoPrivacy } from '@shared/models'
 import {
   cleanupTests,
   createUser,
@@ -13,14 +20,12 @@ import {
   ServerInfo,
   setAccessTokensToServers,
   uploadVideo,
+  uploadVideoAndGetId,
   userLogin
 } from '../../../shared/extra-utils'
-import * as libxmljs from 'libxmljs'
-import { addVideoCommentThread } from '../../../shared/extra-utils/videos/video-comments'
 import { waitJobs } from '../../../shared/extra-utils/server/jobs'
+import { addVideoCommentThread } from '../../../shared/extra-utils/videos/video-comments'
 import { User } from '../../../shared/models/users'
-import { VideoPrivacy } from '@shared/models'
-import { addAccountToServerBlocklist } from '@shared/extra-utils/users/blocklist'
 
 chai.use(require('chai-xml'))
 chai.use(require('chai-json-schema'))
@@ -219,7 +224,11 @@ describe('Test syndication feeds', () => {
     })
 
     it('Should not list comments from muted accounts or instances', async function () {
-      await addAccountToServerBlocklist(servers[1].url, servers[1].accessToken, 'root@localhost:' + servers[0].port)
+      this.timeout(30000)
+
+      const remoteHandle = 'root@localhost:' + servers[0].port
+
+      await addAccountToServerBlocklist(servers[1].url, servers[1].accessToken, remoteHandle)
 
       {
         const json = await getJSONfeed(servers[1].url, 'video-comments', { version: 2 })
@@ -227,6 +236,26 @@ describe('Test syndication feeds', () => {
         expect(jsonObj.items.length).to.be.equal(0)
       }
 
+      await removeAccountFromServerBlocklist(servers[1].url, servers[1].accessToken, remoteHandle)
+
+      {
+        const videoUUID = (await uploadVideoAndGetId({ server: servers[1], videoName: 'server 2' })).uuid
+        await waitJobs(servers)
+        await addVideoCommentThread(servers[0].url, servers[0].accessToken, videoUUID, 'super comment')
+        await waitJobs(servers)
+
+        const json = await getJSONfeed(servers[1].url, 'video-comments', { version: 3 })
+        const jsonObj = JSON.parse(json.text)
+        expect(jsonObj.items.length).to.be.equal(3)
+      }
+
+      await addAccountToAccountBlocklist(servers[1].url, servers[1].accessToken, remoteHandle)
+
+      {
+        const json = await getJSONfeed(servers[1].url, 'video-comments', { version: 4 })
+        const jsonObj = JSON.parse(json.text)
+        expect(jsonObj.items.length).to.be.equal(2)
+      }
     })
   })
 

shared/extra-utils/videos/video-comments.ts

@@ -61,6 +61,12 @@ function addVideoCommentReply (
     .expect(expectedStatus)
 }
 
+async function findCommentId (url: string, videoId: number | string, text: string) {
+  const res = await getVideoCommentThreads(url, videoId, 0, 25, '-createdAt')
+
+  return res.body.data.find(c => c.text === text).id as number
+}
+
 function deleteVideoComment (
   url: string,
   token: string,
@@ -85,5 +91,6 @@ export {
   getVideoThreadComments,
   addVideoCommentThread,
   addVideoCommentReply,
+  findCommentId,
   deleteVideoComment
 }

shared/extra-utils/videos/videos.ts

@@ -95,6 +95,12 @@ function getVideo (url: string, id: number | string, expectedStatus = 200) {
           .expect(expectedStatus)
 }
 
+async function getVideoIdFromUUID (url: string, uuid: string) {
+  const res = await getVideo(url, uuid)
+
+  return res.body.id
+}
+
 function getVideoFileMetadataUrl (url: string) {
   return request(url)
     .get('/')
@@ -669,5 +675,6 @@ export {
   checkVideoFilesWereRemoved,
   getPlaylistVideos,
   uploadVideoAndGetId,
-  getLocalIdByUUID
+  getLocalIdByUUID,
+  getVideoIdFromUUID
 }