Don't notify on muted instance
authorChocobozzz <me@florianbigard.com>
Thu, 19 Dec 2019 09:35:47 +0000 (10:35 +0100)
committerChocobozzz <me@florianbigard.com>
Thu, 19 Dec 2019 10:34:00 +0000 (11:34 +0100)
server/lib/notifier.ts
server/models/account/account-blocklist.ts
server/models/server/server-blocklist.ts
server/tests/api/users/blocklist.ts

index b7cc2607d3160370f36eb5c29192b87027c7506a..679b9bcf688ec9885e6b4a98bbd65c6976ac0589 100644 (file)
@@ -17,14 +17,16 @@ import {
   MVideoFullLight
 } from '../typings/models/video'
 import {
-  MUser,
+  MUser, MUserAccount,
   MUserDefault,
   MUserNotifSettingAccount,
   MUserWithNotificationSetting,
   UserNotificationModelForApi
 } from '@server/typings/models/user'
-import { MActorFollowFull } from '../typings/models'
+import { MAccountDefault, MActorFollowFull } from '../typings/models'
 import { MVideoImportVideo } from '@server/typings/models/video/video-import'
+import { ServerBlocklistModel } from '@server/models/server/server-blocklist'
+import { getServerActor } from '@server/helpers/utils'
 
 class Notifier {
 
@@ -164,8 +166,7 @@ class Notifier {
     // Not our user or user comments its own video
     if (!user || comment.Account.userId === user.id) return
 
-    const accountMuted = await AccountBlocklistModel.isAccountMutedBy(user.Account.id, comment.accountId)
-    if (accountMuted) return
+    if (await this.isBlockedByServerOrAccount(user, comment.Account)) return
 
     logger.info('Notifying user %s of new comment %s.', user.username, comment.url)
 
@@ -210,12 +211,22 @@ class Notifier {
 
     if (users.length === 0) return
 
-    const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(users.map(u => u.Account.id), comment.accountId)
+    const serverAccountId = (await getServerActor()).Account.id
+    const sourceAccounts = users.map(u => u.Account.id).concat([ serverAccountId ])
+
+    const accountMutedHash = await AccountBlocklistModel.isAccountMutedByMulti(sourceAccounts, comment.accountId)
+    const instanceMutedHash = await ServerBlocklistModel.isServerMutedByMulti(sourceAccounts, comment.Account.Actor.serverId)
 
     logger.info('Notifying %d users of new comment %s.', users.length, comment.url)
 
     function settingGetter (user: MUserNotifSettingAccount) {
-      if (accountMutedHash[user.Account.id] === true) return UserNotificationSettingValue.NONE
+      const accountId = user.Account.id
+      if (
+        accountMutedHash[accountId] === true || instanceMutedHash[accountId] === true ||
+        accountMutedHash[serverAccountId] === true || instanceMutedHash[serverAccountId] === true
+      ) {
+        return UserNotificationSettingValue.NONE
+      }
 
       return user.NotificationSetting.commentMention
     }
@@ -254,9 +265,9 @@ class Notifier {
     if (!user) return
 
     const followerAccount = actorFollow.ActorFollower.Account
+    const followerAccountWithActor = Object.assign(followerAccount, { Actor: actorFollow.ActorFollower })
 
-    const accountMuted = await AccountBlocklistModel.isAccountMutedBy(user.Account.id, followerAccount.id)
-    if (accountMuted) return
+    if (await this.isBlockedByServerOrAccount(user, followerAccountWithActor)) return
 
     logger.info('Notifying user %s of new follower: %s.', user.username, followerAccount.getDisplayName())
 
@@ -572,6 +583,19 @@ class Notifier {
     return value & UserNotificationSettingValue.WEB
   }
 
+  private async isBlockedByServerOrAccount (user: MUserAccount, targetAccount: MAccountDefault) {
+    const serverAccountId = (await getServerActor()).Account.id
+    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 () {
     return this.instance || (this.instance = new this())
   }
index 8bcaca8280162c39f51015506c8fd79a37078e6e..6ebe325564290f18cbb54e43363f299e7de04db9 100644 (file)
@@ -75,11 +75,6 @@ export class AccountBlocklistModel extends Model<AccountBlocklistModel> {
   })
   BlockedAccount: AccountModel
 
-  static isAccountMutedBy (accountId: number, targetAccountId: number) {
-    return AccountBlocklistModel.isAccountMutedByMulti([ accountId ], targetAccountId)
-      .then(result => result[accountId])
-  }
-
   static isAccountMutedByMulti (accountIds: number[], targetAccountId: number) {
     const query = {
       attributes: [ 'accountId', 'id' ],
index 3e96871911f6215b9f613d776056811e58d45a9e..b88df4fd52700d072082f4c1b4037e7d39b1258c 100644 (file)
@@ -5,6 +5,7 @@ import { ServerBlock } from '../../../shared/models/blocklist'
 import { getSort } from '../utils'
 import * as Bluebird from 'bluebird'
 import { MServerBlocklist, MServerBlocklistAccountServer, MServerBlocklistFormattable } from '@server/typings/models'
+import { Op } from 'sequelize'
 
 enum ScopeNames {
   WITH_ACCOUNT = 'WITH_ACCOUNT',
@@ -75,6 +76,31 @@ export class ServerBlocklistModel extends Model<ServerBlocklistModel> {
   })
   BlockedServer: ServerModel
 
+  static isServerMutedByMulti (accountIds: number[], targetServerId: number) {
+    const query = {
+      attributes: [ 'accountId', 'id' ],
+      where: {
+        accountId: {
+          [Op.in]: accountIds // FIXME: sequelize ANY seems broken
+        },
+        targetServerId
+      },
+      raw: true
+    }
+
+    return ServerBlocklistModel.unscoped()
+                                .findAll(query)
+                                .then(rows => {
+                                  const result: { [accountId: number]: boolean } = {}
+
+                                  for (const accountId of accountIds) {
+                                    result[accountId] = !!rows.find(r => r.accountId === accountId)
+                                  }
+
+                                  return result
+                                })
+  }
+
   static loadByAccountAndHost (accountId: number, host: string): Bluebird<MServerBlocklist> {
     const query = {
       where: {
index c25e85ada6039edfbfc08eb684c5649dab3b443d..09e72d068bffc1a528bec7bba682e9a29cfd62d1 100644 (file)
@@ -2,10 +2,10 @@
 
 import * as chai from 'chai'
 import 'mocha'
-import { AccountBlock, ServerBlock, Video } from '../../../../shared/index'
+import { AccountBlock, ServerBlock, UserNotificationType, Video } from '../../../../shared/index'
 import {
   cleanupTests,
-  createUser,
+  createUser, deleteVideoComment,
   doubleFollow,
   flushAndRunMultipleServers,
   flushTests,
@@ -38,6 +38,7 @@ import {
   removeServerFromAccountBlocklist,
   removeServerFromServerBlocklist
 } from '../../../../shared/extra-utils/users/blocklist'
+import { getUserNotifications } from '@shared/extra-utils/users/user-notifications'
 
 const expect = chai.expect
 
@@ -56,9 +57,10 @@ async function checkAllVideos (url: string, token: string) {
 }
 
 async function checkAllComments (url: string, token: string, videoUUID: string) {
-  const resThreads = await getVideoCommentThreads(url, videoUUID, 0, 5, '-createdAt', token)
+  const resThreads = await getVideoCommentThreads(url, videoUUID, 0, 25, '-createdAt', token)
 
-  const threads: VideoComment[] = resThreads.body.data
+  const allThreads: VideoComment[] = resThreads.body.data
+  const threads = allThreads.filter(t => t.isDeleted === false)
   expect(threads).to.have.lengthOf(2)
 
   for (const thread of threads) {
@@ -69,6 +71,28 @@ async function checkAllComments (url: string, token: string, videoUUID: string)
   }
 }
 
+async function checkCommentNotification (
+  mainServer: ServerInfo,
+  comment: { server: ServerInfo, token: string, videoUUID: string, text: string },
+  check: 'presence' | 'absence'
+) {
+  const resComment = await addVideoCommentThread(comment.server.url, comment.token, comment.videoUUID, comment.text)
+  const threadId = resComment.body.comment.id
+
+  await waitJobs([ mainServer, comment.server])
+
+  const res = await getUserNotifications(mainServer.url, mainServer.accessToken, 0, 30)
+  const commentNotifications = res.body.data
+                                  .filter(n => n.comment && n.comment.id === threadId)
+
+  if (check === 'presence') expect(commentNotifications).to.have.lengthOf(1)
+  else expect(commentNotifications).to.have.lengthOf(0)
+
+  await deleteVideoComment(comment.server.url, comment.token, comment.videoUUID, threadId)
+
+  await waitJobs([ mainServer, comment.server])
+}
+
 describe('Test blocklist', function () {
   let servers: ServerInfo[]
   let videoUUID1: string
@@ -189,6 +213,25 @@ describe('Test blocklist', function () {
         }
       })
 
+      it('Should not have notifications from blocked accounts', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 0 ], token: userToken1, videoUUID: videoUUID1, text: 'hidden comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 0 ],
+            token: userToken1,
+            videoUUID: videoUUID2,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+      })
+
       it('Should list all the videos with another user', async function () {
         return checkAllVideos(servers[ 0 ].url, userToken1)
       })
@@ -248,6 +291,25 @@ describe('Test blocklist', function () {
       it('Should display its comments', function () {
         return checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1)
       })
+
+      it('Should have a notification from a non blocked account', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 0 ],
+            token: userToken1,
+            videoUUID: videoUUID2,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+      })
     })
 
     describe('When managing server blocklist', function () {
@@ -280,7 +342,37 @@ describe('Test blocklist', function () {
         return checkAllVideos(servers[ 0 ].url, userToken1)
       })
 
-      it('Should hide its comments')
+      it('Should hide its comments', async function () {
+        this.timeout(10000)
+
+        const resThreads = await addVideoCommentThread(servers[ 1 ].url, userToken2, videoUUID1, 'hidden comment 2')
+        const threadId = resThreads.body.comment.id
+
+        await waitJobs(servers)
+
+        await checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1)
+
+        await deleteVideoComment(servers[ 1 ].url, userToken2, videoUUID1, threadId)
+      })
+
+      it('Should not have notifications from blocked server', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'hidden comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 1 ],
+            token: userToken2,
+            videoUUID: videoUUID1,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+      })
 
       it('Should list blocked servers', async function () {
         const res = await getServerBlocklistByAccount(servers[ 0 ].url, servers[ 0 ].accessToken, 0, 1, 'createdAt')
@@ -305,6 +397,25 @@ describe('Test blocklist', function () {
       it('Should display its comments', function () {
         return checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1)
       })
+
+      it('Should have notification from unblocked server', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 1 ],
+            token: userToken2,
+            videoUUID: videoUUID1,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+      })
     })
   })
 
@@ -375,6 +486,25 @@ describe('Test blocklist', function () {
         }
       })
 
+      it('Should not have notification from blocked accounts by instance', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 0 ], token: userToken1, videoUUID: videoUUID1, text: 'hidden comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 1 ],
+            token: userToken2,
+            videoUUID: videoUUID1,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+      })
+
       it('Should list blocked accounts', async function () {
         {
           const res = await getAccountBlocklistByServer(servers[ 0 ].url, servers[ 0 ].accessToken, 0, 1, 'createdAt')
@@ -430,6 +560,25 @@ describe('Test blocklist', function () {
           await checkAllComments(servers[ 0 ].url, token, videoUUID1)
         }
       })
+
+      it('Should have notifications from unblocked accounts', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 0 ], token: userToken1, videoUUID: videoUUID1, text: 'displayed comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 1 ],
+            token: userToken2,
+            videoUUID: videoUUID1,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+      })
     })
 
     describe('When managing server blocklist', function () {
@@ -467,7 +616,37 @@ describe('Test blocklist', function () {
         }
       })
 
-      it('Should hide its comments')
+      it('Should hide its comments', async function () {
+        this.timeout(10000)
+
+        const resThreads = await addVideoCommentThread(servers[ 1 ].url, userToken2, videoUUID1, 'hidden comment 2')
+        const threadId = resThreads.body.comment.id
+
+        await waitJobs(servers)
+
+        await checkAllComments(servers[ 0 ].url, servers[ 0 ].accessToken, videoUUID1)
+
+        await deleteVideoComment(servers[ 1 ].url, userToken2, videoUUID1, threadId)
+      })
+
+      it('Should not have notification from blocked instances by instance', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'hidden comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 1 ],
+            token: userToken2,
+            videoUUID: videoUUID1,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'absence')
+        }
+      })
 
       it('Should list blocked servers', async function () {
         const res = await getServerBlocklistByServer(servers[ 0 ].url, servers[ 0 ].accessToken, 0, 1, 'createdAt')
@@ -496,6 +675,25 @@ describe('Test blocklist', function () {
           await checkAllComments(servers[ 0 ].url, token, videoUUID1)
         }
       })
+
+      it('Should have notification from unblocked instances', async function () {
+        this.timeout(20000)
+
+        {
+          const comment = { server: servers[ 1 ], token: userToken2, videoUUID: videoUUID1, text: 'displayed comment' }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+
+        {
+          const comment = {
+            server: servers[ 1 ],
+            token: userToken2,
+            videoUUID: videoUUID1,
+            text: 'hello @root@localhost:' + servers[ 0 ].port
+          }
+          await checkCommentNotification(servers[ 0 ], comment, 'presence')
+        }
+      })
     })
   })