Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions app/portal/[eventSlug]/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,16 +274,27 @@ export async function saveSubmissionAction(_prev: FormState, formData: FormData)
const { ctx, me, eventSlug } = await actionSession(formData);
const submissionId = text(formData, 'submissionId');
const formId = text(formData, 'formId');
const fields = formId ? await submissionFields(formId) : [];

const mine = await listMySubmissions(me.id);
if (!mine.some((entry) => entry.id === submissionId)) throw notFound('That session');
const current = mine.find((entry) => entry.id === submissionId);
if (!current) throw notFound('That session');

/*
Only the questions this submission was asked are read back. Reading every question the form
holds records an empty answer to one that was never shown, which afterwards is
indistinguishable from a speaker who was asked and skipped it.
*/
const fields = formId ? await submissionFields(formId, current) : [];
await recordRevision(ctx, 'session', submissionId, 'Edited the session content');

await updateMySubmission(ctx, me.id, submissionId, {
title: text(formData, 'title'),
descriptionMarkdown: text(formData, 'descriptionMarkdown'),
level: text(formData, 'level'),
formatId: text(formData, 'formatId'),
trackId: text(formData, 'trackId'),
/* Checkboxes, so an empty list is "untag everything" and has to reach the service as one. */
tagIds: formData.getAll('tagIds').map((entry) => String(entry)),
answers: fields.length > 0 ? readAnswers(fields, formData) : undefined,
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
'use client';

import { useActionState, useState } from 'react';
import { Card, CardBody, CardHeader, CardTitle, Input, Select, Textarea } from '@/components/ui';
import {
Card,
CardBody,
CardHeader,
CardTitle,
Checkbox,
Input,
Select,
Textarea,
} from '@/components/ui';
import type { FormFieldSpec } from '@/lib/forms/contract';
import { renderMarkdown } from '@/lib/markdown';
import type { PortalSubmission } from '@/lib/services/portal';
import type { PortalSubmission, PortalTaxonomy } from '@/lib/services/portal';
import { IDLE_STATE } from '../../../form-state';
import styles from '../../../portal.module.css';
import { saveSubmissionAction, withdrawSubmissionAction } from '../../actions';
Expand All @@ -19,12 +28,15 @@ export function SubmissionEditor({
submission,
fields,
levelOptions,
taxonomy,
}: {
eventSlug: string;
submission: PortalSubmission;
fields: FormFieldSpec[];
/** What the form offers for Audience level, or `null` if it does not ask. */
levelOptions: string[] | null;
/** The event's formats, tracks and tags — each `null` where the form does not ask. */
taxonomy: PortalTaxonomy;
}) {
const [state, action] = useActionState(saveSubmissionAction, IDLE_STATE);
const [description, setDescription] = useState(submission.descriptionMarkdown ?? '');
Expand Down Expand Up @@ -118,6 +130,85 @@ export function SubmissionEditor({
<FieldError state={state} field="level" />
</div>
)}

{/*
Session format and Track. Both were asked on the way in and neither was editable
afterwards — Track appeared only as read-only header text — so a speaker who picked
the wrong one had no way to say so, on the two fields organizers filter and route on.
The options are the event's own lists, and the value is the id rather than the name,
because renaming a track must not detach the submissions that chose it.
*/}
{taxonomy.formats && (
<div className={styles.field}>
<label className={styles.label} htmlFor="formatId">
Session format
</label>
<Select
id="formatId"
name="formatId"
defaultValue={submission.formatId ?? ''}
invalid={Boolean(state.details?.formatId)}
>
<option value="">Not specified</option>
{taxonomy.formats.map((option) => (
<option key={option.id} value={option.id}>
{option.name}
</option>
))}
</Select>
<FieldError state={state} field="formatId" />
</div>
)}

{taxonomy.tracks && (
<div className={styles.field}>
<label className={styles.label} htmlFor="trackId">
Track
</label>
<Select
id="trackId"
name="trackId"
defaultValue={submission.trackId ?? ''}
invalid={Boolean(state.details?.trackId)}
>
<option value="">Not specified</option>
{taxonomy.tracks.map((option) => (
<option key={option.id} value={option.id}>
{option.name}
</option>
))}
</Select>
<FieldError state={state} field="trackId" />
</div>
)}

{/*
Checkboxes rather than a multi-select, which is how the public form asks it. An event
with the tags question but no tags yet gets the note instead of an empty box, so the
blank is explained rather than looking broken.
*/}
{taxonomy.tags && (
<div className={styles.field}>
<span className={styles.label}>Tags</span>
{taxonomy.tags.length === 0 ? (
<span className={styles.hint}>The organizers have not set up any tags yet.</span>
) : (
<div className={styles.choiceList}>
{taxonomy.tags.map((option) => (
<label key={option.id} className={styles.choice}>
<Checkbox
name="tagIds"
value={option.id}
defaultChecked={submission.tagIds.includes(option.id)}
/>
{option.name}
</label>
))}
</div>
)}
<FieldError state={state} field="tagIds" />
</div>
)}
</div>
</CardBody>
</Card>
Expand Down
7 changes: 5 additions & 2 deletions app/portal/[eventSlug]/submissions/[submissionId]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
listGroupMembers,
submissionFields,
submissionLevelOptions,
submissionTaxonomy,
type PortalSubmission,
} from '@/lib/services/portal';
import {
Expand Down Expand Up @@ -39,9 +40,10 @@ export default async function SubmissionDetailPage({
throw error;
}

const [fields, levelOptions, members] = await Promise.all([
submissionFields(submission.formId),
const [fields, levelOptions, taxonomy, members] = await Promise.all([
submissionFields(submission.formId, submission),
submissionLevelOptions(submission.formId),
submissionTaxonomy(submission.formId, event.id),
listGroupMembers(submission.id, me.id),
]);

Expand Down Expand Up @@ -142,6 +144,7 @@ export default async function SubmissionDetailPage({
submission={submission}
fields={fields}
levelOptions={levelOptions}
taxonomy={taxonomy}
/>
) : (
<Card>
Expand Down
12 changes: 10 additions & 2 deletions lib/forms/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,10 @@ export function evaluateCondition(condition: Condition, value: AnswerValue): boo
* Whether a field is currently on screen. Because conditions are one hop, this is a direct lookup
* against the referenced field's own answer — never a recursive walk, and never dependent on
* whether the referenced field is itself visible.
*
* A condition whose question is not in `byId` shows the field. That is the right answer for a rule
* left dangling by a deleted question, and the wrong one for a caller that handed over a filtered
* list — pass the whole form, then drop what you are not rendering.
*/
export function isFieldVisible(field: FormFieldSpec, values: AnswerMap, byId: Map<string, FormFieldSpec>): boolean {
if (!field.showIf) return true;
Expand All @@ -326,8 +330,12 @@ export function isFieldVisible(field: FormFieldSpec, values: AnswerMap, byId: Ma
return evaluateCondition(field.showIf, values[target.key] ?? null);
}

export function visibleFields(fields: FormFieldSpec[], values: AnswerMap): FormFieldSpec[] {
const byId = new Map(fields.map((field) => [field.id, field]));
/**
* Generic in the field, so a caller holding `RuntimeField`s gets `RuntimeField`s back rather than
* having to cast its resolved options away to ask what is on screen.
*/
export function visibleFields<T extends FormFieldSpec>(fields: T[], values: AnswerMap): T[] {
const byId = new Map<string, FormFieldSpec>(fields.map((field) => [field.id, field]));
return fields.filter((field) => isFieldVisible(field, values, byId));
}

Expand Down
160 changes: 160 additions & 0 deletions lib/services/asked-questions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import { describe, expect, it } from 'vitest';
import type { Condition, FormFieldSpec } from '../forms/contract';
import { askedAnswers } from './review';
import { askedQuestions, type AskedSource } from './submissions';

/*
`CFP-S2`. A form whose workshop question is gated on the built-in Session format, which is the
shape the evaluator built and the one the seeded demo happens not to have: its only condition
points at a custom field, so the bug never showed there.
*/

const FORMAT_ID = 'fmt-workshop';
const TALK_ID = 'fmt-talk';

function field(overrides: Partial<FormFieldSpec> & { id: string; key: string }): FormFieldSpec {
return {
builtinKey: null,
type: 'short_text',
label: overrides.key,
position: 0,
step: 1,
required: false,
options: null,
showIf: null,
minLength: null,
maxLength: null,
charLimitGroup: null,
...overrides,
};
}

const TITLE = field({ id: 'f-title', key: 'title', builtinKey: 'title', position: 0 });
const FORMAT = field({
id: 'f-format',
key: 'format',
builtinKey: 'format',
type: 'select',
options: [TALK_ID, FORMAT_ID],
position: 1,
});
const PREREQS = field({
id: 'f-prereqs',
key: 'prerequisites',
label: 'Workshop prerequisites',
type: 'long_text',
position: 2,
showIf: { fieldId: FORMAT.id, op: 'eq', value: FORMAT_ID } satisfies Condition,
});
const NOTES = field({ id: 'f-notes', key: 'notes', label: 'Anything else', position: 3 });

const FORM = [TITLE, FORMAT, PREREQS, NOTES];

function row(overrides: Partial<AskedSource> = {}): AskedSource {
return {
title: 'Taming 40-Minute CI',
descriptionMarkdown: 'A talk about builds.',
formatId: TALK_ID,
trackId: null,
level: null,
answers: {},
tagIds: [],
...overrides,
};
}

describe('askedQuestions', () => {
it('leaves out a question gated on a format the submission is not', () => {
const keys = askedQuestions(FORM, row()).map((entry) => entry.key);
expect(keys).toEqual(['title', 'format', 'notes']);
});

it('includes it once the submission is that format', () => {
const keys = askedQuestions(FORM, row({ formatId: FORMAT_ID })).map((entry) => entry.key);
expect(keys).toContain('prerequisites');
});

/* The regression itself: the gating value lives in a column, not in `answers`. */
it('reads the gating value from the column rather than from the answer map', () => {
const asked = askedQuestions(FORM, row({ formatId: FORMAT_ID, answers: {} }));
expect(asked.map((entry) => entry.key)).toContain('prerequisites');
});

it('resolves a condition on the built-in track the same way', () => {
const gated = field({
id: 'f-cfp',
key: 'cfp_notes',
position: 4,
showIf: { fieldId: 'f-track', op: 'eq', value: 'trk-infra' },
});
const track = field({ id: 'f-track', key: 'track', builtinKey: 'track', type: 'select' });
const form = [...FORM, track, gated];

expect(askedQuestions(form, row({ trackId: 'trk-infra' })).map((e) => e.key)).toContain(
'cfp_notes',
);
expect(askedQuestions(form, row({ trackId: 'trk-web' })).map((e) => e.key)).not.toContain(
'cfp_notes',
);
});

it('answers questions gated on a custom answer from the answer map', () => {
const parent = field({ id: 'f-recorded', key: 'prior_recording', type: 'select' });
const child = field({
id: 'f-link',
key: 'recording_link',
position: 5,
showIf: { fieldId: parent.id, op: 'eq', value: 'yes' },
});
const form = [...FORM, parent, child];

expect(
askedQuestions(form, row({ answers: { prior_recording: 'yes' } })).map((e) => e.key),
).toContain('recording_link');
expect(
askedQuestions(form, row({ answers: { prior_recording: 'no' } })).map((e) => e.key),
).not.toContain('recording_link');
});

it('keeps the order the form is in', () => {
expect(askedQuestions(FORM, row({ formatId: FORMAT_ID })).map((entry) => entry.key)).toEqual([
'title',
'format',
'prerequisites',
'notes',
]);
});
});

describe('askedAnswers', () => {
it('hides an answer stored against a question this submission was never asked', () => {
const answers = { prerequisites: '', notes: 'Please schedule me early.' };
expect(askedAnswers(answers, FORM, row())).toEqual({ notes: 'Please schedule me early.' });
});

it('shows it on a submission that was asked', () => {
const answers = { prerequisites: 'Bring a laptop.', notes: '' };
expect(askedAnswers(answers, FORM, row({ formatId: FORMAT_ID }))).toEqual(answers);
});

/*
Already-stored empties are the damage the write path did before the fix, so filtering has to be
at render — a row saved by the old portal keeps the key.
*/
it('hides a non-empty answer too, because a value there is stale rather than asked', () => {
const answers = { prerequisites: 'Left over from when this was a workshop.' };
expect(askedAnswers(answers, FORM, row())).toEqual({});
});

/* An answer with no question left to consult. The panel falls back to the raw key for these. */
it('keeps an answer whose question is no longer on the form', () => {
const answers = { retired_question: 'Answered before that question was deleted.' };
expect(askedAnswers(answers, FORM, row())).toEqual(answers);
});

it('leaves an unconditional questionnaire untouched', () => {
const form = [TITLE, NOTES];
const answers = { notes: 'No conditions anywhere on this form.' };
expect(askedAnswers(answers, form, row())).toEqual(answers);
});
});
Loading