m/fdbck
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: server-side wiring through the questions registry — closes the gap

Browse Source 
Flips three callers from inline per-type strips to registry dispatch.
THE main payoff: the date_ranked_choice required-validation gap that the
audit doc flagged is now closed by construction.

submit/+server.ts:

- Replace the inline empty-check loop with
  `getQuestion(q.type).isAnswerEmpty(q, body.answers[q.id])`.
- The legacy rule matched only string/array empties; date_ranked_choice's
  `{}` answer (object exists but no options rated) passed even when the
  question was required. ONE source of truth, two callers (client +
  server), no drift possible.

results.ts:

- aggregateResults walks the form definition and routes each question
  through `getQuestion(q.type).{emptyStats, ingest, finalise}`.
- All seven per-type emptyStats / ingest / finalise functions deleted
  (~150 lines).
- publicResults delegates to each module's sanitizeForPublic. The
  hardcoded text-strip rule moves into short_text/long_text modules.
- ScaleStatsWip / DateRankedOptionStatsWip private types deleted —
  each module now owns its own accumulator shape.

export/+server.ts:

- Replace the per-type CSV column expansion (the date_ranked_choice
  one-column-per-option special case + the otherwise-one-column-per-question
  fallthrough) with a uniform `getQuestion(q.type).csvColumns(q)` walk.
- Replace the per-cell answer extraction with
  `getQuestion(q.type).csvCellFor(q, answer, col)`.

schemas.ts:

- Per-question-type schemas deleted (~40 lines). FeedbackQuestionSchema
  imports each module's schema directly and assembles the discriminated
  union. Adding a new type means: create lib/questions/<type>.ts +
  register in registry.ts + add one import + one tuple entry here.
  (Kept the explicit tuple because TypeScript can't infer a properly-
  narrowed discriminated union from a runtime-built array — the inferred
  FeedbackQuestion type would lose specificity at every call site.)

123 server tests pass — all existing assertions still hold against the
registry-routed aggregator. svelte-check + bun run build clean.

Client-side wiring (FormBuilder, participant page, Results.svelte, admin
detail submissions table) lands in the next commit.
This commit is contained in:
mAi
2026-05-07 20:26:49 +02:00
parent 1ab6ef7f22
commit 5fe4b417bf
4 changed files with 83 additions and 259 deletions
Download Patch File Download Diff File Expand all files Collapse all files

83
src/lib/schemas.ts
View File

@@ -1,66 +1,37 @@
/**
* Zod schemas for fdbck request body validation.
*
* The per-question-type schemas now live in `lib/questions/<type>.ts`. This
* file imports them directly and assembles the discriminated union — adding
* a new question type means creating one file under `lib/questions/`,
* registering it in `lib/questions/registry.ts`, AND adding one line here
* (TypeScript's discriminated-union inference can't pick up a runtime-built
* array, so the tuple stays explicit to keep `FeedbackQuestion` properly
* narrowed at the call sites).
*/
import { z } from 'zod';
import { ShortTextQuestionSchema } from './questions/short_text';
import { LongTextQuestionSchema } from './questions/long_text';
import { SingleChoiceQuestionSchema } from './questions/single_choice';
import { MultiChoiceQuestionSchema } from './questions/multi_choice';
import { ScaleQuestionSchema } from './questions/scale';
import { BooleanQuestionSchema } from './questions/boolean';
import {
DateRankedChoiceQuestionSchema,
DateRankedOptionSchema as PerTypeDateRankedOptionSchema,
} from './questions/date_ranked_choice';
const FeedbackQuestionBaseSchema = z.object({
id: z.string().min(1).max(64),
label: z.string().min(1).max(200),
required: z.boolean().optional(),
help: z.string().max(500).optional(),
});
/** One date/time option in a `date_ranked_choice` question. Times are stored as UTC ISO 8601 strings. */
export const DateRankedOptionSchema = z.object({
id: z.string().min(1).max(64).regex(/^[a-zA-Z0-9_-]+$/, {
message: 'option id may only contain letters, digits, "-" and "_"',
}),
start: z.string().datetime({ offset: true }),
end: z.string().datetime({ offset: true }).optional(),
label: z.string().max(200).optional(),
});
/** Re-exported for callers that need the option shape standalone. */
export const DateRankedOptionSchema = PerTypeDateRankedOptionSchema;
export const FeedbackQuestionSchema = z.discriminatedUnion('type', [
FeedbackQuestionBaseSchema.extend({
type: z.literal('short_text'),
placeholder: z.string().max(100).optional(),
}),
FeedbackQuestionBaseSchema.extend({
type: z.literal('long_text'),
placeholder: z.string().max(100).optional(),
}),
FeedbackQuestionBaseSchema.extend({
type: z.literal('single_choice'),
options: z.array(z.string().min(1).max(200)).min(2).max(20),
}),
FeedbackQuestionBaseSchema.extend({
type: z.literal('multi_choice'),
options: z.array(z.string().min(1).max(200)).min(2).max(20),
}),
FeedbackQuestionBaseSchema.extend({
type: z.literal('scale'),
min: z.number().int().min(0).max(100),
max: z.number().int().min(1).max(100),
min_label: z.string().max(50).optional(),
max_label: z.string().max(50).optional(),
}),
FeedbackQuestionBaseSchema.extend({
type: z.literal('boolean'),
}),
FeedbackQuestionBaseSchema.extend({
type: z.literal('date_ranked_choice'),
options: z.array(DateRankedOptionSchema).min(2).max(50)
.refine(
(opts) => new Set(opts.map((o) => o.id)).size === opts.length,
{ message: 'date_ranked_choice option ids must be unique' },
),
// Scale is locked at 1-5 (5-point Likert) per design — only the labels are author-configurable.
scale: z.object({
min_label: z.string().max(50).optional(),
max_label: z.string().max(50).optional(),
}).optional(),
allow_partial: z.boolean().optional(),
}),
ShortTextQuestionSchema,
LongTextQuestionSchema,
SingleChoiceQuestionSchema,
MultiChoiceQuestionSchema,
ScaleQuestionSchema,
BooleanQuestionSchema,
DateRankedChoiceQuestionSchema,
]);
/** Version stamp like `0.260505` (YYMMDD) or `0.260505.b` for same-day re-edits. */

216
src/lib/server/results.ts
View File

@@ -3,8 +3,15 @@
*
* Aggregations are computed from the form_snapshot stored on each submission,
* so historical results stay correct even after the form is later edited.
*
* The per-type aggregation logic (emptyStats / ingest / finalise / sanitize)
* lives in `$lib/questions/<type>.ts`. This file is now a thin dispatcher:
* `aggregateResults` walks the questions and routes each one through the
* registry. Adding a new question type means writing one module file — no
* edit here.
*/
import type { FeedbackFormDefinition, FeedbackQuestion } from '../schemas';
import { getQuestion } from '../questions/registry';
export interface ScaleStats {
type: 'scale';
@@ -81,200 +88,49 @@ export function aggregateResults(
current: FeedbackFormDefinition,
subs: SubmissionRow[],
): AggregatedResults {
const questions: QuestionResult[] = current.questions.map((q) => ({
id: q.id,
label: q.label,
type: q.type,
stats: emptyStats(q),
}));
const byId = new Map(questions.map((q) => [q.id, q]));
const questions: QuestionResult[] = current.questions.map((q) => {
const mod = getQuestion(q.type);
return {
id: q.id,
label: q.label,
type: q.type,
// Each module's emptyStats is typed by its own question variant; the
// top-level FeedbackQuestion[] has been narrowed by getQuestion's
// dispatch on `q.type` so the cast is sound.
stats: mod.emptyStats(q as never) as QuestionStats,
};
});
const qById = new Map(current.questions.map((q) => [q.id, q]));
const resultById = new Map(questions.map((q) => [q.id, q]));
for (const sub of subs) {
for (const q of questions) {
for (const q of current.questions) {
const v = sub.answers?.[q.id];
if (v === undefined || v === null) continue;
ingest(byId.get(q.id)!, current.questions.find((cq) => cq.id === q.id)!, v, sub.created_at);
const result = resultById.get(q.id)!;
getQuestion(q.type).ingest(result.stats as never, q as never, v, sub.created_at);
}
}
for (const q of questions) finalise(q.stats);
for (const q of questions) {
const def = qById.get(q.id);
if (!def) continue;
getQuestion(def.type).finalise(q.stats as never);
}
return { total_submissions: subs.length, questions };
}
// Aggregation-time accumulator types. The `_sum` field is a real, typed
// member during ingest; finalise reads it and drops it before returning the
// public stats shape. Cleaner than the previous (s as X & { _sum?: number })
// inline-cast-and-coalesce pattern.
type ScaleStatsWip = ScaleStats & { _sum: number };
type DateRankedOptionStatsWip = DateRankedOptionStats & { _sum: number };
function emptyStats(q: FeedbackQuestion): QuestionStats {
switch (q.type) {
case 'scale': {
const wip: ScaleStatsWip = {
type: 'scale',
count: 0,
min: q.min,
max: q.max,
mean: null,
histogram: Array.from({ length: q.max - q.min + 1 }, (_, i) => ({
value: q.min + i,
count: 0,
})),
_sum: 0,
};
return wip;
}
case 'single_choice':
case 'multi_choice':
return {
type: q.type,
count: 0,
options: q.options.map((option) => ({ option, count: 0 })),
other_count: 0,
};
case 'boolean':
return { type: 'boolean', count: 0, yes: 0, no: 0 };
case 'short_text':
case 'long_text':
return { type: q.type, count: 0, answers: [] };
case 'date_ranked_choice':
return {
type: 'date_ranked_choice',
count: 0,
options: q.options.map((opt) => {
const wip: DateRankedOptionStatsWip = {
id: opt.id,
start: opt.start,
end: opt.end ?? null,
label: opt.label ?? null,
count: 0,
mean: null,
histogram: [1, 2, 3, 4, 5].map((value) => ({ value, count: 0 })),
_sum: 0,
};
return wip;
}),
};
}
}
function ingest(
out: QuestionResult,
q: FeedbackQuestion,
v: unknown,
created_at: string,
): void {
const s = out.stats;
switch (s.type) {
case 'scale': {
if (typeof v !== 'number' || !Number.isFinite(v)) return;
const acc = s as ScaleStatsWip;
acc.count++;
const bucket = acc.histogram.find((b) => b.value === v);
if (bucket) bucket.count++;
acc._sum += v;
return;
}
case 'single_choice': {
if (typeof v !== 'string') return;
s.count++;
const hit = s.options.find((o) => o.option === v);
if (hit) hit.count++;
else s.other_count++;
return;
}
case 'multi_choice': {
if (!Array.isArray(v)) return;
if (v.length === 0) return;
s.count++;
for (const choice of v) {
if (typeof choice !== 'string') continue;
const hit = s.options.find((o) => o.option === choice);
if (hit) hit.count++;
else s.other_count++;
}
return;
}
case 'boolean': {
if (typeof v !== 'boolean') return;
s.count++;
if (v) s.yes++;
else s.no++;
return;
}
case 'short_text':
case 'long_text': {
if (typeof v !== 'string' || v.trim() === '') return;
s.count++;
s.answers.push({ value: v, created_at });
return;
}
case 'date_ranked_choice': {
if (!v || typeof v !== 'object' || Array.isArray(v)) return;
const ratings = v as Record<string, unknown>;
let touched = false;
for (const opt of s.options) {
const raw = ratings[opt.id];
if (raw === undefined || raw === null) continue;
if (typeof raw !== 'number' || !Number.isInteger(raw) || raw < 1 || raw > 5) continue;
const acc = opt as DateRankedOptionStatsWip;
acc.count++;
const bucket = acc.histogram.find((b) => b.value === raw);
if (bucket) bucket.count++;
acc._sum += raw;
touched = true;
}
if (touched) s.count++;
return;
}
}
void q; // q is the schema definition; ingest dispatches on s.type which
// is derived from it via emptyStats. Kept in the signature so a
// future per-type module can take the question for richer logic.
}
function finalise(s: QuestionStats): void {
if (s.type === 'scale') {
const acc = s as ScaleStatsWip;
s.mean = s.count > 0 ? acc._sum / s.count : null;
delete (s as Partial<ScaleStatsWip>)._sum;
return;
}
if (s.type === 'date_ranked_choice') {
for (const opt of s.options) {
const acc = opt as DateRankedOptionStatsWip;
opt.mean = opt.count > 0 ? acc._sum / opt.count : null;
delete (opt as Partial<DateRankedOptionStatsWip>)._sum;
}
// Sort by mean desc with tiebreaks: count of "5"s, then "4"s, then count desc, then id.
s.options.sort((a, b) => {
const am = a.mean ?? -Infinity;
const bm = b.mean ?? -Infinity;
if (am !== bm) return bm - am;
const a5 = a.histogram.find((h) => h.value === 5)?.count ?? 0;
const b5 = b.histogram.find((h) => h.value === 5)?.count ?? 0;
if (a5 !== b5) return b5 - a5;
const a4 = a.histogram.find((h) => h.value === 4)?.count ?? 0;
const b4 = b.histogram.find((h) => h.value === 4)?.count ?? 0;
if (a4 !== b4) return b4 - a4;
if (a.count !== b.count) return b.count - a.count;
return a.id.localeCompare(b.id);
});
}
}
/** Public-safe results: drop free-text answers (PII / open data). */
/** Public-safe results: drop free-text answers (PII / open data).
* Each module's sanitizeForPublic decides what survives — text types strip
* the answers list and keep only the count; everything else passes through. */
export function publicResults(r: AggregatedResults): AggregatedResults {
return {
total_submissions: r.total_submissions,
questions: r.questions.map((q) => {
if (q.stats.type === 'short_text' || q.stats.type === 'long_text') {
return { ...q, stats: { type: q.stats.type, count: q.stats.count, answers: [] } };
}
return q;
}),
questions: r.questions.map((q) => ({
...q,
stats: getQuestion(q.type).sanitizeForPublic(q.stats as never) as QuestionStats,
})),
};
}

27
src/routes/api/admin/feedback/[id]/export/+server.ts
View File

@@ -5,6 +5,8 @@ import { badRequest } from '$lib/server/errors';
import { fdb } from '$lib/server/fdb';
import { withOwnedInstance } from '$lib/server/admin-route';
import type { FeedbackFormDefinition } from '$lib/schemas';
import { getQuestion } from '$lib/questions/registry';
import type { CsvColumn } from '$lib/questions/types';
interface SubmissionRow {
id: string;
@@ -92,16 +94,14 @@ export const GET = withOwnedInstance(async ({ inst, event }) => {
const formDef = inst.form_definition as FeedbackFormDefinition | null;
const questions = formDef?.questions ?? [];
// Expand `date_ranked_choice` questions into one column per option (e.g. `kickoff_when[opt1]`).
// Other types stay as a single column keyed by question id.
const colSpecs: { qid: string; optId?: string; header: string }[] = [];
// Per-question column expansion. date_ranked_choice yields one column
// per option; everything else is a single column keyed by question id.
// The shape is owned by each module's csvColumns / csvCellFor.
const colSpecs: (CsvColumn & { question: typeof questions[number] })[] = [];
for (const q of questions) {
if (q.type === 'date_ranked_choice') {
for (const opt of q.options) {
colSpecs.push({ qid: q.id, optId: opt.id, header: `${q.id}[${opt.id}]` });
}
} else {
colSpecs.push({ qid: q.id, header: q.id });
const mod = getQuestion(q.type);
for (const col of mod.csvColumns(q)) {
colSpecs.push({ ...col, question: q });
}
}
@@ -109,13 +109,8 @@ export const GET = withOwnedInstance(async ({ inst, event }) => {
const subRows = submissions.map((row) => [
row.id, row.created_at, row.display_name, row.client_session_id,
...colSpecs.map((c) => {
const v = row.answers?.[c.qid];
if (c.optId) {
if (!v || typeof v !== 'object' || Array.isArray(v)) return '';
const r = (v as Record<string, unknown>)[c.optId];
return r === null || r === undefined ? '' : r;
}
return v ?? '';
const answer = row.answers?.[c.qid];
return getQuestion(c.question.type).csvCellFor(c.question, answer, c);
}),
]);
const submissionsCsv = rowsToCsv(subHeaders, subRows);

16
src/routes/api/public/feedback/[slug]/submit/+server.ts
View File

@@ -12,6 +12,7 @@ import {
findExistingSubmission,
isHoneypotTrap,
} from '$lib/server/feedback';
import { getQuestion } from '$lib/questions/registry';
import { checkRate } from '$lib/server/rate-limit';
import { fdb } from '$lib/server/fdb';
@@ -40,15 +41,16 @@ export const POST: RequestHandler = async ({ params, request, getClientAddress }
for (const id of Object.keys(body.answers)) {
if (!knownIds.has(id)) return badRequest(`Unknown question id: ${id}`);
}
// Required-question gate. Per-type isAnswerEmpty is THE source of truth —
// the legacy inline empty-check missed `date_ranked_choice` answers of
// shape `{}` (object exists but no options rated). The registry rule
// now closes that gap by construction.
for (const q of formDef.questions) {
if (q.required) {
const v = body.answers[q.id];
const empty =
v === undefined ||
v === null ||
(typeof v === 'string' && v.trim() === '') ||
(Array.isArray(v) && v.length === 0);
if (empty) return badRequest(`Missing answer for required question: ${q.id}`);
const mod = getQuestion(q.type);
if (mod.isAnswerEmpty(q, body.answers[q.id])) {
return badRequest(`Missing answer for required question: ${q.id}`);
}
}
}