fix(safety): team notifications skip members who block the author
#233 left this open: the forum hid a blocked author's posts from a member, but the member was still pushed, put in the inbox and emailed about them. - teamNotify.forumPost excludes every member who blocks the post's author, or the member who started the thread (the forum 404s that thread for them). One recipient list, so push, the inbox and email skip the same people. userSafety.blockersOf answers it, and a staff author is blocked by nobody (S8). A failed lookup notifies without it. - The daily digest applies each recipient's forumFilter at send time: blocked authors, blocked thread starters, hidden threads and posts. digestPostsSince now also returns the thread's created_by. - A reply's notify payload carries threadAuthorUserId. The Team activity feed needed nothing: forum posts are never written to it (membership, renames and module game events only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
|
||||
@@ -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 },
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 = ?
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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 })
|
||||
|
||||
|
||||
@@ -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' },
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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 [] })
|
||||
|
||||
Reference in New Issue
Block a user