From b458c1f46fb9d32b274dbdfc881a5444b54d6542 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Tue, 18 Aug 2026 14:35:08 -0500 Subject: [PATCH] =?UTF-8?q?feat(teams):=20the=20web=20surface=20=E2=80=94?= =?UTF-8?q?=20a=20notifications=20screen=20that=20did=20not=20exist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is phase 6's first finding, and it changed the phase's shape. TEAMS.md §6.3 says the per-Team mute list is surfaced "under the existing notification settings screen". There was no such screen. `/auth/me/notifications/*` was built for the Android app in M7 and had ZERO web consumers — a browser could not see the stream catalog or its own subscriptions at all. That is tolerable while push is the only sink, because push needs the app anyway. It is not tolerable for email, whose entire argument is the web-only user who runs neither the app nor Discord, so the sink and the screen to configure it had to ship together. `/account/notifications` carries all three: what to be told about, which Teams, and whether any of it reaches a mailbox — in the order a user actually reasons about them. The mute toggle goes in a THIRD module-declared slot, above the roster, because muting is an action ON the guild page while the feed and forum are content IN it. It renders nothing for a viewer with no preference available, which is a privacy property rather than a tidiness one: whether a preference EXISTS for a Team answers "is this person in it", and the guild page is public. `/unsubscribe/:token` is public and POSTs on mount — the link the user clicked was a GET, and a GET that mutated would be triggered by every mail-client link scanner. Co-Authored-By: Claude --- client/src/App.jsx | 7 + client/src/api/client.js | 20 ++ client/src/main.jsx | 7 + client/src/modules/TeamNotifyToggle.jsx | 102 +++++++ .../src/routes/player/PlayerNotifications.jsx | 268 ++++++++++++++++++ .../src/routes/player/PlayerPortalLayout.jsx | 3 + client/src/routes/player/Unsubscribe.jsx | 69 +++++ client/test/teamNotify.test.js | 87 ++++++ 8 files changed, 563 insertions(+) create mode 100644 client/src/modules/TeamNotifyToggle.jsx create mode 100644 client/src/routes/player/PlayerNotifications.jsx create mode 100644 client/src/routes/player/Unsubscribe.jsx create mode 100644 client/test/teamNotify.test.js diff --git a/client/src/App.jsx b/client/src/App.jsx index a63c557..88ff3f7 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -57,6 +57,8 @@ import ResetPassword from './routes/player/ResetPassword.jsx' import AcceptInvite from './routes/player/AcceptInvite.jsx' import PlayerPortalLayout, { PlayerIndex } from './routes/player/PlayerPortalLayout.jsx' import PlayerAccount from './routes/player/PlayerAccount.jsx' +import PlayerNotifications from './routes/player/PlayerNotifications.jsx' +import Unsubscribe from './routes/player/Unsubscribe.jsx' import PlayerAppeals from './routes/player/PlayerAppeals.jsx' export default function App() { @@ -204,6 +206,10 @@ export default function App() { } /> } /> } /> + {/* PUBLIC, and grouped with the other tokened landings above rather + than with the portal below: the person following an unsubscribe + link is reading their mail, not signed in (TEAMS.md §6.4). */} + } /> @@ -218,6 +224,7 @@ export default function App() { } /> } /> } /> + } /> {/* Installed modules' player-portal pages, at /player//…. This group's own routes are absolute (its layout route has no path), so the prefix is written here rather than inherited — the one diff --git a/client/src/api/client.js b/client/src/api/client.js index a8ae529..010d9c7 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -187,6 +187,26 @@ export const api = { req(`/player/teams/${encodeURIComponent(slug)}/grants`, { method: 'POST', body }), teamGrantRevoke: (slug, userId) => req(`/player/teams/${encodeURIComponent(slug)}/grants/${userId}`, { method: 'DELETE' }), + + // ----- notifications (TEAMS.md Part 6) ----- + // + // Under /auth/me rather than /player: these are role-agnostic self-service, the + // same rule that put the forum under /player rather than behind a staff gate. + // The streams catalog and the per-stream subscriptions were built for the app + // and had no web consumer at all until phase 6 gave them one. + notificationStreams: () => req('/auth/me/notifications/streams'), + notificationSubscriptions: () => req('/auth/me/notifications/subscriptions'), + // `streams` is always sent, empty array included — the endpoint requires the + // field, so clearing the last subscription must not become an absent key. + setNotificationSubscriptions: (streams) => + req('/auth/me/notifications/subscriptions', { method: 'PUT', body: { streams } }), + teamNotificationPrefs: () => req('/auth/me/notifications/teams'), + setTeamNotificationPrefs: (teams) => + req('/auth/me/notifications/teams', { method: 'PUT', body: { teams } }), + // Unauthenticated, and the one write in the public tier: the caller is reading + // their mail, not signed in. Always resolves 200 whatever the token was. + unsubscribeTeam: (token) => + req(`/public/teams/unsubscribe/${encodeURIComponent(token)}`, { method: 'POST' }), wikiTags: () => req('/public/wiki/tags'), wikiPage: (slug) => req(`/public/wiki/${slug}`), // CMS pages (block-based). Published-only for the public; a draft-preview link diff --git a/client/src/main.jsx b/client/src/main.jsx index 0d26693..4b6cb27 100644 --- a/client/src/main.jsx +++ b/client/src/main.jsx @@ -6,6 +6,7 @@ import { publishSharedDependencies } from './modules/shared.js' import { declareSlot, applyCoreFills, fillModuleSlot } from './modules/registry.js' import TeamActivityFeed from './modules/TeamActivityFeed.jsx' import TeamForumPanel from './modules/TeamForumPanel.jsx' +import TeamNotifyToggle from './modules/TeamNotifyToggle.jsx' import './styles/theme.css' // Publish window.__rg BEFORE rendering and before any module chunk evaluates. @@ -84,6 +85,12 @@ fillModuleSlot('uo.guild.detail', TeamActivityFeed) // exactly as before. fillModuleSlot('uo.guild.forum', TeamForumPanel) +// And the notification control, in a third place the module declares ABOVE its +// roster. A third slot rather than a corner of the feed for the same reason there +// were two: this is an action on the page and the other two are content in it, +// and only the module can say where each belongs on a page it owns. +fillModuleSlot('uo.guild.header', TeamNotifyToggle) + // Render on DOMContentLoaded rather than immediately, and that is the one line // of core's boot the module system changes. // diff --git a/client/src/modules/TeamNotifyToggle.jsx b/client/src/modules/TeamNotifyToggle.jsx new file mode 100644 index 0000000..e48c8e6 --- /dev/null +++ b/client/src/modules/TeamNotifyToggle.jsx @@ -0,0 +1,102 @@ +import { useCallback, useEffect, useState } from 'react' +import { Link } from 'react-router-dom' +import { api } from '../api/client.js' +import { useAuth } from '../contexts/AuthContext.jsx' + +// Core's per-Team notification control, rendered into a THIRD slot a module +// declares (TEAMS.md §6.3, phase 6). +// +// **Why this is a slot at all, and why it is the third one.** Teams have no core +// page — the module that owns the vocabulary owns the page — so a control that +// acts on one Team has nowhere of core's to live. The feed and the forum go below +// the module's roster; this goes above it, because muting a guild is an action ON +// the page rather than more content in it, and that is exactly the placement +// decision a module cannot make if core stacks everything into one fill. +// +// **It renders nothing for a viewer who is not in the Team**, including anonymous +// ones, and that is a privacy property rather than a tidiness one: whether a +// notification preference EXISTS for a Team answers "is this person in it", and +// the guild page is public. The server decides — the preference list only contains +// Teams the caller may be notified about — and this file never infers membership +// from anything it can see on the page. +// +// **Muting is per-Team and covers all four streams.** The per-stream on/off lives +// on the account screen, where the catalog does; the thing that could not be +// expressed before phase 6 is "I am in five Teams and want notifications from +// one", and that is the only question this control asks. + +export default function TeamNotifyToggle({ externalId, moduleId }) { + const { user } = useAuth() + const [state, setState] = useState({ loading: true, team: null, pref: null }) + const [busy, setBusy] = useState(false) + + const load = useCallback(async () => { + // Anonymous viewers never fetch. The endpoint would 401 harmlessly, but a + // guild page rendering a public roster should not put an authenticated + // request on the wire for every visitor. + if (!user) return setState({ loading: false, team: null, pref: null }) + try { + const team = await api.teamByExternalId(moduleId, externalId) + const { teams } = await api.teamNotificationPrefs() + const pref = (teams || []).find((t) => t.teamId === team.id) || null + setState({ loading: false, team, pref }) + } catch { + // Same rule as the feed and the forum: this is core's content on a page + // core does not own, so a failure renders nothing rather than putting an + // error box on somebody else's surface. + setState({ loading: false, team: null, pref: null }) + } + }, [externalId, moduleId, user]) + + useEffect(() => { load() }, [load]) + + const { loading, pref } = state + if (loading || !pref) return null + + async function toggle() { + setBusy(true) + // Optimistic, and reconciled from the server's echo rather than assumed: a + // PUT that silently dropped the entry (a Team left in another tab) must not + // leave the control claiming a state the server does not hold. + const next = { ...pref, muted: !pref.muted } + setState((s) => ({ ...s, pref: next })) + try { + const { teams } = await api.setTeamNotificationPrefs([ + { teamId: pref.teamId, muted: next.muted, emailMode: pref.emailMode }, + ]) + const echoed = (teams || []).find((t) => t.teamId === pref.teamId) + if (echoed) setState((s) => ({ ...s, pref: echoed })) + } catch { + setState((s) => ({ ...s, pref })) + } finally { + setBusy(false) + } + } + + return ( +
+ + + {pref.muted + ? 'You get no notifications about this team.' + : 'You get notifications about this team.'} + + {/* The one link off this control, because "mute" is a blunt answer to a + question the account screen asks properly — which streams, and whether + email is on at all. */} + All notification settings +
+ ) +} diff --git a/client/src/routes/player/PlayerNotifications.jsx b/client/src/routes/player/PlayerNotifications.jsx new file mode 100644 index 0000000..ad9ac9b --- /dev/null +++ b/client/src/routes/player/PlayerNotifications.jsx @@ -0,0 +1,268 @@ +import { useCallback, useEffect, useState } from 'react' +import { Loading, ErrorState } from '../../components/PageState.jsx' +import { api } from '../../api/client.js' + +// The account's notification settings (TEAMS.md §6.3/§6.4, phase 6). +// +// **This screen did not exist before phase 6, and that was the phase's first +// finding.** §6.3 says the per-Team mute list is "surfaced under the existing +// notification settings screen" — there was no such screen on the web. The stream +// catalog and the per-stream subscriptions have been built and shipped since M7, +// with the Android app as their only consumer; a browser could not see them at +// all. That is tolerable for push, which needs the app anyway. It is not tolerable +// for email, whose whole reason for existing (§6.4) is the web-only user who runs +// neither the app nor Discord — so the sink and the screen to configure it had to +// arrive together. +// +// Three blocks, in the order a user actually reasons about them: what kinds of +// thing to be told about, then which Teams, then whether any of it should reach a +// mailbox. + +const EMAIL_MODES = [ + { value: 'off', label: 'No email' }, + { value: 'digest', label: 'Daily digest' }, + { value: 'immediate', label: 'Every post' }, +] + +// Streams whose scoping lives in this page's second block rather than in the +// first. Shown as a group so a user does not toggle `team.forum.post` off site- +// wide when what they meant was "not this one guild". +const isTeamStream = (id) => String(id).startsWith('team.') + +function Section({ title, hint, children }) { + return ( +
+

{title}

+ {hint &&

{hint}

} + {children} +
+ ) +} + +function Note({ msg, error }) { + if (!msg && !error) return null + return ( +

+ {error || msg} +

+ ) +} + +// ── What to be told about ────────────────────────────────────────────────── + +function Streams({ streams, subscribed, onSave, busy, msg, error }) { + const [set, setSet] = useState(() => new Set(subscribed)) + useEffect(() => { setSet(new Set(subscribed)) }, [subscribed]) + + const toggle = (id) => { + const next = new Set(set) + if (next.has(id)) next.delete(id) + else next.add(id) + setSet(next) + } + + const team = streams.filter((s) => isTeamStream(s.id)) + const rest = streams.filter((s) => !isTeamStream(s.id)) + + const row = (s) => ( + + ) + + return ( +
+
{rest.map(row)}
+ {team.length > 0 && ( + <> +

+ Teams +

+
{team.map(row)}
+ + )} +
+ +
+ +
+ ) +} + +// ── Which Teams, and whether by email ────────────────────────────────────── + +function Teams({ teams, onSave, busy, msg, error }) { + const [rows, setRows] = useState(teams) + useEffect(() => { setRows(teams) }, [teams]) + + const patch = (teamId, change) => + setRows((rs) => rs.map((r) => (r.teamId === teamId ? { ...r, ...change } : r))) + + if (rows.length === 0) { + return ( +
+

+ You are not in a team, and nobody has given you access to a team forum. There is nothing to + configure here yet. +

+
+ ) + } + + return ( +
+
+ + + + + + + + + + {rows.map((t) => ( + + + + + + ))} + +
TeamNotificationsEmail
+ {t.name} + {/* An archived Team is still listed when a preference exists for + it, so a mute does not silently vanish when a guild disbands + and reappear if it re-forms under the same name. */} + {t.archived && · archived} + + + + +
+
+
+ +
+ +
+ ) +} + +// ── Page ─────────────────────────────────────────────────────────────────── + +export default function PlayerNotifications() { + const [loading, setLoading] = useState(true) + const [error, setError] = useState('') + const [streams, setStreams] = useState([]) + const [subscribed, setSubscribed] = useState([]) + const [teams, setTeams] = useState([]) + const [saving, setSaving] = useState({ streams: false, teams: false }) + const [notes, setNotes] = useState({ streams: '', teams: '', streamsError: '', teamsError: '' }) + + const load = useCallback(async () => { + setLoading(true) + try { + // Three reads in parallel: the catalog is boot-fixed, the subscriptions and + // the Team list are this user's. None depends on another. + const [cat, subs, prefs] = await Promise.all([ + api.notificationStreams(), + api.notificationSubscriptions(), + api.teamNotificationPrefs(), + ]) + setStreams(cat.streams || []) + setSubscribed(subs.streams || []) + setTeams(prefs.teams || []) + setError('') + } catch { + setError('Could not load your notification settings.') + } finally { + setLoading(false) + } + }, []) + + useEffect(() => { load() }, [load]) + + const saveStreams = useCallback(async (ids) => { + setSaving((s) => ({ ...s, streams: true })) + setNotes((n) => ({ ...n, streams: '', streamsError: '' })) + try { + const { streams: stored } = await api.setNotificationSubscriptions(ids) + setSubscribed(stored || []) + setNotes((n) => ({ ...n, streams: 'Saved.' })) + } catch { + setNotes((n) => ({ ...n, streamsError: 'Could not save that.' })) + } finally { + setSaving((s) => ({ ...s, streams: false })) + } + }, []) + + const saveTeams = useCallback(async (rows) => { + setSaving((s) => ({ ...s, teams: true })) + setNotes((n) => ({ ...n, teams: '', teamsError: '' })) + try { + // The whole set, every time, and the array is sent even when empty — the + // endpoint requires the field (docs/android/PLAN.md §11). + const { teams: stored } = await api.setTeamNotificationPrefs( + rows.map((t) => ({ teamId: t.teamId, muted: t.muted, emailMode: t.emailMode })), + ) + setTeams(stored || []) + setNotes((n) => ({ ...n, teams: 'Saved.' })) + } catch { + setNotes((n) => ({ ...n, teamsError: 'Could not save that.' })) + } finally { + setSaving((s) => ({ ...s, teams: false })) + } + }, []) + + if (loading) return + if (error) return + + return ( +
+

+ Choose what you are told about, and how. Nothing here is on by default except team + notifications to the app, which you can mute per team below. +

+ + +
+ ) +} diff --git a/client/src/routes/player/PlayerPortalLayout.jsx b/client/src/routes/player/PlayerPortalLayout.jsx index 12e9153..35c3312 100644 --- a/client/src/routes/player/PlayerPortalLayout.jsx +++ b/client/src/routes/player/PlayerPortalLayout.jsx @@ -35,6 +35,7 @@ function Icon({ children, size = 16 }) { } const IconGear = () => const IconShield = () => +const IconBell = () => // Exported because Admin -> Navigation edits this list. It stays declared here; // the editor may only relabel, reorder and hide what it finds (§7). No CORE row @@ -47,6 +48,7 @@ const IconShield = () => + + {state === 'working' &&

One moment…

} + {state === 'done' && ( + <> +

+ You will not receive further notification emails about this team. +

+

+ This muted the team rather than switching off your account’s email, so your other + teams are unaffected. You can turn it back on any time under{' '} + notification settings. +

+ + )} + {state === 'failed' && ( +

+ We could not reach the site to record that. Please try the link again, or change the + setting yourself under notification settings. +

+ )} + + ) +} diff --git a/client/test/teamNotify.test.js b/client/test/teamNotify.test.js new file mode 100644 index 0000000..fb39a0a --- /dev/null +++ b/client/test/teamNotify.test.js @@ -0,0 +1,87 @@ +import { test, beforeEach, afterEach } from 'node:test' +import assert from 'node:assert/strict' +import { api } from '../src/api/client.js' + +// The client half of Team notifications (docs/website/TEAMS.md Part 6, phase 6). +// +// There is no DOM in this runner, so what is asserted here is the WIRE — which is +// where this feature's client-side mistakes actually live. Two of them have +// already been made once in this repo and are recorded rather than re-derived: +// +// 1. **A PUT-the-whole-set body must always carry its array**, empty included. +// `docs/android/PLAN.md` §11: a DTO field with a default is dropped by +// kotlinx when it equals that default, so "clear the last entry" arrives as a +// body with no array at all and 400s. The web client has no such +// serialisation quirk, but it shares the endpoint's contract, and a test that +// pins the shape here is what keeps the two clients honest about the same +// rule. +// 2. **The unsubscribe call is a POST**, not the GET the link in the mail was. +// A GET that mutated would be triggered by every mail-client link scanner. + +let calls +const realFetch = global.fetch + +function reply(body = {}) { + return { + ok: true, + status: 200, + statusText: 'OK', + text: async () => JSON.stringify(body), + } +} + +beforeEach(() => { + calls = [] + global.fetch = async (url, opts = {}) => { + calls.push({ url, opts }) + return reply({ teams: [], streams: [], ok: true }) + } +}) + +afterEach(() => { global.fetch = realFetch }) + +const body = (i = 0) => JSON.parse(calls[i].opts.body) + +test('the per-Team preference endpoints sit under /auth/me, not /player', async () => { + await api.teamNotificationPrefs() + // Role-agnostic self-service, the same rule that put the Team forum under + // /player rather than behind a staff gate: staff are a superset of players and + // manage their own notifications like anyone else. + assert.match(calls[0].url, /\/auth\/me\/notifications\/teams$/) + assert.equal(calls[0].opts.method ?? 'GET', 'GET') +}) + +test('saving preferences PUTs the whole set under a `teams` key', async () => { + await api.setTeamNotificationPrefs([{ teamId: 3, muted: true, emailMode: 'digest' }]) + assert.equal(calls[0].opts.method, 'PUT') + assert.deepEqual(body(), { teams: [{ teamId: 3, muted: true, emailMode: 'digest' }] }) +}) + +test('clearing every preference still sends the array, never an absent key', async () => { + await api.setTeamNotificationPrefs([]) + assert.deepEqual(body(), { teams: [] }) + assert.equal('teams' in body(), true) +}) + +test('the same rule holds for the stream subscriptions beside them', async () => { + await api.setNotificationSubscriptions([]) + assert.deepEqual(body(), { streams: [] }) +}) + +test('unsubscribe is a POST to the public tier, with the token encoded into the path', async () => { + await api.unsubscribeTeam('1.7.3.abcDEF') + assert.equal(calls[0].opts.method, 'POST') + assert.match(calls[0].url, /\/public\/teams\/unsubscribe\/1\.7\.3\.abcDEF$/) +}) + +test('a token with url-unsafe characters is encoded rather than pasted in', async () => { + await api.unsubscribeTeam('a/b c') + assert.match(calls[0].url, /unsubscribe\/a%2Fb%20c$/) +}) + +test('the streams catalog and subscriptions are separate reads', async () => { + await api.notificationStreams() + await api.notificationSubscriptions() + assert.match(calls[0].url, /\/notifications\/streams$/) + assert.match(calls[1].url, /\/notifications\/subscriptions$/) +})