refactor(dashboard): rethink settings, split out subreddits and accounts
The settings page had accumulated five tabs whose names no longer matched their contents, three different save models, and the whole subreddit configuration buried under a tab named after a checkbox. Structure: - Subreddits becomes its own page: a list showing each community's mode, tone, computed share of polling, and live sent/reply stats, with one subreddit edited at a time in a drawer. - Accounts becomes its own page. It is session lifecycle, not config. - Settings collapses to one scrolling page: Safety, Pacing (Monitoring and Rate limits merged), Notifications, Advanced. - The sidebar is grouped, having gone from 9 to 11 destinations. Saving: - Every section owns its own draft, validation, risk gate and save via useConfigCard, submitting only the keys it owns. A syntax error in the advanced JSON can no longer block a rate-limit change, and enabling an autonomous policy is its own deliberate commit. - UnsavedChangesProvider restores the leave-page guard across cards that no longer share a draft. Subreddit editing: - Errors are keyed by field name instead of array index, removing the defensive invalidation that existed because row indices shifted on add and remove. Confirming an autonomous mode can no longer be attributed to the wrong row. - Weight is shown as the share it actually produces, live while editing. - Bulk actions cover start/pause polling, tone, policy and removal, because production runs 30-40 subreddits and per-row editing does not scale. Bulk policy changes name every subreddit turning autonomous, and normalise the hybrid mode's turn budget, which has no form to catch it. Also surfaces the effective sending rate, since the hourly limit and the minimum delay both throttle and only the binding one matters. Legacy ?section= links redirect or resolve to anchors. Fixes found while verifying in a browser: a radio-card rule was shrinking nested number inputs; .share-preview had no styles; the throughput readout reported a rate from a blank field; and the reconcile logic warned about a server change whenever a sibling card saved. Tests: 116 files, 1105 passing (from 113, 1027). Adds subreddit, accounts and model unit suites covering share maths, bulk normalisation and the confirm-by-name behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7defff422d
commit
5a0846965a
35 files changed
+4129
-1530
No files matched your search
@@ -0,0 +1,280 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
createPacingDraft,
|
||||
estimateThroughput,
|
||||
pacingRisks,
|
||||
safetyRisks,
|
||||
validateAdvancedDraft,
|
||||
validatePacingDraft,
|
||||
type PacingDraft,
|
||||
} from '../../apps/dashboard/src/features/settings/settings-model.js';
|
||||
import {
|
||||
computeShares,
|
||||
formatShare,
|
||||
normaliseSubredditName,
|
||||
previewShare,
|
||||
validateSubredditDraft,
|
||||
createEmptySubredditDraft,
|
||||
type SubredditConfig,
|
||||
type SubredditDraft,
|
||||
} from '../../apps/dashboard/src/features/subreddits/subreddit-model.js';
|
||||
|
||||
function pacing(overrides: Partial<PacingDraft> = {}): PacingDraft {
|
||||
return {
|
||||
minDelayMinutes: '5',
|
||||
maxMessagesPerHour: '8',
|
||||
jitterSeconds: '60',
|
||||
prioritizeRecentPosts: false,
|
||||
recentWindowMinutes: '60',
|
||||
maxReadyCandidates: '25',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function subreddit(overrides: Partial<SubredditConfig> = {}): SubredditConfig {
|
||||
return {
|
||||
name: 'example',
|
||||
enabled: true,
|
||||
type: 'chat',
|
||||
tone: 'default',
|
||||
conversation: { mode: 'legacy', maxAutomatedTurns: 3, rapportTurnsBeforeMediaAsk: 3 },
|
||||
...overrides,
|
||||
} as SubredditConfig;
|
||||
}
|
||||
|
||||
function draft(overrides: Partial<SubredditDraft> = {}): SubredditDraft {
|
||||
return { ...createEmptySubredditDraft(), name: 'example', ...overrides };
|
||||
}
|
||||
|
||||
describe('createPacingDraft', () => {
|
||||
it('converts stored milliseconds into the units the operator types', () => {
|
||||
const result = createPacingDraft({
|
||||
rateLimit: { minDelayBetweenMessagesMs: 300_000, maxMessagesPerHour: 8, jitterMs: 90_000 },
|
||||
browser: { dryRun: true },
|
||||
reddit: { prioritizeRecentPosts: true, recentWindowMinutes: 45, maxReadyCandidates: 30 },
|
||||
subreddits: [],
|
||||
tones: [],
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
minDelayMinutes: '5',
|
||||
maxMessagesPerHour: '8',
|
||||
jitterSeconds: '90',
|
||||
prioritizeRecentPosts: true,
|
||||
recentWindowMinutes: '45',
|
||||
maxReadyCandidates: '30',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('validatePacingDraft', () => {
|
||||
it('converts back into milliseconds and omits keys it does not own', () => {
|
||||
const { payload, errors } = validatePacingDraft(pacing({ jitterSeconds: '30' }));
|
||||
|
||||
expect(errors).toEqual({});
|
||||
expect(payload).toEqual({
|
||||
rateLimit: { minDelayBetweenMessagesMs: 300_000, maxMessagesPerHour: 8, jitterMs: 30_000 },
|
||||
reddit: { prioritizeRecentPosts: false, recentWindowMinutes: 60, maxReadyCandidates: 25 },
|
||||
});
|
||||
});
|
||||
|
||||
it('collects every problem in one pass rather than one at a time', () => {
|
||||
const { payload, errors } = validatePacingDraft(pacing({
|
||||
minDelayMinutes: '',
|
||||
jitterSeconds: '-1',
|
||||
maxReadyCandidates: '0',
|
||||
}));
|
||||
|
||||
expect(payload).toBeUndefined();
|
||||
expect(Object.keys(errors).sort()).toEqual(['jitterSeconds', 'maxReadyCandidates', 'minDelayMinutes']);
|
||||
});
|
||||
|
||||
it('rejects a fractional hourly limit', () => {
|
||||
expect(validatePacingDraft(pacing({ maxMessagesPerHour: '2.5' })).errors)
|
||||
.toHaveProperty('maxMessagesPerHour');
|
||||
});
|
||||
});
|
||||
|
||||
describe('pacingRisks', () => {
|
||||
it('warns only when the limit both rises and clears the safe ceiling', () => {
|
||||
expect(pacingRisks(pacing({ maxMessagesPerHour: '21' }), pacing())).toHaveLength(1);
|
||||
expect(pacingRisks(pacing({ maxMessagesPerHour: '20' }), pacing())).toEqual([]);
|
||||
// Lowering an already-high limit is a de-escalation, not a risk.
|
||||
expect(pacingRisks(pacing({ maxMessagesPerHour: '30' }), pacing({ maxMessagesPerHour: '40' }))).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('safetyRisks', () => {
|
||||
it('warns only on the transition from dry run into live sending', () => {
|
||||
expect(safetyRisks({ dryRun: false }, { dryRun: true })).toHaveLength(1);
|
||||
expect(safetyRisks({ dryRun: true }, { dryRun: false })).toEqual([]);
|
||||
expect(safetyRisks({ dryRun: false }, { dryRun: false })).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('estimateThroughput', () => {
|
||||
it('reports the hourly cap when it binds before the delay', () => {
|
||||
const estimate = estimateThroughput(pacing({ minDelayMinutes: '5', jitterSeconds: '0' }));
|
||||
expect(estimate).toMatchObject({ effectivePerHour: 8, isDelayBound: false });
|
||||
});
|
||||
|
||||
it('reports the delay when it binds before the hourly cap', () => {
|
||||
const estimate = estimateThroughput(pacing({ minDelayMinutes: '30', jitterSeconds: '0' }));
|
||||
expect(estimate).toMatchObject({ effectivePerHour: 2, isDelayBound: true });
|
||||
});
|
||||
|
||||
it('counts average jitter as part of the spacing', () => {
|
||||
// 10 min delay plus 120s average jitter (half of 240) is 12 min spacing.
|
||||
const estimate = estimateThroughput(pacing({ minDelayMinutes: '10', jitterSeconds: '240' }));
|
||||
expect(estimate?.spacingLabel).toBe('about one every 12 min');
|
||||
expect(estimate?.effectivePerHour).toBe(5);
|
||||
});
|
||||
|
||||
it('gives up on unusable numbers rather than reporting nonsense', () => {
|
||||
expect(estimateThroughput(pacing({ minDelayMinutes: '' }))).toBeNull();
|
||||
expect(estimateThroughput(pacing({ maxMessagesPerHour: '0' }))).toBeNull();
|
||||
expect(estimateThroughput(pacing({ jitterSeconds: '-5' }))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateAdvancedDraft', () => {
|
||||
it('accepts a JSON object', () => {
|
||||
expect(validateAdvancedDraft({ advancedConfigJson: '{"a":1}' }).payload).toEqual({ advanced: { a: 1 } });
|
||||
});
|
||||
|
||||
it('names the syntax problem rather than just failing', () => {
|
||||
const { errors } = validateAdvancedDraft({ advancedConfigJson: '{"a":' });
|
||||
expect(errors.advancedConfigJson).toMatch(/^Not valid JSON — /);
|
||||
});
|
||||
|
||||
it('refuses arrays, bare values and an emptied editor', () => {
|
||||
expect(validateAdvancedDraft({ advancedConfigJson: '[1]' }).errors.advancedConfigJson).toMatch(/JSON object/);
|
||||
expect(validateAdvancedDraft({ advancedConfigJson: '"text"' }).errors.advancedConfigJson).toMatch(/JSON object/);
|
||||
expect(validateAdvancedDraft({ advancedConfigJson: ' ' }).errors.advancedConfigJson).toMatch(/cannot be emptied/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('computeShares', () => {
|
||||
it('splits attention across enabled subreddits in proportion to weight', () => {
|
||||
const shares = computeShares([
|
||||
subreddit({ name: 'a', weight: 3 }),
|
||||
subreddit({ name: 'b', weight: 1 }),
|
||||
]);
|
||||
|
||||
expect(shares.get('a')?.share).toBeCloseTo(0.75);
|
||||
expect(shares.get('b')?.share).toBeCloseTo(0.25);
|
||||
});
|
||||
|
||||
it('treats a missing weight as the runtime default of 1', () => {
|
||||
const shares = computeShares([subreddit({ name: 'a' }), subreddit({ name: 'b' })]);
|
||||
|
||||
expect(shares.get('a')).toEqual({ effectiveWeight: 1, share: 0.5 });
|
||||
});
|
||||
|
||||
it('gives disabled subreddits no share and excludes them from the total', () => {
|
||||
const shares = computeShares([
|
||||
subreddit({ name: 'a', weight: 1 }),
|
||||
subreddit({ name: 'off', weight: 99, enabled: false }),
|
||||
]);
|
||||
|
||||
expect(shares.get('off')?.share).toBe(0);
|
||||
expect(shares.get('a')?.share).toBe(1);
|
||||
});
|
||||
|
||||
it('reports zero rather than dividing by zero when everything is disabled', () => {
|
||||
const shares = computeShares([subreddit({ name: 'a', enabled: false })]);
|
||||
expect(shares.get('a')?.share).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatShare', () => {
|
||||
it('keeps tiny but non-zero shares visible', () => {
|
||||
expect(formatShare(0.004)).toBe('<1%');
|
||||
expect(formatShare(0)).toBe('—');
|
||||
expect(formatShare(0.256)).toBe('26%');
|
||||
});
|
||||
});
|
||||
|
||||
describe('previewShare', () => {
|
||||
const existing = [subreddit({ name: 'a', weight: 1 }), subreddit({ name: 'b', weight: 1 })];
|
||||
|
||||
it('excludes the edited row from the baseline so its old weight is not counted twice', () => {
|
||||
// Editing 'a' up to weight 3 leaves 'b' at 1, so 'a' takes three quarters.
|
||||
expect(previewShare(draft({ name: 'a', weight: '3' }), existing, 'a')).toBeCloseTo(0.75);
|
||||
});
|
||||
|
||||
it('adds a brand new row on top of the existing total', () => {
|
||||
expect(previewShare(draft({ name: 'c', weight: '2' }), existing, null)).toBeCloseTo(0.5);
|
||||
});
|
||||
|
||||
it('is zero for a disabled or unusable weight', () => {
|
||||
expect(previewShare(draft({ name: 'a', weight: '1', enabled: false }), existing, 'a')).toBe(0);
|
||||
expect(previewShare(draft({ name: 'a', weight: '-2' }), existing, 'a')).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('normaliseSubredditName', () => {
|
||||
it('strips the prefix operators paste in', () => {
|
||||
expect(normaliseSubredditName(' r/Example ')).toBe('Example');
|
||||
expect(normaliseSubredditName('/r/example')).toBe('example');
|
||||
expect(normaliseSubredditName('example')).toBe('example');
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateSubredditDraft', () => {
|
||||
it('keys errors by field name, with no array index to go stale', () => {
|
||||
const { errors } = validateSubredditDraft(draft({ name: '', weight: '0' }), []);
|
||||
expect(Object.keys(errors).sort()).toEqual(['name', 'weight']);
|
||||
});
|
||||
|
||||
it('rejects a duplicate name regardless of case', () => {
|
||||
expect(validateSubredditDraft(draft({ name: 'Example' }), ['example']).errors)
|
||||
.toHaveProperty('name', 'r/Example is already configured.');
|
||||
});
|
||||
|
||||
it('allows a name that only collides with the row being edited', () => {
|
||||
// The caller excludes the edited row from `takenNames`, so saving a
|
||||
// subreddit without renaming it is not a conflict.
|
||||
expect(validateSubredditDraft(draft({ name: 'example' }), []).errors).toEqual({});
|
||||
});
|
||||
|
||||
it('rejects names Reddit itself would not accept', () => {
|
||||
for (const name of ['a', 'has spaces', '_leading', 'way-too-long-for-reddit-to-accept']) {
|
||||
expect(validateSubredditDraft(draft({ name }), []).errors, name).toHaveProperty('name');
|
||||
}
|
||||
});
|
||||
|
||||
it('omits an empty weight and scripted message instead of sending blanks', () => {
|
||||
const { config } = validateSubredditDraft(draft({ weight: '', scripted: ' ' }), []);
|
||||
expect(config).not.toHaveProperty('weight');
|
||||
expect(config).not.toHaveProperty('scripted');
|
||||
});
|
||||
|
||||
it('normalises turn settings away for modes that do not use them', () => {
|
||||
const { config } = validateSubredditDraft(
|
||||
draft({ conversationMode: 'legacy', maxAutomatedTurns: '19', rapportTurnsBeforeMediaAsk: '17' }),
|
||||
[],
|
||||
);
|
||||
expect(config?.conversation).toEqual({ mode: 'legacy', maxAutomatedTurns: 3, rapportTurnsBeforeMediaAsk: 3 });
|
||||
});
|
||||
|
||||
it('requires rapport turns below the total budget for the hybrid mode', () => {
|
||||
const { errors } = validateSubredditDraft(
|
||||
draft({ conversationMode: 'r4r-media', maxAutomatedTurns: '4', rapportTurnsBeforeMediaAsk: '4' }),
|
||||
[],
|
||||
);
|
||||
expect(errors).toHaveProperty('rapportTurnsBeforeMediaAsk');
|
||||
});
|
||||
|
||||
it('rejects a turn budget above the configured maximum', () => {
|
||||
const { errors } = validateSubredditDraft(
|
||||
draft({ conversationMode: 'r4r', maxAutomatedTurns: '21' }),
|
||||
[],
|
||||
);
|
||||
expect(errors).toHaveProperty('maxAutomatedTurns');
|
||||
});
|
||||
|
||||
it('defaults a new subreddit to the mode that cannot send unattended', () => {
|
||||
expect(createEmptySubredditDraft().conversationMode).toBe('manual');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user