Skip to content

Commit b9a8d49

Browse files
Auto-submit review fixes: safe modal lifecycle, resilient portal drafts
pr-review findings: - The modal clears the Turnstile token after EVERY finalize attempt: the server verifies the captcha before any other check, so a 409/422 consumed the single-use token and every retry then failed at Cloudflare with the button still enabled — a permanent dead-end. - The modal only renders when the prompt id matches the map on screen: the store is module-global, so a stale id could finalize (publish) a map the user was no longer looking at. - The Save & Share entry point is gated on ready_to_share — finalize hard-requires it, so the button previously walked users into a guaranteed 409 (and a burned captcha). - The abbreviated form renders through FormField: the hand-rolled inputs dropped Select options (free-text states break the gallery's exact-match state filter), zip pattern anchoring, label association, and require_email_confirm entirely. - create_document degrades a missing FormConfig to a normal map (log + no draft) instead of 404ing the whole creation — a portal page can outlive its config; test updated to pin the new decision, and a CMS test pins the negative injection case (config-less portals get no portalId on their create buttons). - Moderation queue defaults to status=submitted: draft rows point at the author's LIVE working map, which nobody consented to share yet. - Draft registry hardening: shape-check on read, quota-safe writes (same policy as session.ts), finalized entries pruned. - Dead handleCreateBlankMetadataObject deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent cc063b9 commit b9a8d49

8 files changed

Lines changed: 146 additions & 67 deletions

File tree

app/src/app/components/MapPage/SubmitToPortalModal.tsx

Lines changed: 55 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
'use client';
22
import {useEffect, useMemo, useState} from 'react';
3-
import {Blockquote, Box, Button, Checkbox, Dialog, Flex, Text, TextArea} from '@radix-ui/themes';
3+
import {Blockquote, Button, Checkbox, Dialog, Flex, Text} from '@radix-ui/themes';
44
import {useMapStore} from '@/app/store/mapStore';
5+
import {useFormState} from '@/app/store/formState';
56
import {useDraftSubmissionStore} from '@/app/store/draftSubmissionStore';
67
import {getDraftSubmission, updateDraftSubmission} from '@/app/utils/draftSubmissions';
78
import {
@@ -10,6 +11,7 @@ import {
1011
type FormConfigPublic,
1112
} from '@/app/utils/api/apiHandlers/postSubmission';
1213
import {FIELD_ORDER, FIELD_REGISTRY} from '@/app/components/Forms/fieldRegistry';
14+
import {FormField} from '@/app/components/Forms/FormField';
1315
import {useTurnstile} from '@/app/hooks/useTurnstile';
1416

1517
const EMAIL_RE = /^[^@\s]+@[^@\s]+\.[^@\s]+$/;
@@ -26,10 +28,13 @@ export const SubmitToPortalModal: React.FC = () => {
2628
const promptDocumentId = useDraftSubmissionStore(state => state.promptDocumentId);
2729
const closePrompt = useDraftSubmissionStore(state => state.closePrompt);
2830
const setNotification = useMapStore(state => state.setNotification);
31+
const currentDocumentId = useMapStore(state => state.mapDocument?.document_id);
32+
const setCaptchaToken = useFormState(state => state.setCaptchaToken);
2933

3034
const draft = useMemo(() => getDraftSubmission(promptDocumentId), [promptDocumentId]);
3135
const [config, setConfig] = useState<FormConfigPublic | null>(null);
3236
const [values, setValues] = useState<Record<string, string>>({});
37+
const [emailConfirm, setEmailConfirm] = useState('');
3338
const [acknowledged, setAcknowledged] = useState(false);
3439
const [isSubmitting, setIsSubmitting] = useState(false);
3540
const [error, setError] = useState('');
@@ -38,29 +43,40 @@ export const SubmitToPortalModal: React.FC = () => {
3843
useEffect(() => {
3944
setConfig(null);
4045
setValues({});
46+
setEmailConfirm('');
4147
setAcknowledged(false);
4248
setError('');
4349
if (!draft) return;
4450
getFormConfig(draft.portalId).then(response => {
4551
if (response.ok) setConfig(response.response);
4652
else setError('Could not load the portal form. Please try again later.');
4753
});
54+
// eslint-disable-next-line react-hooks/exhaustive-deps
4855
}, [draft?.portalId, promptDocumentId]);
4956

50-
if (!promptDocumentId || !draft || draft.submitted) return null;
57+
// The prompt id must match the map on screen: the store is module-global,
58+
// so a stale id from a previous map would otherwise finalize (publish) a
59+
// map the user is no longer looking at.
60+
if (!promptDocumentId || promptDocumentId !== currentDocumentId || !draft || draft.submitted) {
61+
return null;
62+
}
5163

5264
const requiredFields = FIELD_ORDER.filter(name => config?.required_fields?.includes(name));
65+
const needsEmailConfirm = !!config?.require_email_confirm && requiredFields.includes('email');
66+
const fieldIsValid = (name: string) => {
67+
const value = (values[name] ?? '').trim();
68+
if (!value) return false;
69+
const spec = FIELD_REGISTRY[name];
70+
if (name === 'email') return EMAIL_RE.test(value);
71+
if (spec.pattern && !new RegExp(`^(?:${spec.pattern})$`).test(value)) return false;
72+
return !spec.validator || spec.validator(value);
73+
};
5374
const isValid =
5475
!!config &&
5576
acknowledged &&
5677
!!captchaToken &&
57-
requiredFields.every(name => {
58-
const value = (values[name] ?? '').trim();
59-
if (!value) return false;
60-
if (name === 'email') return EMAIL_RE.test(value);
61-
const spec = FIELD_REGISTRY[name];
62-
return !spec.validator || spec.validator(value);
63-
});
78+
requiredFields.every(fieldIsValid) &&
79+
(!needsEmailConfirm || emailConfirm === values['email']);
6480

6581
const dismiss = () => {
6682
updateDraftSubmission(promptDocumentId, {suppressed: true});
@@ -75,6 +91,11 @@ export const SubmitToPortalModal: React.FC = () => {
7591
tags: [],
7692
turnstile_token: captchaToken,
7793
});
94+
// Turnstile tokens are single-use: the server verifies the captcha
95+
// BEFORE any other check, so even a 409/422 consumed it. Clearing the
96+
// token re-renders the widget; without this every retry fails at
97+
// Cloudflare with the button still enabled.
98+
setCaptchaToken('');
7899
setIsSubmitting(false);
79100
if (response.ok) {
80101
updateDraftSubmission(promptDocumentId, {submitted: true});
@@ -86,7 +107,7 @@ export const SubmitToPortalModal: React.FC = () => {
86107
type: 'success',
87108
});
88109
} else {
89-
setError(response.error);
110+
setError(response.error || 'Something went wrong — please try again.');
90111
}
91112
};
92113

@@ -106,26 +127,32 @@ export const SubmitToPortalModal: React.FC = () => {
106127
<Flex direction="column" gap="3" mt="3">
107128
{requiredFields.map(name => {
108129
const spec = FIELD_REGISTRY[name];
109-
const isTextArea = spec.component === TextArea;
110130
return (
111131
<Flex key={name} direction="column" gap="1">
112-
<Text as="label" size="2" weight="medium">
113-
{spec.label} *
114-
</Text>
115-
{isTextArea ? (
116-
<TextArea
117-
value={values[name] ?? ''}
118-
placeholder={spec.label}
119-
onChange={e => setValues(v => ({...v, [name]: e.target.value}))}
120-
/>
121-
) : (
122-
<input
123-
className="rt-TextFieldInput rt-r-size-2 border border-slate-300 rounded p-2"
124-
type={spec.type ?? 'text'}
125-
value={values[name] ?? ''}
126-
placeholder={spec.label}
127-
autoComplete={spec.autoComplete}
128-
onChange={e => setValues(v => ({...v, [name]: e.target.value}))}
132+
<FormField
133+
name={`portal_${name}`}
134+
label={`${spec.label} *`}
135+
type={spec.type}
136+
component={spec.component}
137+
options={spec.options}
138+
autoComplete={spec.autoComplete}
139+
pattern={spec.pattern}
140+
validator={spec.validator}
141+
invalidMessage={spec.invalidMessage}
142+
required
143+
value={values[name] ?? ''}
144+
onChangeValue={value => setValues(v => ({...v, [name]: value}))}
145+
/>
146+
{name === 'email' && needsEmailConfirm && (
147+
<FormField
148+
name="portal_email_confirm"
149+
label="Confirm Email *"
150+
type="email"
151+
required
152+
value={emailConfirm}
153+
onChangeValue={setEmailConfirm}
154+
validator={value => value === values['email']}
155+
invalidMessage="Email addresses must match"
129156
/>
130157
)}
131158
</Flex>

app/src/app/components/Toolbar/SaveShareModal/SaveShareModal.tsx

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {ShareMapSection} from './ShareMapSection';
77
import {useSaveShareStore} from '@/app/store/saveShareStore';
88
import {Link1Icon} from '@radix-ui/react-icons';
99
import {useMapMetadata} from '@/app/hooks/useMapMetadata';
10+
import {DRAFT_STATUSES} from '@/app/constants/document/draftStatus';
1011
import {useEditableDocId} from '@/app/hooks/useEditableDocId';
1112
import {DEFAULT_MAP_METADATA} from '@/app/utils/language';
1213
import {routeForType} from '@constants/document/routes';
@@ -116,22 +117,27 @@ export const SaveShareModal: React.FC<{
116117
/>
117118
<hr className="my-4" />
118119
<ShareMapSection isEditing={isEditing} />
119-
{isEditing && draftSubmission && !draftSubmission.submitted && (
120-
<Button
121-
variant="soft"
122-
color="violet"
123-
size="3"
124-
className="mt-2"
125-
onClick={() => {
126-
if (mapDocument?.document_id) {
127-
onClose();
128-
openSubmitPrompt(mapDocument.document_id);
129-
}
130-
}}
131-
>
132-
Submit to the {draftSubmission.portalId} portal
133-
</Button>
134-
)}
120+
{isEditing &&
121+
draftSubmission &&
122+
!draftSubmission.submitted &&
123+
// Finalize hard-requires ready_to_share server-side; offering the
124+
// modal earlier guarantees a 409 (and burns a captcha token).
125+
mapMetadata?.draft_status === DRAFT_STATUSES.READY_TO_SHARE && (
126+
<Button
127+
variant="soft"
128+
color="violet"
129+
size="3"
130+
className="mt-2"
131+
onClick={() => {
132+
if (mapDocument?.document_id) {
133+
onClose();
134+
openSubmitPrompt(mapDocument.document_id);
135+
}
136+
}}
137+
>
138+
Submit to the {draftSubmission.portalId} portal
139+
</Button>
140+
)}
135141
{isEditing ? (
136142
<Flex direction="column" gap="2" className="mt-4">
137143
<Flex direction="row" gap="2" justify="between">

app/src/app/utils/draftSubmissions.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,29 @@ const readAll = (): DraftSubmissionMap => {
2727
};
2828

2929
const writeAll = (all: DraftSubmissionMap) => {
30-
window.localStorage.setItem(STORAGE_KEY, JSON.stringify(all));
30+
try {
31+
window.localStorage.setItem(STORAGE_KEY, JSON.stringify(all));
32+
} catch {
33+
// ignore storage errors (private mode, quota) — same policy as
34+
// utils/api/session.ts; the draft flow degrades to "no prompt".
35+
}
3136
};
3237

33-
export const getDraftSubmission = (documentId?: string | null): DraftSubmission | null =>
34-
(documentId && readAll()[documentId]) || null;
38+
export const getDraftSubmission = (documentId?: string | null): DraftSubmission | null => {
39+
if (!documentId) return null;
40+
const entry = readAll()[documentId];
41+
// Shape-check: a corrupt/legacy value would otherwise flow into
42+
// getFormConfig(undefined) and dead-end the modal with no way to clear it.
43+
return entry && typeof entry === 'object' && entry.submissionId && entry.portalId ? entry : null;
44+
};
3545

3646
export const setDraftSubmission = (documentId: string, draft: DraftSubmission) => {
37-
writeAll({...readAll(), [documentId]: draft});
47+
const all = readAll();
48+
// Prune finalized entries — the registry is otherwise append-only.
49+
for (const [key, entry] of Object.entries(all)) {
50+
if (entry?.submitted) delete all[key];
51+
}
52+
writeAll({...all, [documentId]: draft});
3853
};
3954

4055
export const updateDraftSubmission = (documentId: string, updates: Partial<DraftSubmission>) => {

app/src/app/utils/metadata/handleCreateBlankMetadataObject.ts

Lines changed: 0 additions & 13 deletions
This file was deleted.

backend/app/main.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,18 @@ async def create_document(
445445
# uses to finalize (submit to the portal's gallery).
446446
draft_submission_id: str | None = None
447447
if data.portal_id is not None:
448-
# Reuse the submissions module's resolver (single source of the
449-
# portal-validity rule and its 404 message).
450-
submissions.get_form_config(data.portal_id, session)
448+
# portal_id is advisory metadata: a portal page can outlive its
449+
# FormConfig (config deleted, cached page), and losing the draft
450+
# must degrade to "a normal map", never "no map".
451+
try:
452+
submissions.get_form_config(data.portal_id, session)
453+
except HTTPException:
454+
logger.warning(
455+
f"create_document: no form config for portal {data.portal_id!r}; "
456+
"creating the map without a draft submission"
457+
)
458+
data.portal_id = None
459+
if data.portal_id is not None:
451460
draft = Submission(
452461
portal_id=data.portal_id,
453462
map_public_id=new_document.public_id,

backend/tests/test_submissions.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,12 +403,24 @@ def test_create_document_with_portal_creates_draft(
403403
# Drafts are invisible publicly and never in the gallery.
404404
assert client.get(f"/api/submissions?portal_id={PORTAL}").json() == []
405405

406-
def test_create_document_with_unknown_portal_404(self, client, form_config):
406+
def test_create_document_with_unknown_portal_degrades(
407+
self, client, form_config, session
408+
):
409+
# portal_id is advisory: a portal page can outlive its FormConfig, so
410+
# a missing config must degrade to a normal map (no draft), never
411+
# abort document creation on a live portal page.
407412
response = client.post(
408413
"/api/create_document",
409414
json={"districtr_map_slug": GERRY_DB_FIXTURE_NAME, "portal_id": "nope"},
410415
)
411-
assert response.status_code == 404
416+
assert response.status_code == 201, response.json()
417+
assert response.json().get("submission_id") is None
418+
assert (
419+
session.exec(
420+
select(Submission).where(col(Submission.portal_id) == "nope")
421+
).first()
422+
is None
423+
)
412424

413425
def test_unknown_capability_404(self, client, form_config):
414426
response = self._finalize(client, "00000000-0000-0000-0000-000000000000")

cms/content/tests.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,6 +1248,26 @@ def test_portal_without_config_serves_null_fields(self):
12481248
value = self._form_block("bare")
12491249
self.assertIsNone(value["fields"])
12501250

1251+
def test_map_create_buttons_without_config_get_no_portal_id(self):
1252+
# The decision that matters: a config-less portal must NOT stamp
1253+
# portalId onto its create buttons — that key is what makes
1254+
# create_document mint a draft, and the backend logs-and-degrades
1255+
# only because the CMS normally withholds it here.
1256+
from core.testing import make_portal
1257+
1258+
bare = make_portal("bare-buttons")
1259+
bare.body = [
1260+
{"type": "map_create_buttons", "value": {"views": [], "type": "simple"}}
1261+
]
1262+
bare.save_revision(clean=False).publish()
1263+
payload = self.client.get("/api/content/tags/slug/bare-buttons").json()
1264+
buttons = next(
1265+
block["value"]
1266+
for block in payload["content"]["body"]
1267+
if block["type"] == "map_create_buttons"
1268+
)
1269+
self.assertNotIn("portalId", buttons)
1270+
12511271

12521272
# ---------------------------------------------------------------------------
12531273
# Portal wizard

cms/moderation/views.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,10 @@ def portal_review(request, slug):
154154
if kind not in ("comments", "maps"):
155155
kind = "comments"
156156

157-
form = SubmissionFilterForm(request.GET or {})
157+
# Default to submitted: draft rows point at the author's LIVE working
158+
# map, which no one has consented to share yet — reviewers opt in to
159+
# seeing drafts explicitly.
160+
form = SubmissionFilterForm(request.GET or {"status": "submitted"})
158161

159162
def fetch(user, **params):
160163
params["portal_id"] = portal.slug

0 commit comments

Comments
 (0)