Merge pull request 'fix(safety): team notifications skip members who block the author' (#234) from fix/safety-team-notifications into main

Reviewed-on: #234

server/src/model/safety/userSafety.db.js

@@ -86,6 +86,19 @@ async function convertToAccountBlock(id, blockedUserId) {
   }
 }
 
+/**
+ * Everyone who has blocked this account, as account blocks. The reverse of
+ * `blocksFor`, for the notification fan-out: a post goes out to a Team, and the
+ * members who block its author are the ones it must skip.
+ */
+async function blockersOf(blockedUserId) {
+  const rows = await query(
+    'SELECT blocker_user_id FROM user_blocks WHERE blocked_user_id = ?',
+    [blockedUserId],
+  )
+  return rows.map((r) => Number(r.blocker_user_id))
+}
+
 /** Undo one of this member's blocks. Scoped to the blocker, so an id is not a capability. */
 async function deleteBlock(blockerUserId, id) {
   const res = await query('DELETE FROM user_blocks WHERE id = ? AND blocker_user_id = ?', [id, blockerUserId])
@@ -152,6 +165,7 @@ module.exports = {
   insertAccountBlock,
   insertGameBlock,
   convertToAccountBlock,
+  blockersOf,
   deleteBlock,
   staffAmong,
   userById,

server/src/model/safety/userSafety.model.js

@@ -280,6 +280,25 @@ async function forModuleViewer(moduleId, userId) {
   return out
 }
 
+/**
+ * The members whose blocks cover this author, for a notification about something
+ * the author wrote: a forum post is pushed, mailed and put in the inbox of every
+ * member of its Team, and a member who blocks the author must not be told about it
+ * (the forum itself already leaves the post out for them).
+ *
+ * S8 applies here too: a staff author is blocked by nobody, so this answers empty
+ * and the post reaches everyone. Answers account blocks only. A game block on the
+ * author's Steam id that S14 has not yet rewritten counts from the next time its
+ * owner reads the forum or their blocks, which is when the rewrite happens.
+ */
+async function blockersOf(authorUserId) {
+  const id = Number(authorUserId)
+  if (!Number.isInteger(id) || id <= 0) return []
+  const author = await db.userById(id)
+  if (!author || isStaffRole(author.role)) return []
+  return db.blockersOf(id)
+}
+
 /** `ctx.safety.staffAmong` (S13): which of these accounts are staff. */
 async function staffAmong(userIds) {
   const ids = [...new Set((Array.isArray(userIds) ? userIds : [])
@@ -301,6 +320,7 @@ module.exports = {
   clearHidden,
   forumFilter,
   forModuleViewer,
+  blockersOf,
   staffAmong,
   publicBlock,
 }

server/src/model/teams/teamForum.model.js

@@ -318,7 +318,14 @@ async function createPost({ team, threadId, actor, body }) {
   // what a recipient needs to know is which conversation moved. `type` is always
   // 'discussion' here (an announcement takes no replies) and is carried anyway so
   // the controller has one shape to hand the fan-out from both routes.
-  return { ok: true, threadId, postId, notify: { threadId, title: thread.title, type: thread.type, bodyHtml: cleaned } }
+  return {
+    ok: true,
+    threadId,
+    postId,
+    // `threadAuthorUserId`: a member who blocks whoever started the thread cannot
+    // open it (leftOutThread), so they are not told about replies in it either.
+    notify: { threadId, title: thread.title, type: thread.type, bodyHtml: cleaned, threadAuthorUserId: thread.created_by ?? null },
+  }
 }
 
 /**

server/src/model/teams/teamNotify.db.js

@@ -198,7 +198,7 @@ async function teamsWithForumActivitySince(since) {
 async function digestPostsSince(teamId, since, limit = 20) {
   return query(
     `SELECT po.id, po.thread_id, po.body_html, po.created_at, po.author_user_id, po.author_username,
-            th.title, th.type
+            th.title, th.type, th.created_by AS thread_created_by
        FROM team_forum_posts po
        JOIN team_forum_threads th ON th.id = po.thread_id
       WHERE th.team_id = ?

server/src/router/v1/player/teamForum.controller.js

@@ -114,6 +114,7 @@ async function announce(ctx, actor, notify) {
     threadTitle: notify.title,
     type: notify.type,
     authorUserId: actor.id,
+    threadAuthorUserId: notify.threadAuthorUserId ?? null,
     authorName: actor.username,
     bodyHtml: notify.bodyHtml,
   })

server/src/utils/teamDigestWorker.js

@@ -42,6 +42,7 @@ const templates = require('../engagement/templates')
 const emailChannel = require('../engagement/emailChannel')
 const mailer = require('./mailer')
 const notify = require('./teamNotify')
+const safety = require('../model/safety/userSafety.model')
 const { DELETED_PLAYER } = require('./displayAuthor')
 const log = require('./logger')('team-digest')
 
@@ -114,7 +115,17 @@ async function digestRule() {
 async function sendOne(team, recipient, rule, now) {
   const scopeKey = notify.scopeKey(team)
   const since = clampSince(recipient.last_digest_at, now)
-  const posts = await teamNotify.digestPostsSince(team.id, since, MAX_ITEMS)
+  const all = await teamNotify.digestPostsSince(team.id, since, MAX_ITEMS)
+  // What the forum would leave out for THIS recipient stays out of their mail too:
+  // posts by an account they block, posts in a thread whose starter they block, and
+  // anything they hid (USER_SAFETY.md §4.1). Same filter, read at send time like
+  // the rest of the digest.
+  const filter = await safety.forumFilter(recipient.user_id)
+  const posts = all.filter((p) =>
+    !filter.hidden.team_forum_post.has(Number(p.id))
+    && !filter.hidden.team_forum_thread.has(Number(p.thread_id))
+    && !(p.author_user_id != null && filter.blockedUserIds.has(Number(p.author_user_id)))
+    && !(p.thread_created_by != null && filter.blockedUserIds.has(Number(p.thread_created_by))))
   // Nothing new for THIS recipient — which is not the same as nothing new for the
   // Team, because each recipient has their own `since`. No mail, and no stamp: a
   // stamp here would move the window past posts they have not been told about.

server/src/utils/teamNotify.js

@@ -50,6 +50,7 @@ const pushDispatch = require('./pushDispatch')
 const teamBridge = require('./teamBridge')
 const teamNotify = require('../model/teams/teamNotify.model')
 const forumSettings = require('../model/teams/teamForumSettings.model')
+const safety = require('../model/safety/userSafety.model')
 const engagementEmit = require('./engagementEmit')
 const registries = require('../modules/registries')
 const log = require('./logger')('team-notify')
@@ -131,6 +132,25 @@ function excerpt(html) {
   return text.length > EXCERPT_CHARS ? `${text.slice(0, EXCERPT_CHARS - 1)}…` : text
 }
 
+/**
+ * The members a forum post must skip because they block its author, or the
+ * author of the thread it is in (USER_SAFETY.md §4.1): the forum leaves both out
+ * of their view, so a notification would point at something they cannot open.
+ *
+ * A failed lookup answers empty and is logged. A notification that reaches one
+ * member too many is the lesser failure than a forum post that tells nobody.
+ */
+async function blockedRecipients(...authorUserIds) {
+  try {
+    const ids = [...new Set(authorUserIds.filter((id) => id != null).map(Number))]
+    const lists = await Promise.all(ids.map((id) => safety.blockersOf(id)))
+    return [...new Set(lists.flat())]
+  } catch (err) {
+    log.warn('block lookup failed; notifying without it', { message: err.message })
+    return []
+  }
+}
+
 /** The push half. Resolves recipients, honours mutes and subscriptions, never throws. */
 async function tickle(streamId, team, { ref, exclude = [] } = {}) {
   const userIds = await teamNotify.recipientIds(team.id, { exclude })
@@ -238,7 +258,9 @@ async function leadershipChanged(team, { names = [] } = {}) {
  *
  * The author is excluded from the tickle and from the emitted audience. Not as a
  * nicety — a forum that emails you your own post is the first thing anyone turns
- * off, and turning it off costs the deployment every other notification too.
+ * off, and turning it off costs the deployment every other notification too. So
+ * is every member who blocks the author or the thread's starter: the forum hides
+ * the post from them, and the notification follows the forum.
  *
  * @returns {{push: number, emitted: boolean, bridged: boolean}} `emitted` says the
  *   event reached the engine, NOT that anybody was mailed. Whether a mail goes out
@@ -246,7 +268,7 @@ async function leadershipChanged(team, { names = [] } = {}) {
  *   that reported "3 emails sent" from here would be reporting a decision that has
  *   not been taken yet. The old `emails` count is gone for that reason.
  */
-async function forumPost({ team, threadId, threadTitle, type, authorUserId, authorName, bodyHtml }) {
+async function forumPost({ team, threadId, threadTitle, type, authorUserId, threadAuthorUserId = null, authorName, bodyHtml }) {
   try {
     // Belt and braces with the routes, which already 404 when forums are off. The
     // digest worker has no route in front of it, so the check has to live here as
@@ -256,7 +278,12 @@ async function forumPost({ team, threadId, threadTitle, type, authorUserId, auth
 
     const announcement = type === 'announcement'
     const stream = announcement ? STREAMS.ANNOUNCEMENT : STREAMS.FORUM_POST
-    const exclude = authorUserId ? [authorUserId] : []
+    // The author, and every member who blocks the author or the thread's starter.
+    // One list, so push, the inbox and email all skip the same people.
+    const exclude = [
+      ...(authorUserId ? [authorUserId] : []),
+      ...(await blockedRecipients(authorUserId, threadAuthorUserId)),
+    ]
     const recipientIds = await teamNotify.recipientIds(team.id, { exclude })
     const push = await tickle(stream, team, { ref: `team:${team.id}:thread:${threadId}`, exclude })
 

server/test/displayAuthor.test.js

@@ -145,7 +145,12 @@ test('the digest mail names a deleted author "[deleted player]"', async () => {
   const worker = require('../src/utils/teamDigestWorker')
   const teamNotify = require('../src/model/teams/teamNotify.model')
   const templates = require('../src/engagement/templates')
+  const safety = require('../src/model/safety/userSafety.model')
   let items = null
+  patch(safety, 'forumFilter', async () => ({
+    blockedUserIds: new Set(),
+    hidden: { team_forum_thread: new Set(), team_forum_post: new Set() },
+  }))
   patch(teamNotify, 'digestPostsSince', async () => [
     { id: 1, thread_id: 5, body_html: '<p>a</p>', created_at: new Date(), author_user_id: null, author_username: 'Ravenna', title: 'Raid night', type: 'discussion' },
     { id: 2, thread_id: 5, body_html: '<p>b</p>', created_at: new Date(), author_user_id: 4, author_username: 'Thorne', title: 'Raid night', type: 'discussion' },

server/test/teamBridge.test.js

@@ -28,6 +28,7 @@ const notifyDb = require('../src/model/teams/teamNotify.db')
 const forumSettings = require('../src/model/teams/teamForumSettings.model')
 const pushDispatch = require('../src/utils/pushDispatch')
 const mailer = require('../src/utils/mailer')
+const safety = require('../src/model/safety/userSafety.model')
 
 const saved = new Map()
 
@@ -59,6 +60,7 @@ beforeEach(() => {
   patch(notifyDb, 'emailRecipients', async () => [])
   patch(forumSettings, 'forumsEnabled', async () => true)
   patch(mailer, 'isConfigured', async () => false)
+  patch(safety, 'blockersOf', async () => [])
 })
 
 afterEach(restore)

server/test/teamNotifyDispatch.test.js

@@ -34,6 +34,7 @@ const rulesDb = require('../src/model/engagement/engagementRules.db')
 const sendsDb = require('../src/model/engagement/engagementSends.db')
 const templates = require('../src/engagement/templates')
 const registries = require('../src/modules/registries')
+const safety = require('../src/model/safety/userSafety.model')
 
 const saved = new Map()
 
@@ -71,6 +72,8 @@ function stub({
   recipients = [10, 11],
   emailRows = [],
   sendOk = true,
+  blockers = {},      // authorUserId → the members who block them
+  readerFilter = {},  // userId → { blocked: [...], hiddenPosts: [...], hiddenThreads: [...] }
 } = {}) {
   sent = []
   emitted = []
@@ -108,6 +111,14 @@ function stub({
   // No module registered: the default in most tests, so the link-building ones
   // have to opt in and the absence is exercised rather than assumed.
   patch(registries, 'registeredTeamProvider', () => null)
+  patch(safety, 'blockersOf', async (authorId) => blockers[authorId] || [])
+  patch(safety, 'forumFilter', async (userId) => {
+    const f = readerFilter[userId] || {}
+    return {
+      blockedUserIds: new Set(f.blocked || []),
+      hidden: { team_forum_thread: new Set(f.hiddenThreads || []), team_forum_post: new Set(f.hiddenPosts || []) },
+    }
+  })
 }
 
 beforeEach(() => stub())
@@ -142,6 +153,32 @@ test('the author is not among the tickled, nor among the emitted audience', asyn
   assert.deepEqual(emitted[0].recipientUserIds, [11])
 })
 
+// USER_SAFETY.md §4.1: the forum leaves a blocked author's posts out of a reader's
+// view, so the notification must not point that reader at one.
+test('a member who blocks the author is not tickled or told', async () => {
+  stub({ recipients: [10, 11, 12], blockers: { 10: [12] } })
+  await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 })
+  assert.deepEqual(sent[0].userIds, [11])
+  assert.deepEqual(emitted[0].recipientUserIds, [11])
+})
+
+test('a member who blocks the thread’s starter is not told about replies in it', async () => {
+  // A reply by 11 in a thread 10 started; 12 blocks 10, so cannot open the thread.
+  stub({ recipients: [10, 11, 12], blockers: { 10: [12] } })
+  await notify.forumPost({
+    team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 11, threadAuthorUserId: 10,
+  })
+  assert.deepEqual(sent[0].userIds, [10])
+  assert.deepEqual(emitted[0].recipientUserIds, [10])
+})
+
+test('a block lookup that fails still notifies everyone else', async () => {
+  patch(safety, 'blockersOf', async () => { throw new Error('database is on fire') })
+  const res = await notify.forumPost({ team: TEAM, threadId: 7, threadTitle: 'Hi', type: 'discussion', authorUserId: 10 })
+  assert.equal(res.push, 1)
+  assert.deepEqual(emitted[0].recipientUserIds, [11])
+})
+
 test('roster events fire one tickle for the run, not one per member', async () => {
   await notify.memberJoined(TEAM)
   await notify.leadershipChanged(TEAM)
@@ -312,6 +349,29 @@ test('the digest renders the rule’s digest template, not its instant one', asy
   assert.match(mails[0].rendered.subject, /^\[notify\.digest\]/)
 })
 
+test('a digest leaves out what the forum leaves out for that reader', async () => {
+  stub({ readerFilter: { 11: { blocked: [40, 50], hiddenPosts: [3], hiddenThreads: [9] } } })
+  stubDigest({
+    posts: [
+      { id: 1, thread_id: 7, title: 'Raid', body_html: '<p>kept</p>', author_user_id: 20, author_username: 'twenty' },
+      { id: 2, thread_id: 7, title: 'Raid', body_html: '<p>blocked author</p>', author_user_id: 40, author_username: 'forty' },
+      { id: 3, thread_id: 7, title: 'Raid', body_html: '<p>hidden post</p>', author_user_id: 20, author_username: 'twenty' },
+      { id: 4, thread_id: 9, title: 'Hidden', body_html: '<p>hidden thread</p>', author_user_id: 20, author_username: 'twenty' },
+      { id: 5, thread_id: 8, title: 'Theirs', body_html: '<p>blocked starter</p>', author_user_id: 20, author_username: 'twenty', thread_created_by: 50 },
+    ],
+  })
+  await digest.tick(new Date())
+  assert.deepEqual(mails[0].rendered.values.items.map((i) => i.excerpt), ['kept'])
+})
+
+test('a digest whose every post is filtered out sends nothing and stamps nothing', async () => {
+  stub({ readerFilter: { 11: { blocked: [40] } } })
+  stubDigest({ posts: [{ id: 2, thread_id: 7, title: 'Raid', body_html: '<p>x</p>', author_user_id: 40, author_username: 'forty' }] })
+  const res = await digest.tick(new Date())
+  assert.equal(res.sent, 0)
+  assert.equal(world.stamped.length, 0)
+})
+
 test('a recipient with nothing new gets no mail and no stamp', async () => {
   stubDigest({ posts: [] })
   const res = await digest.tick(new Date())

server/test/userSafety.test.js

@@ -201,6 +201,20 @@ test('a signed-out reader has no blocks and no hides', async () => {
   assert.equal((await safety.forumFilter(null)).blockedUserIds.size, 0)
 })
 
+test('blockersOf answers who blocks an author, and nobody for a staff author (S8)', async () => {
+  patch(safetyDb, 'blockersOf', async () => [21, 22])
+  patch(safetyDb, 'userById', async (id) => ({ id, username: 'rusty', role: 'player' }))
+  assert.deepEqual(await safety.blockersOf(12), [21, 22])
+
+  patch(safetyDb, 'userById', async (id) => ({ id, username: 'mod', role: 'moderator' }))
+  assert.deepEqual(await safety.blockersOf(2), [])
+
+  // A deleted account or no author at all: nothing to look up.
+  patch(safetyDb, 'userById', async () => null)
+  assert.deepEqual(await safety.blockersOf(99), [])
+  assert.deepEqual(await safety.blockersOf(null), [])
+})
+
 test('staffAmong takes only positive integer ids', async () => {
   let asked
   patch(safetyDb, 'staffAmong', async (ids) => { asked = ids; return [] })