Skip to content

Commit 4834847

Browse files
Frontend submissions review fixes: portal-scoped posting and galleries
pr-review findings: - submitForm filters the persisted fields record to the portal's own config: the store is shared across portals, so a leftover key from another portal 422'd every submit with an error about a field that has no input on the page — and the only recovery (Reset) destroyed the user's text. - Unparseable map links no longer submit silently with map_ref=null; the user is told the link couldn't be read. - require_email_confirm is actually enforced (checkValidity never saw the match rule, so 'confirm your email' accepted any non-empty text). - Portal pages' comment galleries are scoped to their portal: the CMS injects portalId into comment_gallery blocks and the gallery passes it through — an empty editor tags field listed EVERY portal's submissions, and user tag filters (OR semantics) widened it back. - Unknown config fields warn loudly instead of silently rendering an unsubmittable form (3-file registry lockstep drift). - 422 details render one message per error ('; '-joined) and FastAPI's object-shaped validation errors no longer print [object Object]. - Backend tests pin the loosened public list boundary: portal_id optional spans portals, ids narrows, and ids can NOT fish out hidden rows or drafts. - Dead code removed: the legacy FORM_ATTRIBUTES DOM plumbing (form blocks render via StreamRenderer's block.value spread), MapLink's unreachable zone badge, and the orphaned comment/commenter types. - zustand migrate() added so v0 drafts drop without a console error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 8ba29c1 commit 4834847

10 files changed

Lines changed: 111 additions & 84 deletions

File tree

app/src/app/components/Cms/RichTextEditor/extensions/CommentGallery/CommentGallery.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ function useDebouncedValue<T>(value: T, delay: number): T {
6262
/** Props for CommentGallery - matches attributes defined in CommentGalleryNode */
6363
export interface CommentGalleryProps {
6464
ids?: number[];
65+
/** Injected by the CMS on portal pages: scopes the gallery to one portal
66+
* regardless of tag filters (tags are OR'd and user-extendable). */
67+
portalId?: string;
6568
tags?: string[];
6669
place?: string;
6770
state?: string;
@@ -247,6 +250,7 @@ const FilterControls: React.FC<{
247250

248251
export const CommentGallery: React.FC<CommentGalleryProps> = ({
249252
ids,
253+
portalId,
250254
tags: initialTags,
251255
place: initialPlace,
252256
state: initialState,
@@ -288,6 +292,7 @@ export const CommentGallery: React.FC<CommentGalleryProps> = ({
288292
const filters: CommentFilters = useMemo(
289293
() => ({
290294
ids: ids,
295+
portalId: portalId,
291296
// Merge initial tags with user-added tags
292297
tags:
293298
initialTags || debouncedUserFilters.tags.length > 0
@@ -304,6 +309,7 @@ export const CommentGallery: React.FC<CommentGalleryProps> = ({
304309
}),
305310
[
306311
ids,
312+
portalId,
307313
initialTags,
308314
debouncedUserFilters,
309315
initialPlace,

app/src/app/components/Cms/RichTextEditor/extensions/CommentGallery/CommentGalleryRenderers.tsx

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ const ReportButton: React.FC<{submissionId: number}> = ({submissionId}) => {
6464
};
6565

6666
/** Map link component for comments with associated maps */
67-
const MapLink: React.FC<{publicId: number; zone?: number | null}> = ({publicId, zone}) => (
67+
const MapLink: React.FC<{publicId: number}> = ({publicId}) => (
6868
<a
6969
href={`/map/${publicId}`}
7070
target="_blank"
@@ -74,11 +74,6 @@ const MapLink: React.FC<{publicId: number; zone?: number | null}> = ({publicId,
7474
>
7575
<GlobeIcon className="w-4 h-4" />
7676
View Map
77-
{zone !== null && zone !== undefined && (
78-
<Badge size="1" color="blue" variant="soft">
79-
Zone {zone}
80-
</Badge>
81-
)}
8277
</a>
8378
);
8479

app/src/app/components/Forms/SubmissionForm.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,17 @@ export const SubmissionForm: React.FC<SubmissionFormProps> = ({
6969

7070
const required = new Set(requiredFields ?? []);
7171
const shown = FIELD_ORDER.filter(name => fields.includes(name));
72+
// Registry drift (a config field this frontend build doesn't know) would
73+
// otherwise render no input while the backend keeps requiring it — an
74+
// unsubmittable form with no visible cause.
75+
const unknown = fields.filter(name => !(name in FIELD_REGISTRY));
76+
if (unknown.length) {
77+
console.warn(
78+
`SubmissionForm: unknown field(s) in portal config: ${unknown.join(', ')} — ` +
79+
'update fieldRegistry.tsx (3-file lockstep with backend fields.py and ' +
80+
'the CMS SUBMISSION_FIELD_CHOICES).'
81+
);
82+
}
7283
const submissionFields = shown.filter(name => FIELD_REGISTRY[name].section === 'submission');
7384
const aboutFields = shown.filter(name => FIELD_REGISTRY[name].section === 'about');
7485

@@ -142,8 +153,13 @@ export const SubmissionForm: React.FC<SubmissionFormProps> = ({
142153
<form
143154
onSubmit={e => {
144155
e.preventDefault();
145-
if (captchaToken && formIsValid) {
146-
submitForm(portalId);
156+
// checkValidity() only sees native constraints, so the confirm
157+
// field's match rule must be enforced here — otherwise
158+
// require_email_confirm degrades to "type anything twice".
159+
const emailConfirmed =
160+
!requireEmailConfirm || (shown.includes('email') && emailConfirm === emailValue);
161+
if (captchaToken && formIsValid && emailConfirmed) {
162+
submitForm(portalId, shown);
147163
}
148164
}}
149165
ref={formRef}

app/src/app/components/RichTextRenderer/CustomRenderers/DomNodeRenderers.tsx

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {DOMNode} from 'html-react-parser';
22
import BoilerplateNodeRenderer from '../../Cms/RichTextEditor/extensions/Boilerplate/BoilerplateNodeRenderer';
33
import {ContentHeader} from '../../Static/ContentHeader';
4-
import {SubmissionForm} from '../../Forms/SubmissionForm';
54
import {
65
PlanGallery,
76
PlanGalleryProps,
@@ -19,7 +18,6 @@ import {
1918
RICH_TEXT_NODE_TYPES,
2019
BOILERPLATE_ATTRIBUTE_NAME,
2120
SECTION_HEADER_ATTRIBUTE_NAME,
22-
FORM_ATTRIBUTES,
2321
MAP_CREATE_BUTTONS_ATTRIBUTES,
2422
COMMENT_GALLERY_ATTRIBUTES,
2523
PLAN_GALLERY_ATTRIBUTES,
@@ -48,17 +46,6 @@ export const domNodeReplacers = (disabled: boolean) => {
4846
) as PlanGalleryProps;
4947
return <PlanGallery {...props} />;
5048
}
51-
case RICH_TEXT_NODE_TYPES.FORM: {
52-
const props = Object.fromEntries(
53-
FORM_ATTRIBUTES.map(attr => [
54-
attr.name,
55-
JSON.parse(domNode.attribs[attr.name] ?? 'null'),
56-
])
57-
);
58-
// Legacy embedded form nodes carry no form config; SubmissionForm
59-
// renders nothing without one (converted pages use stream blocks).
60-
return <SubmissionForm disabled={disabled} {...(props as any)} />;
61-
}
6249
case RICH_TEXT_NODE_TYPES.MAP_CREATE_BUTTONS: {
6350
const props = Object.fromEntries(
6451
MAP_CREATE_BUTTONS_ATTRIBUTES.map(attr => [

app/src/app/constants/cms.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,10 @@ export const RICH_TEXT_NODE_TYPES = {
1616
export const BOILERPLATE_ATTRIBUTE_NAME = 'data-custom-content';
1717
export const SECTION_HEADER_ATTRIBUTE_NAME = 'data-title';
1818

19-
export const FORM_ATTRIBUTES = [
20-
{name: 'mandatoryTags', default: []},
21-
{name: 'allowListModules', default: null},
22-
// Injected by the CMS content API from the portal's FormConfig
23-
// (cms/content/api.py::_inject_form_config).
24-
{name: 'portalId', default: null},
25-
{name: 'fields', default: null},
26-
{name: 'requiredFields', default: null},
27-
{name: 'requireEmailConfirm', default: false},
28-
] as const;
19+
// NOTE: form blocks render through StreamRenderer, which spreads the
20+
// CMS-injected block.value straight into SubmissionForm — there is no
21+
// DOM-attribute plumbing for them (the legacy FORM_ATTRIBUTES list was dead
22+
// code and was removed).
2923

3024
type MapCreateButtonsAttrSpec<K extends keyof MapCreateButtonsProps> = {
3125
name: K;

app/src/app/store/formState.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export interface FormState {
2222
/** The pasted/selected map link; parsed to a document ref at submit. */
2323
mapRef: string;
2424
setMapRef: (mapRef: string) => void;
25-
submitForm: (portalId: string) => Promise<void>;
25+
submitForm: (portalId: string, allowedFields: string[]) => Promise<void>;
2626
clear: () => void;
2727
error: string;
2828
setError: (error: string) => void;
@@ -106,7 +106,7 @@ export const useFormState = create<FormState>()(
106106
},
107107
error: '',
108108
success: '',
109-
submitForm: async (portalId: string) => {
109+
submitForm: async (portalId: string, allowedFields: string[]) => {
110110
const {
111111
clear,
112112
setIsSubmitting,
@@ -126,11 +126,27 @@ export const useFormState = create<FormState>()(
126126
set({error: 'Please acknowledge all statements', isSubmitting: false});
127127
return;
128128
}
129+
// The persisted store is shared across portals, so it can hold keys
130+
// this portal's config doesn't allow — posting them verbatim would
131+
// 422 with an error about a field that has no input on the page,
132+
// unrecoverably (clear() only runs on success).
133+
const allowed = new Set(allowedFields);
134+
const portalFields = Object.fromEntries(
135+
Object.entries(fields).filter(([key]) => allowed.has(key))
136+
);
137+
const mapRefParsed = showMapSelector ? parseMapRef(mapRef) : null;
138+
if (showMapSelector && mapRef.trim() && !mapRefParsed) {
139+
set({
140+
error: 'We could not read that map link — paste the share URL or map id.',
141+
isSubmitting: false,
142+
});
143+
return;
144+
}
129145
const response = await postSubmission({
130146
portal_id: portalId,
131-
fields,
147+
fields: portalFields,
132148
tags: Array.from(tags),
133-
map_ref: showMapSelector ? parseMapRef(mapRef) : null,
149+
map_ref: mapRefParsed,
134150
turnstile_token: captchaToken,
135151
});
136152
set({
@@ -182,7 +198,11 @@ export const useFormState = create<FormState>()(
182198
name: 'form-state',
183199
storage: createJSONStorage(() => localStorage),
184200
// v2: comment/commenter split replaced by the sparse fields record.
201+
// migrate: dropping v0/v1 drafts is deliberate (the shape changed);
202+
// without it zustand logs a scary console.error for returning
203+
// visitors while doing the same thing.
185204
version: 2,
205+
migrate: () => ({}),
186206
partialize: state => ({
187207
fields: state.fields,
188208
tags: state.tags,

app/src/app/utils/api/apiHandlers/postSubmission.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ export interface SubmissionCreated {
1717
type Result = {ok: true; data: SubmissionCreated} | {ok: false; error: string};
1818

1919
const formatError = (detail: unknown): string =>
20-
Array.isArray(detail) ? detail.join(' ') : String(detail);
20+
// Aggregated validation errors are list[str]; FastAPI's own request
21+
// validation is list[{msg,...}] — normalize both, never "[object Object]".
22+
Array.isArray(detail)
23+
? detail
24+
.map(d => (typeof d === 'string' ? d : ((d as {msg?: string})?.msg ?? JSON.stringify(d))))
25+
.join('; ')
26+
: String(detail);
2127

2228
export const postSubmission = async (body: SubmissionCreate): Promise<Result> => {
2329
const response = await post<SubmissionCreate, SubmissionCreated>('submissions')({body});

app/src/app/utils/api/apiHandlers/types.ts

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -167,54 +167,6 @@ export type MapGroup = {
167167
slug: string;
168168
};
169169

170-
export interface CommentCreate {
171-
title: string;
172-
comment: string;
173-
commenter_id: number | null;
174-
document_id: string | null;
175-
}
176-
177-
export interface CommentPublic {
178-
created_at: string | null;
179-
updated_at: string | null;
180-
}
181-
182-
export interface CommenterCreate {
183-
first_name: string;
184-
email: string;
185-
salutation: string | null;
186-
last_name: string | null;
187-
place: string | null;
188-
state: string | null;
189-
zip_code: string | null;
190-
}
191-
192-
export interface CommenterPublic {
193-
created_at: string | null;
194-
updated_at: string | null;
195-
}
196-
197-
export interface TagPublic {
198-
slug: string;
199-
}
200-
201-
export interface TagCreate {
202-
tag: string;
203-
}
204-
205-
export interface FullCommentForm {
206-
comment: CommentCreate;
207-
commenter: CommenterCreate;
208-
tags: TagCreate[];
209-
turnstile_token: string;
210-
}
211-
212-
export interface FullCommentFormResponse {
213-
comment: CommentPublic;
214-
commenter: CommenterPublic;
215-
tags: TagPublic[];
216-
}
217-
218170
export interface Overlay {
219171
overlay_id: string;
220172
name: string;

backend/tests/test_submissions.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,3 +593,48 @@ def test_form_config_shape(self, client, form_config):
593593
assert body["required_fields"] == form_config.required_fields
594594
assert "admin_teams" not in body
595595
assert "id" not in body
596+
597+
598+
class TestPublicListParams:
599+
"""The public list's newly-loosened boundary: portal_id optional (cross-
600+
portal galleries) and an ids filter — neither may widen visibility past
601+
submitted + not-hidden."""
602+
603+
def test_omitting_portal_id_spans_portals(self, client, form_config):
604+
a = _submit(client).json()["id"]
605+
b = _submit(client, fields={"title": "other"}, portal_id=OTHER_PORTAL).json()[
606+
"id"
607+
]
608+
listed = {s["id"] for s in client.get("/api/submissions").json()}
609+
assert {a, b} <= listed
610+
611+
def test_ids_filter_narrows(self, client, form_config):
612+
a = _submit(client).json()["id"]
613+
_submit(client, fields={"title": "other"}, portal_id=OTHER_PORTAL)
614+
listed = client.get(f"/api/submissions?ids={a}").json()
615+
assert [s["id"] for s in listed] == [a]
616+
617+
def test_ids_cannot_fish_out_hidden_rows(self, client, form_config, session):
618+
submission_id = _submit(client).json()["id"]
619+
_set_auth(TEAM_A_PAYLOAD)
620+
response = client.post(
621+
f"/api/submissions/admin/{submission_id}/hidden", json={"hidden": True}
622+
)
623+
assert response.status_code == 200
624+
assert client.get(f"/api/submissions?ids={submission_id}").json() == []
625+
626+
def test_ids_cannot_fish_out_drafts(
627+
self, client, form_config, ks_demo_view_census_blocks_districtrmap, session
628+
):
629+
response = client.post(
630+
"/api/create_document",
631+
json={
632+
"districtr_map_slug": "ks_demo_view_census_blocks",
633+
"portal_id": PORTAL,
634+
},
635+
)
636+
assert response.status_code == 201
637+
draft_pk = session.exec(
638+
select(Submission.id).where(col(Submission.portal_id) == PORTAL)
639+
).one()
640+
assert client.get(f"/api/submissions?ids={draft_pk}").json() == []

cms/content/api.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ def _inject_portal_tag(body_data, portal_slug):
5959
tags = list(block.get("value", {}).get("mandatoryTags") or [])
6060
if portal_slug not in tags:
6161
block["value"]["mandatoryTags"] = [portal_slug, *tags]
62+
elif block.get("type") == "comment_gallery":
63+
# A portal page's gallery lists ITS portal's submissions —
64+
# without this, an empty editor `tags` field would list every
65+
# portal's submissions, and user-added tag filters (OR
66+
# semantics) would widen back across portals.
67+
block["value"]["portalId"] = portal_slug
6268
return body_data
6369

6470

0 commit comments

Comments
 (0)