Skip to content

Commit eb5b810

Browse files
Collection-mode review fixes: auto entries moderated, withdrawable, oracle-free
pr-review findings: - Auto-collected submissions were NEVER moderated ('no text content' — false: the gallery card renders the map's name/description, which the scorer deliberately covers). auto_finalize now returns flipped ids and both callers enqueue moderation post-commit; renaming a live-referenced map re-schedules scoring, since the card text is live. - Live-referenced (map_is_clone=false) auto entries flip BOTH ways: regressing below the submitted tier withdraws the entry — the author never filled a consent form, and /api/submissions has no draft_status filter, so one-way stranded a map its author pulled back to scratch. Clone-backed submissions stay one-way. Docstring corrected (it claimed a filter that only exists on documents/list). - /api/submissions/flag refuses internal-portal rows: flagging them spammed the staff queue and leaked an existence oracle for staff-only galleries. - 'excluded from ALL public reads' claims scoped honestly to LISTINGS (any map's metadata was always fetchable by sequential public_id). - get_document_list uses SUBMITTED_DRAFT_STATUSES instead of restating it; migration index renamed to the autogenerate convention; stray empty backend/uv.lock deleted; auto_finalize takes the same row lock as finalize. - New tests: auto entries surface publicly (the auto_public/internal distinction had no positive assertion), map_is_clone stays false, takedown never demotes the live map, create-with-metadata flips at creation with moderation scheduled, rename re-scores, withdrawal round-trip, textarea cap at both bounds, internal flag-oracle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1cd5ae8 commit eb5b810

6 files changed

Lines changed: 244 additions & 37 deletions

File tree

backend/app/alembic/versions/e4a7c318b9d2_collection_mode_and_custom_fields.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def upgrade() -> None:
8585
schema="comments",
8686
)
8787
op.create_index(
88-
"idx_form_fields_custom_portal",
88+
"ix_comments_form_fields_custom_portal_id",
8989
"form_fields_custom",
9090
["portal_id"],
9191
schema="comments",

backend/app/main.py

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
from app.evaluation.types import MetricsEnvelope
7070
import app.save_share.main as save_share
7171
import app.submissions.main as submissions
72-
from app.submissions.main import auto_finalize_draft_submissions
7372
from app.submissions.models import (
7473
CollectionMode,
7574
FormConfig,
@@ -97,7 +96,7 @@
9796
AssignmentsCreate,
9897
NumDistrictsSetResult,
9998
)
100-
from app.save_share.models import DocumentDraftStatus
99+
from app.save_share.models import SUBMITTED_DRAFT_STATUSES, DocumentDraftStatus
101100
from pydantic_geojson import FeatureModel, PolygonModel
102101
from pydantic import BaseModel, ValidationError
103102
from pydantic_geojson._base import Coordinates
@@ -584,10 +583,14 @@ async def create_document(
584583
if data.portal_id is not None:
585584
# A creation payload can already carry a submitted-tier status
586585
# (e.g. copies); apply the same auto-collect flip as the
587-
# metadata endpoint.
588-
auto_finalize_draft_submissions(
586+
# metadata endpoint, with the same post-commit moderation pass
587+
# (the gallery card renders the map's name/description).
588+
for flipped_id in submissions.auto_finalize_draft_submissions(
589589
session, new_document.public_id, data.metadata.draft_status
590-
)
590+
):
591+
background_tasks.add_task(
592+
submissions.moderate_submission_by_id, flipped_id
593+
)
591594

592595
stmt = (
593596
select( # type: ignore[no-matching-overload] # ty: ignore[no-matching-overload]
@@ -1464,7 +1467,10 @@ async def get_document_list(
14641467
col(Submission.status) == SubmissionStatus.submitted,
14651468
col(Submission.hidden).is_(False),
14661469
col(Submission.nsfw).is_(False),
1467-
# Internal-mode portals never surface publicly.
1470+
# Internal-mode portals never surface in tag galleries
1471+
# or the public submissions list. (This is a LISTING
1472+
# guarantee: any map's metadata remains fetchable by its
1473+
# sequential public_id, as it always has been.)
14681474
col(FormConfig.collection_mode) != CollectionMode.internal,
14691475
)
14701476
)
@@ -1476,10 +1482,7 @@ async def get_document_list(
14761482
# (deliberate submissions are frozen clones at ready_to_share;
14771483
# auto-collected ones are live maps whose status this reflects).
14781484
if len(draft_status) == 0:
1479-
draft_status = [
1480-
DocumentDraftStatus.in_progress,
1481-
DocumentDraftStatus.ready_to_share,
1482-
]
1485+
draft_status = list(SUBMITTED_DRAFT_STATUSES)
14831486

14841487
if len(draft_status) > 0:
14851488
# this is fine to keep as ->> because you're comparing to text
@@ -1758,6 +1761,7 @@ async def get_connected_component_bboxes(
17581761
)
17591762
async def update_districtrmap_metadata(
17601763
metadata: DocumentMetadata,
1764+
background_tasks: BackgroundTasks,
17611765
document: Document = Depends(get_document),
17621766
session: Session = Depends(get_session),
17631767
):
@@ -1776,11 +1780,32 @@ async def update_districtrmap_metadata(
17761780
)
17771781
session.connection().execute(stmt)
17781782
# Auto-collect portals: reaching a submitted-tier status flips this
1779-
# map's draft submission to submitted (live reference, no clone).
1780-
auto_finalize_draft_submissions(
1783+
# map's draft submission to submitted (live reference, no clone);
1784+
# regressing withdraws it.
1785+
flipped = submissions.auto_finalize_draft_submissions(
17811786
session, document.public_id, merged.get("draft_status")
17821787
)
1788+
# Auto entries are live references, so the rendered card text (map
1789+
# name/description) can change AFTER the initial score — re-score
1790+
# submitted live-ref entries whenever those fields are touched.
1791+
rescore: set[int] = set(flipped)
1792+
if metadata.name is not None or metadata.description is not None:
1793+
rescore.update(
1794+
session.exec(
1795+
select(Submission.id).where(
1796+
and_(
1797+
col(Submission.map_public_id) == document.public_id,
1798+
col(Submission.status) == SubmissionStatus.submitted,
1799+
col(Submission.map_is_clone).is_(False),
1800+
)
1801+
)
1802+
).all()
1803+
)
17831804
session.commit()
1805+
for submission_id in rescore:
1806+
background_tasks.add_task(
1807+
submissions.moderate_submission_by_id, submission_id
1808+
)
17841809

17851810
except Exception as e:
17861811
logger.error(f"Unexpected error: {e}")

backend/app/submissions/main.py

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -99,36 +99,54 @@ def get_form_config(portal_id: str, session: Session) -> FormConfig:
9999

100100
def auto_finalize_draft_submissions(
101101
session: Session, public_id: int | None, draft_status: str | None
102-
) -> int:
103-
"""Flip auto-mode draft submissions to submitted when their map reaches a
104-
submitted-tier draft_status (in_progress / ready_to_share).
102+
) -> list[int]:
103+
"""Sync auto-mode submissions with their map's draft_status.
105104
106105
The auto-collect contract: entries keep their LIVE map reference — no
107-
clone, no form, no moderation task (there is no text content). Idempotent
108-
(only drafts match) and one-way (regressing to scratch does not
109-
un-submit; the gallery's draft_status filter hides scratch maps anyway).
110-
Called inside the caller's transaction; does not commit.
106+
clone, no form. Reaching a submitted-tier status (in_progress /
107+
ready_to_share) flips drafts to submitted; REGRESSING below it flips a
108+
live-referenced (map_is_clone=false) submission back to draft — the
109+
author never filled a consent form, so withdrawing their map must
110+
un-publish it everywhere (/api/submissions has no draft_status filter,
111+
so one-way would strand the entry there). Clone-backed submissions stay
112+
one-way: those were deliberate, consented submissions.
113+
114+
Idempotent; called inside the caller's transaction; does not commit.
115+
Returns the ids of rows flipped to submitted — callers MUST enqueue
116+
moderate_submission_by_id for each AFTER commit: the gallery card
117+
renders the map's name/description, so an unscored auto entry would let
118+
an abusive map title sail past the nsfw filter.
111119
"""
112-
if public_id is None or draft_status not in [
113-
s.value for s in SUBMITTED_DRAFT_STATUSES
114-
]:
115-
return 0
116-
drafts = session.exec(
120+
if public_id is None:
121+
return []
122+
submitted_tier = draft_status in [s.value for s in SUBMITTED_DRAFT_STATUSES]
123+
rows = session.exec(
117124
select(Submission)
118125
.join(FormConfig, col(FormConfig.portal_id) == Submission.portal_id)
119126
.where(
120127
and_(
121128
col(Submission.map_public_id) == public_id,
122-
col(Submission.status) == SubmissionStatus.draft,
123129
col(FormConfig.collection_mode).in_(CollectionMode.auto_modes),
124130
)
125131
)
132+
.with_for_update(of=Submission)
126133
).all()
127-
for draft in drafts:
128-
draft.status = SubmissionStatus.submitted
129-
draft.submitted_at = datetime.now(timezone.utc)
130-
session.add(draft)
131-
return len(drafts)
134+
flipped: list[int] = []
135+
for row in rows:
136+
if submitted_tier and row.status == SubmissionStatus.draft:
137+
row.status = SubmissionStatus.submitted
138+
row.submitted_at = datetime.now(timezone.utc)
139+
session.add(row)
140+
flipped.append(row.id)
141+
elif (
142+
not submitted_tier
143+
and row.status == SubmissionStatus.submitted
144+
and not row.map_is_clone
145+
):
146+
row.status = SubmissionStatus.draft
147+
row.submitted_at = None
148+
session.add(row)
149+
return flipped
132150

133151

134152
def get_custom_fields(portal_id: str, session: Session) -> list[FormFieldCustom]:
@@ -541,15 +559,21 @@ async def flag_submission(
541559
session: Session = Depends(get_session),
542560
):
543561
"""Report a submission for reviewer attention. Only publicly visible
544-
submissions can be flagged — flagging hidden ones gives moderators no
545-
signal and is a way to harass the queue."""
562+
submissions can be flagged — flagging hidden or internal-portal ones
563+
gives moderators no signal, is a way to harass the queue, and would
564+
leak an existence oracle for staff-only galleries."""
546565
submission = session.get(Submission, body.id)
547566
if (
548567
submission is None
549568
or submission.hidden
550569
or submission.status != SubmissionStatus.submitted
551570
):
552571
raise HTTPException(status_code=404, detail="Submission not found")
572+
config = session.exec(
573+
select(FormConfig).where(col(FormConfig.portal_id) == submission.portal_id)
574+
).first()
575+
if config is not None and config.collection_mode == CollectionMode.internal:
576+
raise HTTPException(status_code=404, detail="Submission not found")
553577
submission.flagged = True
554578
session.add(submission)
555579
session.commit()

backend/app/submissions/models.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,10 @@ class FormFieldCustom(TimeStampMixin, SQLModel, table=True):
120120
121121
Keys are 'custom_'-prefixed (slugified from the label by the CMS) so they
122122
can never collide with registry field names; values are stored in
123-
submissions_content like any other field and are PUBLIC.
123+
submissions_content like any other field and are PUBLIC — there is no
124+
per-question private bit, so the CMS question editor warns admins not to
125+
collect contact info through custom questions (the registry's email
126+
field is the private channel).
124127
"""
125128

126129
metadata = MetaData(schema=COMMENTS_SCHEMA)

0 commit comments

Comments
 (0)