Skip to content

Commit e092500

Browse files
nofurtherinformationclaude
nofurtherinformation
and
claude
committed
Convert legacy form comments into submissions before the drop
The drop-without-migration decision is revised: dev has live legacy testimony (TN workshop and after), so d8f1b52c96e3 becomes convert-then-drop. Every legacy form comment (any comment that is not a zone note) becomes a submission under a catch-all 'legacy' form config, created only when there is anything to migrate: - tags preserved verbatim, so tag-filtered galleries keep showing them; portal_id re-attribution later is a plain UPDATE (FK is ON UPDATE CASCADE). - map attachments keep their live document reference (legacy behavior) — clone-at-submission applies only to new submissions. - moderation mapping preserves what the old public gate showed: hidden = anything REJECTED (comment, commenter, or tag); nsfw = any score >= 0.2 without an APPROVED override; flagged and max moderation score carried over. - content rows: title/comment plus the commenter's fields, non-empty values only (email stays reviewer-only as before). - submission ids = legacy comment ids offset past MAX(submissions.id), so deployments that already collected new submissions via pr10..13 cannot collide; the sequence is advanced afterwards. Verified end-to-end on a local database seeded with a clean tagged comment with a map link, a REJECTED comment, and a score-flagged one: conversion, public list (hidden excluded, nsfw served, email stripped), and the tag gallery all behave as mapped. Downgrade still recreates the empty legacy tables and deliberately does not reverse the conversion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent f810d3a commit e092500

2 files changed

Lines changed: 195 additions & 58 deletions

File tree

‎backend/app/alembic/versions/d8f1b52c96e3_drop_legacy_comment_tables.py‎

Lines changed: 159 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,28 @@
1-
"""Drop the legacy comment tables
1+
"""Convert legacy form comments into submissions, then drop the comment tables
22
33
The flexible submissions schema (c7e2a94d81f5) and district_notes
44
(b3d9f47a25c1) replaced everything these tables did; the backend module that
55
served them (app/comments) is deleted in the same change. Zone rows were
6-
copied into district_notes by b3d9f47a25c1; form comments/commenters/tags are
7-
dropped without migration by decision.
6+
copied into district_notes by b3d9f47a25c1.
7+
8+
Form comments are real data (dev has live testimony, e.g. the TN workshop),
9+
so before dropping, every legacy form comment becomes a submission under a
10+
catch-all 'legacy' form config:
11+
12+
- tags are preserved verbatim, so tag-filtered galleries keep showing them;
13+
only the per-portal admin queue groups them under 'legacy' (portal_id is
14+
ON UPDATE CASCADE — re-attribute later with a plain UPDATE if wanted).
15+
- map attachments keep their LIVE document reference (legacy behavior);
16+
clone-at-submission applies only to new submissions.
17+
- moderation maps to the new bits preserving what the old public gate
18+
showed: hidden = anything REJECTED (comment, commenter, or a tag);
19+
nsfw = any moderation score >= 0.2 without an APPROVED override.
20+
- submission ids are the legacy comment ids offset past MAX(submissions.id),
21+
so a deploy where pr10..13 already collected new submissions can't collide.
822
923
Downgrade recreates the tables (final shape as of 0db008690d60 + da39a3ee5e6b)
10-
empty — the data is gone; restore from a backup if it's needed.
24+
empty — it does NOT reverse the conversion (converted rows simply remain in
25+
comments.submissions); restore from a backup if the original rows are needed.
1126
1227
Revision ID: d8f1b52c96e3
1328
Revises: c7e2a94d81f5
@@ -59,7 +74,147 @@ def _timestamps():
5974
)
6075

6176

77+
# Form comments = every comment that is not a zone note. document_comment's
78+
# PK is comment_id, so the LEFT JOIN cannot fan out.
79+
_FORM_COMMENT_FILTER = """
80+
LEFT JOIN comments.document_comment dc ON dc.comment_id = c.id
81+
WHERE dc.zone IS NULL
82+
"""
83+
84+
# The full field registry (backend/app/submissions/fields.py) so the catch-all
85+
# config passes the required_fields <@ fields CHECK under any later edit.
86+
_ALL_FIELDS = (
87+
"ARRAY['salutation','first_name','last_name','email','title',"
88+
"'comment','place','state','zip_code']::varchar(64)[]"
89+
)
90+
91+
92+
def _convert_legacy_form_comments(bind) -> None:
93+
# Catch-all portal config, only when there is anything to migrate.
94+
bind.execute(
95+
sa.text(
96+
f"""
97+
INSERT INTO comments.form_configs
98+
(portal_id, name, fields, required_fields,
99+
require_email_confirm, admin_teams)
100+
SELECT 'legacy', 'Legacy submissions', {_ALL_FIELDS},
101+
'{{}}', false, '{{}}'
102+
WHERE EXISTS (
103+
SELECT 1 FROM comments.comment c {_FORM_COMMENT_FILTER}
104+
)
105+
ON CONFLICT (portal_id) DO NOTHING
106+
"""
107+
)
108+
)
109+
110+
offset = bind.execute(
111+
sa.text("SELECT COALESCE(MAX(id), 0) FROM comments.submissions")
112+
).scalar()
113+
114+
bind.execute(
115+
sa.text(
116+
"""
117+
WITH enriched AS (
118+
SELECT
119+
c.id, c.created_at, c.updated_at, c.review_flagged,
120+
d.public_id,
121+
c.review_status::text AS c_status,
122+
c.moderation_score AS c_score,
123+
cm.review_status::text AS m_status,
124+
cm.moderation_score AS m_score,
125+
t.slugs, t.rejected_tag, t.score_flagged_tag, t.max_tag_score
126+
FROM comments.comment c
127+
LEFT JOIN comments.document_comment dc ON dc.comment_id = c.id
128+
LEFT JOIN comments.commenter cm ON cm.id = c.commenter_id
129+
LEFT JOIN LATERAL (
130+
SELECT
131+
array_agg(tg.slug ORDER BY tg.id)::varchar(255)[] AS slugs,
132+
bool_or(tg.review_status::text = 'REJECTED') AS rejected_tag,
133+
bool_or(
134+
tg.moderation_score >= 0.2
135+
AND tg.review_status::text IS DISTINCT FROM 'APPROVED'
136+
) AS score_flagged_tag,
137+
max(tg.moderation_score) AS max_tag_score
138+
FROM comments.comment_tag ct
139+
JOIN comments.tag tg ON tg.id = ct.tag_id
140+
WHERE ct.comment_id = c.id
141+
) t ON true
142+
LEFT JOIN document.document d ON d.document_id = dc.document_id
143+
WHERE dc.zone IS NULL
144+
)
145+
INSERT INTO comments.submissions
146+
(id, portal_id, map_public_id, tags, status, submitted_at,
147+
nsfw, hidden, flagged, moderation_score,
148+
created_at, updated_at)
149+
SELECT
150+
e.id + :offset,
151+
'legacy',
152+
e.public_id,
153+
COALESCE(e.slugs, '{}'),
154+
'submitted',
155+
e.created_at,
156+
COALESCE(
157+
(e.c_score >= 0.2 AND e.c_status IS DISTINCT FROM 'APPROVED')
158+
OR (e.m_score >= 0.2 AND e.m_status IS DISTINCT FROM 'APPROVED')
159+
OR e.score_flagged_tag,
160+
false
161+
),
162+
COALESCE(
163+
e.c_status = 'REJECTED'
164+
OR e.m_status = 'REJECTED'
165+
OR e.rejected_tag,
166+
false
167+
),
168+
e.review_flagged,
169+
GREATEST(e.c_score, e.m_score, e.max_tag_score),
170+
e.created_at,
171+
e.updated_at
172+
FROM enriched e
173+
"""
174+
),
175+
{"offset": offset},
176+
)
177+
178+
bind.execute(
179+
sa.text(
180+
f"""
181+
INSERT INTO comments.submissions_content (submission_id, field, value)
182+
SELECT c.id + :offset, f.field, LEFT(f.value, 5000)
183+
FROM comments.comment c
184+
LEFT JOIN comments.commenter cm ON cm.id = c.commenter_id
185+
CROSS JOIN LATERAL (VALUES
186+
('title', c.title),
187+
('comment', c.comment),
188+
('salutation', cm.salutation),
189+
('first_name', cm.first_name),
190+
('last_name', cm.last_name),
191+
('email', cm.email),
192+
('place', cm.place),
193+
('state', cm.state),
194+
('zip_code', cm.zip_code)
195+
) AS f(field, value)
196+
{_FORM_COMMENT_FILTER}
197+
AND f.value IS NOT NULL AND LENGTH(TRIM(f.value)) > 0
198+
"""
199+
),
200+
{"offset": offset},
201+
)
202+
203+
# Explicit-id inserts bypass the sequence; advance it past the copied ids.
204+
bind.execute(
205+
sa.text(
206+
"""
207+
SELECT setval(
208+
pg_get_serial_sequence('comments.submissions', 'id'),
209+
(SELECT COALESCE(MAX(id), 1) FROM comments.submissions)
210+
)
211+
"""
212+
)
213+
)
214+
215+
62216
def upgrade() -> None:
217+
_convert_legacy_form_comments(op.get_bind())
63218
op.drop_table("document_comment", schema="comments")
64219
op.drop_table("comment_tag", schema="comments")
65220
op.drop_table("comment", schema="comments")

‎backend/tests/test_main.py‎

Lines changed: 36 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,80 +1427,62 @@ def test_document_list(
14271427
assert data[0].get("public_id") == public_id
14281428

14291429

1430-
def test_document_list_metadata_tags_and_draft_status(client, document_id_total_vap):
1431-
# A scratch map with a metadata tag is not yet "submitted" to tag galleries.
1430+
def test_document_list_metadata_tags_are_not_a_gallery_mechanism(
1431+
client, document_id_total_vap
1432+
):
1433+
# Design decision: submissions are the ONLY way a map enters a tag
1434+
# gallery. A map carrying matching metadata tags — even past scratch —
1435+
# must not appear (metadata tags remain display-only annotations).
14321436
response = client.put(
14331437
f"/api/document/{document_id_total_vap}/metadata",
1434-
json={"tags": ["workshop"], "draft_status": "scratch"},
1438+
json={"tags": ["workshop"], "draft_status": "in_progress"},
14351439
)
14361440
assert response.status_code == 200
14371441
response = client.get("/api/documents/list?tags=workshop")
14381442
assert response.status_code == 200
14391443
assert response.json() == []
14401444

1441-
# Moving to in_progress submits it; the partial update must not wipe tags.
1445+
# The partial metadata update must not wipe sibling keys (dev's merge
1446+
# semantics, which the draft-status flows depend on).
14421447
response = client.put(
14431448
f"/api/document/{document_id_total_vap}/metadata",
1444-
json={"draft_status": "in_progress"},
1449+
json={"draft_status": "ready_to_share"},
14451450
)
14461451
assert response.status_code == 200
1447-
response = client.get("/api/documents/list?tags=workshop")
1448-
data = response.json()
1449-
assert len(data) == 1
1450-
assert data[0]["map_metadata"]["tags"] == ["workshop"]
1451-
assert data[0]["map_metadata"]["draft_status"] == "in_progress"
1452+
listed = client.get(
1453+
f"/api/documents/list?ids={_public_id_of(client, document_id_total_vap)}"
1454+
).json()
1455+
assert listed[0]["map_metadata"]["tags"] == ["workshop"]
1456+
assert listed[0]["map_metadata"]["draft_status"] == "ready_to_share"
14521457

1453-
# Explicit completion-status filter narrows the listing.
1454-
response = client.get(
1455-
"/api/documents/list?tags=workshop&draft_status=ready_to_share"
1456-
)
1457-
assert response.json() == []
1458-
response = client.get("/api/documents/list?tags=workshop&draft_status=in_progress")
1459-
assert len(response.json()) == 1
14601458

1459+
def _public_id_of(client, document_id):
1460+
return client.get(f"/api/document/{document_id}").json()["public_id"]
14611461

1462-
def test_document_list_comment_tags(client, document_id_total_vap):
1463-
# The comment-form tag path must match on its own: the document's own
1464-
# metadata tags deliberately do NOT include the queried slug, and
1465-
# in_progress pins the submitted-statuses default on this path too.
1466-
response = client.put(
1467-
f"/api/document/{document_id_total_vap}/metadata",
1468-
json={"tags": ["other"], "draft_status": "in_progress"},
1469-
)
1470-
assert response.status_code == 200
1471-
comment_data = {
1472-
"commenter": {
1473-
"first_name": "Test",
1474-
"email": "test@example.com",
1475-
"place": "Portland",
1476-
"state": "OR",
1477-
},
1478-
"comment": {
1479-
"title": "Test Comment",
1480-
"comment": "This is a test comment with some content.",
1481-
"document_id": document_id_total_vap,
1482-
},
1483-
"tags": [{"tag": "workshop"}],
1484-
"turnstile_token": "test_token",
1485-
}
1486-
response = client.post("/api/comments/submit", json=comment_data)
1487-
assert response.status_code == 201
1488-
handle_full_submission_approve(client, response.json())
1489-
1490-
response = client.get("/api/documents/list?tags=workshop")
1491-
assert response.status_code == 200
1492-
data = response.json()
1493-
assert len(data) == 1
1494-
assert data[0]["map_metadata"]["tags"] == ["other"]
14951462

1496-
# The scratch gate applies to comment-tagged documents as well.
1463+
def test_document_list_draft_status_filter(client, document_id_total_vap):
1464+
# The explicit draft_status filter narrows any listing by the map's own
1465+
# metadata status.
1466+
public_id = _public_id_of(client, document_id_total_vap)
14971467
response = client.put(
14981468
f"/api/document/{document_id_total_vap}/metadata",
1499-
json={"draft_status": "scratch"},
1469+
json={"draft_status": "in_progress"},
15001470
)
15011471
assert response.status_code == 200
1502-
response = client.get("/api/documents/list?tags=workshop")
1503-
assert response.json() == []
1472+
assert (
1473+
client.get(
1474+
f"/api/documents/list?ids={public_id}&draft_status=ready_to_share"
1475+
).json()
1476+
== []
1477+
)
1478+
assert (
1479+
len(
1480+
client.get(
1481+
f"/api/documents/list?ids={public_id}&draft_status=in_progress"
1482+
).json()
1483+
)
1484+
== 1
1485+
)
15041486

15051487

15061488
def test_get_district_unions(client, document_id_total_vap):

0 commit comments

Comments
 (0)