diff --git a/.changeset/admin-ui-channel-settings.md b/.changeset/admin-ui-channel-settings.md new file mode 100644 index 0000000000..cfb9da109b --- /dev/null +++ b/.changeset/admin-ui-channel-settings.md @@ -0,0 +1,38 @@ +--- +--- + +**Admin UI for editorial + announcement channels; Stage 1 env-var → DB migration** + +Closes two follow-ups flagged during Workflow B Stage 2/3 reviews. + +**Admin UI (`admin-settings.html`).** Two new sections on the System +Settings page: + +- *Editorial review channel* — private-channel picker. Stores into the + existing `editorial_slack_channel` setting. Workflow B Stage 1 review + cards land here. +- *Public announcement channel* — public-channel picker (pulls from the + `?visibility=public` variant of the picker endpoint). Stores into + the `announcement_slack_channel` setting added in Stage 2. + +Previously both settings were DB-backed but API-only — editorial team +had to `curl PUT /api/admin/settings/editorial-channel` to configure. + +**Stage 1 reader migration.** `runAnnouncementTriggerJob` and the +backfill script now call a new `resolveEditorialChannel()` that prefers +the DB setting and falls back to the legacy +`SLACK_EDITORIAL_REVIEW_CHANNEL` env var. Safe rollout: existing prod +config keeps working on deploy; once the admin UI is in prod an +operator can set the DB value and we can drop the env var in a later +PR. + +Also: transient DB read failures fall back to env rather than blocking +the job — the job cares about *any* way of getting a channel id, not +whichever storage won the coin flip this hour. + +**Tests.** `tests/announcement/announcement-channel-resolver.test.ts` +— 8 tests covering DB-populated wins over env, env-fallback-on-null, +both-null → null, env whitespace/empty handling, DB-throws → env +fallback, DB-throws + env-unset → null. + +Full announcement suite 143/143 pass; typecheck clean. diff --git a/server/public/admin-settings.html b/server/public/admin-settings.html index 759d8142fe..0f579da6d5 100644 --- a/server/public/admin-settings.html +++ b/server/public/admin-settings.html @@ -243,6 +243,60 @@

Admin notifications channel

+ +
+

Editorial review channel

+

+ Private channel where the editorial working group approves new-member + announcements, content drafts, and other posts that need human review + before they go public. +

+ +
+ Loading... +
+ +
+
+ + + Only private channels are shown. Invite Addie to your editorial channel first. +
+ +
+
+ + +
+

Public announcement channel public

+

+ Public channel where approved new-member announcements are posted. + This is the only picker here that accepts public channels — + announcements are meant for broad visibility. +

+ +
+ Loading... +
+ +
+
+ + + Only public channels are shown. Invite Addie to the channel first. +
+ +
+
+

Prospect notifications channel

@@ -349,6 +403,8 @@

Automatic prospect triage

updateCurrentAdminChannelDisplay(); updateCurrentProspectChannelDisplay(); updateCurrentErrorChannelDisplay(); + updateCurrentEditorialChannelDisplay(); + updateCurrentAnnouncementChannelDisplay(); updateTriageDisplay(); document.getElementById('loading').style.display = 'none'; @@ -363,30 +419,62 @@

Automatic prospect triage

} } - // Load Slack channels for picker + // Load Slack channels for all pickers. Private channels fill six + // of the seven dropdowns; the announcement dropdown pulls from the + // separate `visibility=public` endpoint because an announcement + // posted to a private channel would defeat the point. + // + // The two fetches are independent: if public fails, the five + // private pickers must still populate, so the public fetch is + // isolated in its own try/catch and degrades to a distinct + // error state on the announcement dropdown only. + let publicSlackChannels = []; + let publicFetchError = null; async function loadSlackChannels() { const billingSelect = document.getElementById('billingChannel'); const escalationSelect = document.getElementById('escalationChannel'); const adminSelect = document.getElementById('adminChannel'); const prospectSelect = document.getElementById('prospectChannel'); const errorSelect = document.getElementById('errorChannel'); + const editorialSelect = document.getElementById('editorialChannel'); + const announcementSelect = document.getElementById('announcementChannel'); try { - const response = await fetch('/api/admin/settings/slack-channels'); - if (!response.ok) { - const data = await response.json(); - throw new Error(data.message || 'Failed to load channels'); + const [privateRes, publicRes] = await Promise.all([ + fetch('/api/admin/settings/slack-channels'), + fetch('/api/admin/settings/slack-channels?visibility=public'), + ]); + if (!privateRes.ok) { + const data = await privateRes.json(); + throw new Error(data.message || 'Failed to load private channels'); } - const data = await response.json(); - slackChannels = data.channels; + const privateData = await privateRes.json(); + slackChannels = privateData.channels; - // Build options for all dropdowns + // Public fetch: keep isolated so a failure here doesn't wipe + // the private dropdowns. publicFetchError distinguishes + // "fetch failed" from "really zero public channels". + try { + if (!publicRes.ok) { + const data = await publicRes.json().catch(() => ({})); + throw new Error(data.message || `Public-channel fetch returned ${publicRes.status}`); + } + publicSlackChannels = (await publicRes.json()).channels ?? []; + publicFetchError = null; + } catch (pubErr) { + publicSlackChannels = []; + publicFetchError = pubErr instanceof Error ? pubErr.message : String(pubErr); + showStatusMessage('Could not load public channels: ' + publicFetchError, 'error'); + } + + // Build options for all private-channel dropdowns let billingHtml = ''; let escalationHtml = ''; let adminHtml = ''; let prospectHtml = ''; let errorHtml = ''; + let editorialHtml = ''; if (slackChannels.length === 0) { const emptyOption = ''; @@ -395,6 +483,7 @@

Automatic prospect triage

adminSelect.innerHTML = emptyOption; prospectSelect.innerHTML = emptyOption; errorSelect.innerHTML = emptyOption; + editorialSelect.innerHTML = emptyOption; showStatusMessage('Addie is not in any private channels. Invite @Addie to your channels and refresh.', 'error'); } else { slackChannels.forEach(ch => { @@ -403,11 +492,13 @@

Automatic prospect triage

const adminSelected = currentSettings?.admin_channel?.channel_id === ch.id ? 'selected' : ''; const prospectSelected = currentSettings?.prospect_channel?.channel_id === ch.id ? 'selected' : ''; const errorSelected = currentSettings?.error_channel?.channel_id === ch.id ? 'selected' : ''; + const editorialSelected = currentSettings?.editorial_channel?.channel_id === ch.id ? 'selected' : ''; billingHtml += ``; escalationHtml += ``; adminHtml += ``; prospectHtml += ``; errorHtml += ``; + editorialHtml += ``; }); billingSelect.innerHTML = billingHtml; billingSelect.disabled = false; @@ -428,6 +519,39 @@

Automatic prospect triage

errorSelect.innerHTML = errorHtml; errorSelect.disabled = false; document.getElementById('saveErrorChannelBtn').disabled = false; + + editorialSelect.innerHTML = editorialHtml; + editorialSelect.disabled = false; + document.getElementById('saveEditorialChannelBtn').disabled = false; + } + + // Public channels for the announcement picker. Three states: + // - Fetch failed → keep disabled + distinct error label + // - Fetch succeeded but no channels → invite-Addie hint + + // fire a warning (the picker is unusable without one) + // - Fetch succeeded with channels → normal render + let announcementHtml = ''; + if (publicFetchError) { + announcementSelect.innerHTML = ``; + } else if (publicSlackChannels.length === 0) { + announcementSelect.innerHTML = ''; + // Only warn if the admin needs this picker to work (no + // existing setting). If it's already configured, they + // don't need to see a scary error on every load. + if (!currentSettings?.announcement_channel?.channel_id) { + showStatusMessage( + 'Addie is not in any public channels. Invite @Addie to a public channel and refresh to enable the announcement picker.', + 'error', + ); + } + } else { + publicSlackChannels.forEach(ch => { + const selected = currentSettings?.announcement_channel?.channel_id === ch.id ? 'selected' : ''; + announcementHtml += ``; + }); + announcementSelect.innerHTML = announcementHtml; + announcementSelect.disabled = false; + document.getElementById('saveAnnouncementChannelBtn').disabled = false; } } catch (error) { console.error('Error loading Slack channels:', error); @@ -436,6 +560,8 @@

Automatic prospect triage

adminSelect.innerHTML = ''; prospectSelect.innerHTML = ''; errorSelect.innerHTML = ''; + editorialSelect.innerHTML = ''; + announcementSelect.innerHTML = ''; showStatusMessage('Failed to load Slack channels: ' + error.message, 'error'); } } @@ -752,6 +878,137 @@

Automatic prospect triage

} } + // Update the current editorial channel display. + // NB: we don't claim the feature is "disabled" when unset — the + // server can still fall back to SLACK_EDITORIAL_REVIEW_CHANNEL env + // var, and we don't know from the client whether that's set. "Not + // configured" is the honest label; the admin can reason about + // what that means for their deployment. + function updateCurrentEditorialChannelDisplay() { + const el = document.getElementById('currentEditorialChannel'); + const channel = currentSettings?.editorial_channel; + + if (channel?.channel_id && channel?.channel_name) { + el.className = 'current-value'; + el.innerHTML = `Current: #${escapeHtml(channel.channel_name)}`; + } else { + el.className = 'current-value not-set'; + el.innerHTML = 'Current: Not configured'; + } + } + + // Update the current announcement channel display. + function updateCurrentAnnouncementChannelDisplay() { + const el = document.getElementById('currentAnnouncementChannel'); + const channel = currentSettings?.announcement_channel; + + if (channel?.channel_id && channel?.channel_name) { + el.className = 'current-value'; + el.innerHTML = `Current: #${escapeHtml(channel.channel_name)}`; + } else { + el.className = 'current-value not-set'; + el.innerHTML = 'Current: Not configured'; + } + } + + // Save editorial channel + async function saveEditorialChannel() { + const select = document.getElementById('editorialChannel'); + const saveBtn = document.getElementById('saveEditorialChannelBtn'); + const channelId = select.value || null; + + let channelName = null; + if (channelId) { + const channel = slackChannels.find(c => c.id === channelId); + channelName = channel?.name || null; + } + + saveBtn.disabled = true; + saveBtn.textContent = 'Saving...'; + + try { + const response = await fetch('/api/admin/settings/editorial-channel', { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ channel_id: channelId, channel_name: channelName }), + }); + + if (!response.ok) { + const data = await response.json(); + throw new Error(data.message || 'Failed to save'); + } + + const data = await response.json(); + currentSettings.editorial_channel = data.editorial_channel; + updateCurrentEditorialChannelDisplay(); + + saveBtn.textContent = 'Saved!'; + saveBtn.classList.add('btn-success'); + showStatusMessage('Editorial channel updated successfully', 'success'); + + setTimeout(() => { + saveBtn.textContent = 'Save'; + saveBtn.classList.remove('btn-success'); + saveBtn.disabled = false; + }, 2000); + } catch (error) { + console.error('Error saving editorial channel:', error); + showStatusMessage('Failed to save: ' + error.message, 'error'); + saveBtn.textContent = 'Save'; + saveBtn.disabled = false; + } + } + + // Save announcement channel + async function saveAnnouncementChannel() { + const select = document.getElementById('announcementChannel'); + const saveBtn = document.getElementById('saveAnnouncementChannelBtn'); + const channelId = select.value || null; + + let channelName = null; + if (channelId) { + // Announcement picker pulls from the public channel list, not the + // private one — look up the name there. + const channel = publicSlackChannels.find(c => c.id === channelId); + channelName = channel?.name || null; + } + + saveBtn.disabled = true; + saveBtn.textContent = 'Saving...'; + + try { + const response = await fetch('/api/admin/settings/announcement-channel', { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ channel_id: channelId, channel_name: channelName }), + }); + + if (!response.ok) { + const data = await response.json(); + throw new Error(data.message || 'Failed to save'); + } + + const data = await response.json(); + currentSettings.announcement_channel = data.announcement_channel; + updateCurrentAnnouncementChannelDisplay(); + + saveBtn.textContent = 'Saved!'; + saveBtn.classList.add('btn-success'); + showStatusMessage('Announcement channel updated successfully', 'success'); + + setTimeout(() => { + saveBtn.textContent = 'Save'; + saveBtn.classList.remove('btn-success'); + saveBtn.disabled = false; + }, 2000); + } catch (error) { + console.error('Error saving announcement channel:', error); + showStatusMessage('Failed to save: ' + error.message, 'error'); + saveBtn.textContent = 'Save'; + saveBtn.disabled = false; + } + } + // Update triage toggle display function updateTriageDisplay() { const el = document.getElementById('currentTriageStatus'); diff --git a/server/src/addie/jobs/announcement-trigger.ts b/server/src/addie/jobs/announcement-trigger.ts index f81fcf85f8..dbaf23a7b6 100644 --- a/server/src/addie/jobs/announcement-trigger.ts +++ b/server/src/addie/jobs/announcement-trigger.ts @@ -25,6 +25,7 @@ import { resolveAnnouncementVisual, type VisualResolution, } from '../../services/announcement-visual.js'; +import { getEditorialChannel } from '../../db/system-settings-db.js'; import type { SlackBlock, SlackElement } from '../../slack/types.js'; const logger = createLogger('announcement-trigger'); @@ -416,12 +417,38 @@ async function processAnnounceCandidate( } } +/** + * Resolve the editorial review channel. Prefers the admin-UI DB setting + * (`editorial_slack_channel`), falls back to the legacy + * `SLACK_EDITORIAL_REVIEW_CHANNEL` env var for safe rollout. Both null + * returns `null`; callers should skip the run and log. + * + * Logs at error level when the DB read fails — the env fallback keeps + * the job running for transient blips, but a persistent outage where + * the admin's DB value is stale (or wrong) needs SRE attention, not a + * buried warn. + */ +export async function resolveEditorialChannel(): Promise { + try { + const setting = await getEditorialChannel(); + if (typeof setting.channel_id === 'string' && setting.channel_id.trim()) { + return setting.channel_id.trim(); + } + } catch (err) { + logger.error({ err }, 'resolveEditorialChannel: DB read failed, falling back to env'); + } + const env = process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; + return env && env.trim() ? env.trim() : null; +} + export async function runAnnouncementTriggerJob(): Promise { const result: TriggerResult = { candidates: 0, drafted: 0, failed: 0 }; - const reviewChannel = process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; + const reviewChannel = await resolveEditorialChannel(); if (!reviewChannel) { - logger.warn('SLACK_EDITORIAL_REVIEW_CHANNEL not configured — skipping run'); + logger.warn( + 'Editorial channel not configured — set it at /admin/settings (or SLACK_EDITORIAL_REVIEW_CHANNEL env) — skipping run', + ); return result; } diff --git a/server/src/scripts/backfill-member-announcements.ts b/server/src/scripts/backfill-member-announcements.ts index 773872aeb0..8875e8d7b5 100644 --- a/server/src/scripts/backfill-member-announcements.ts +++ b/server/src/scripts/backfill-member-announcements.ts @@ -44,6 +44,7 @@ import { initializeDatabase, closeDatabase } from '../db/client.js'; import { getDatabaseConfig } from '../config.js'; import { runBackfillAnnouncements, + resolveEditorialChannel, BACKFILL_SOFT_CAP, BACKFILL_ABSOLUTE_MAX, type BackfillPreviewRow, @@ -139,12 +140,6 @@ async function main(): Promise { process.exit(1); } - const reviewChannel = process.env.SLACK_EDITORIAL_REVIEW_CHANNEL ?? ''; - if (!args.dryRun && !reviewChannel) { - console.error('SLACK_EDITORIAL_REVIEW_CHANNEL is required unless --dry-run'); - process.exit(1); - } - if (args.limit > BACKFILL_SOFT_CAP && !args.force) { console.error( `--limit ${args.limit} exceeds the soft cap of ${BACKFILL_SOFT_CAP}. Pass --force to override (max ${BACKFILL_ABSOLUTE_MAX}).`, @@ -154,6 +149,17 @@ async function main(): Promise { initializeDatabase(dbConfig); + // DB first (admin UI), env fallback. Only required on live runs — + // dry-run just wants to enumerate candidates. + const reviewChannel = (await resolveEditorialChannel()) ?? ''; + if (!args.dryRun && !reviewChannel) { + console.error( + 'Editorial channel not configured. Set it at /admin/settings (or SLACK_EDITORIAL_REVIEW_CHANNEL env).', + ); + await closeDatabase(); + process.exit(1); + } + try { const result = await runBackfillAnnouncements({ reviewChannel, diff --git a/tests/announcement/announcement-channel-resolver.test.ts b/tests/announcement/announcement-channel-resolver.test.ts new file mode 100644 index 0000000000..1a0fc78b37 --- /dev/null +++ b/tests/announcement/announcement-channel-resolver.test.ts @@ -0,0 +1,145 @@ +/** + * Tests for `resolveEditorialChannel`: DB-first with env var fallback. + * + * The resolver is the narrow seam that lets Stage 1 + backfill switch + * from the legacy SLACK_EDITORIAL_REVIEW_CHANNEL env var to the admin-UI + * `editorial_slack_channel` system setting without breaking existing + * prod config. Behavior: + * - DB setting populated → use it + * - DB setting null → fall back to env + * - DB setting null AND env unset/empty → return null; caller skips + * - DB read throws → fall back to env (don't block the job on a + * transient DB read) + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +const { mockGetEditorialChannel } = vi.hoisted(() => ({ + mockGetEditorialChannel: vi.fn(), +})); + +vi.mock('../../server/src/db/system-settings-db.js', () => ({ + getEditorialChannel: (...args: unknown[]) => mockGetEditorialChannel(...args), +})); + +// The module-under-test imports slack/client + visual + drafter + DB +// client transitively. We stub everything we don't need so module init +// is a no-op. +vi.mock('../../server/src/db/client.js', () => ({ + query: vi.fn(), + getPool: () => ({ connect: async () => ({ query: vi.fn(), release: () => {} }) }), +})); +vi.mock('../../server/src/slack/client.js', () => ({ + sendChannelMessage: vi.fn(), + deleteChannelMessage: vi.fn(), +})); +vi.mock('../../server/src/services/announcement-drafter.js', () => ({ + draftAnnouncement: vi.fn(), +})); +vi.mock('../../server/src/services/announcement-visual.js', () => ({ + resolveAnnouncementVisual: vi.fn(), + isSafeVisualUrl: () => true, + AAO_FALLBACK_VISUAL_URL: '', +})); + +const ORIGINAL_ENV = process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; + +beforeEach(() => { + vi.clearAllMocks(); + delete process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; +}); + +afterEach(() => { + if (ORIGINAL_ENV === undefined) delete process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; + else process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = ORIGINAL_ENV; +}); + +describe('resolveEditorialChannel', () => { + it('returns the DB setting when configured (preferred over env)', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ + channel_id: 'C0FROMDB01', + channel_name: 'admin-editorial-review', + }); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = 'C0FROMENV01'; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + const resolved = await resolveEditorialChannel(); + expect(resolved).toBe('C0FROMDB01'); + }); + + it('falls back to env var when the DB setting is null', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ channel_id: null, channel_name: null }); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = 'C0FROMENV01'; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBe('C0FROMENV01'); + }); + + it('returns null when both DB and env are unset', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ channel_id: null, channel_name: null }); + delete process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBeNull(); + }); + + it('treats empty string env as unset', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ channel_id: null, channel_name: null }); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = ''; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBeNull(); + }); + + it('treats whitespace-only env as unset', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ channel_id: null, channel_name: null }); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = ' '; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBeNull(); + }); + + it('trims whitespace from env fallback', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ channel_id: null, channel_name: null }); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = ' C0FROMENV01 '; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBe('C0FROMENV01'); + }); + + it('falls back to env when the DB read throws (transient failures should not block the job)', async () => { + mockGetEditorialChannel.mockRejectedValueOnce(new Error('db connection reset')); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = 'C0FROMENV01'; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBe('C0FROMENV01'); + }); + + it('returns null when DB throws and env unset (safe no-op for the caller)', async () => { + mockGetEditorialChannel.mockRejectedValueOnce(new Error('db connection reset')); + delete process.env.SLACK_EDITORIAL_REVIEW_CHANNEL; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBeNull(); + }); + + it('treats whitespace-only DB channel_id as unset (symmetric with env)', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ + channel_id: ' ', + channel_name: 'whatever', + }); + process.env.SLACK_EDITORIAL_REVIEW_CHANNEL = 'C0FROMENV01'; + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBe('C0FROMENV01'); + }); + + it('trims whitespace from DB channel_id', async () => { + mockGetEditorialChannel.mockResolvedValueOnce({ + channel_id: ' C0FROMDB01 ', + channel_name: 'editorial', + }); + + const { resolveEditorialChannel } = await import('../../server/src/addie/jobs/announcement-trigger.js'); + expect(await resolveEditorialChannel()).toBe('C0FROMDB01'); + }); +});