fix(teams): make "replace the whole set" actually replace it
Found walking the live rig, which is the only place it could be found: every unit
test and the settings screen itself send every row, so the bug was invisible to
both.
`PUT /auth/me/notifications/teams` documents itself as replacing the whole set. It
did not — it wrote the entries it was given and left every other preference
standing. So `{"teams": []}` cleared nothing, which is precisely the body the route
requires the array for: the field is mandatory even when empty so that clearing
everything is expressible, and it was the one thing that did not work.
A Team the caller could have named and did not now returns to its defaults. RESET
rather than deleted, and the difference is `last_digest_at`: that column is the
digest worker's state and not a preference, so dropping the row with it would make
every visit to the settings screen re-open a day-wide digest window and mail
somebody a summary they had already read.
Walked again after the fix on the real database: the empty set clears, an entry
naming a Team the caller is not in is still dropped, and the digest stamp survives.
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -82,6 +82,24 @@ async function replacePrefs(userId, entries) {
|
|||||||
await db.setPref(userId, teamId, { muted: Boolean(entry.muted), emailMode })
|
await db.setPref(userId, teamId, { muted: Boolean(entry.muted), emailMode })
|
||||||
written.push(teamId)
|
written.push(teamId)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A Team the caller COULD have named and did not is returned to its defaults.
|
||||||
|
//
|
||||||
|
// Without this, "replace the whole set" was a lie the endpoint told: omitting an
|
||||||
|
// entry left the old preference standing, which made `teams: []` — the body the
|
||||||
|
// route requires precisely so that clearing everything is expressible — clear
|
||||||
|
// nothing at all.
|
||||||
|
//
|
||||||
|
// Reset rather than deleted, and the difference is `last_digest_at`. That column
|
||||||
|
// is the digest worker's state, not a preference; dropping the row with it would
|
||||||
|
// make every visit to the settings screen re-open a day-wide digest window and
|
||||||
|
// mail somebody a summary they already read.
|
||||||
|
for (const teamId of allowed.keys()) {
|
||||||
|
if (written.includes(teamId)) continue
|
||||||
|
// eslint-disable-next-line no-await-in-loop
|
||||||
|
await db.setPref(userId, teamId, { muted: false, emailMode: 'off' })
|
||||||
|
}
|
||||||
|
|
||||||
return { written, prefs: await listPrefs(userId) }
|
return { written, prefs: await listPrefs(userId) }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -214,6 +214,36 @@ test('replacePrefs ignores a Team the caller is not in', async () => {
|
|||||||
assert.equal(store.prefs.some((p) => p.user_id === 20 && p.team_id === 2), false)
|
assert.equal(store.prefs.some((p) => p.user_id === 20 && p.team_id === 2), false)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('replacePrefs really replaces: an omitted Team returns to its defaults', async () => {
|
||||||
|
await notifyModel.replacePrefs(10, [
|
||||||
|
{ teamId: 1, muted: true, emailMode: 'immediate' },
|
||||||
|
{ teamId: 2, muted: true, emailMode: 'digest' },
|
||||||
|
])
|
||||||
|
// Now save a set that names only Team 1. Team 2 was not mentioned, so it goes
|
||||||
|
// back to defaults — otherwise "PUT the whole set" is a lie and `teams: []`
|
||||||
|
// clears nothing, which is the body the route requires so that clearing
|
||||||
|
// everything is expressible in the first place.
|
||||||
|
await notifyModel.replacePrefs(10, [{ teamId: 1, muted: true, emailMode: 'immediate' }])
|
||||||
|
assert.deepEqual(await notifyModel.prefFor(10, 1), { teamId: 1, muted: true, emailMode: 'immediate' })
|
||||||
|
assert.deepEqual(await notifyModel.prefFor(10, 2), { teamId: 2, muted: false, emailMode: 'off' })
|
||||||
|
})
|
||||||
|
|
||||||
|
test('an empty set clears every preference the caller holds', async () => {
|
||||||
|
await notifyModel.replacePrefs(10, [{ teamId: 1, muted: true, emailMode: 'digest' }])
|
||||||
|
await notifyModel.replacePrefs(10, [])
|
||||||
|
assert.equal((await notifyModel.prefFor(10, 1)).muted, false)
|
||||||
|
assert.equal((await notifyModel.recipientIds(1)).includes(10), true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('the reset does not touch last_digest_at — that is the worker’s column', async () => {
|
||||||
|
await notifyModel.replacePrefs(10, [{ teamId: 1, muted: false, emailMode: 'digest' }])
|
||||||
|
const row = store.prefs.find((p) => p.user_id === 10 && p.team_id === 1)
|
||||||
|
row.last_digest_at = '2026-08-18T00:00:00Z'
|
||||||
|
await notifyModel.replacePrefs(10, [])
|
||||||
|
assert.equal(row.last_digest_at, '2026-08-18T00:00:00Z',
|
||||||
|
'dropping it would re-open a day-wide window on every visit to the settings screen')
|
||||||
|
})
|
||||||
|
|
||||||
test('an unknown email mode falls back to off rather than reaching the column', async () => {
|
test('an unknown email mode falls back to off rather than reaching the column', async () => {
|
||||||
await notifyModel.replacePrefs(10, [{ teamId: 1, muted: false, emailMode: 'hourly' }])
|
await notifyModel.replacePrefs(10, [{ teamId: 1, muted: false, emailMode: 'hourly' }])
|
||||||
assert.equal((await notifyModel.prefFor(10, 1)).emailMode, 'off')
|
assert.equal((await notifyModel.prefFor(10, 1)).emailMode, 'off')
|
||||||
|
|||||||
Reference in New Issue
Block a user