From eb0a74384ea51c31283f6c599822822201b59e4a Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 13 Mar 2026 22:17:49 -0500 Subject: [PATCH] fix(issues): address document review comments Co-Authored-By: Paperclip --- packages/shared/src/types/issue.ts | 2 +- server/src/services/documents.ts | 4 +- server/src/services/issues.ts | 12 +++++ ui/src/components/IssueDocumentsSection.tsx | 52 ++++++++++++++++----- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index 4b812623..94e482c3 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -59,7 +59,7 @@ export interface IssueDocumentSummary { key: string; title: string | null; format: DocumentFormat; - latestRevisionId: string; + latestRevisionId: string | null; latestRevisionNumber: number; createdByAgentId: string | null; createdByUserId: string | null; diff --git a/server/src/services/documents.ts b/server/src/services/documents.ts index 7c034cd4..df96f156 100644 --- a/server/src/services/documents.ts +++ b/server/src/services/documents.ts @@ -53,7 +53,7 @@ function mapIssueDocumentRow( title: row.title, format: row.format, ...(includeBody ? { body: row.latestBody } : {}), - latestRevisionId: row.latestRevisionId ?? "", + latestRevisionId: row.latestRevisionId ?? null, latestRevisionNumber: row.latestRevisionNumber, createdByAgentId: row.createdByAgentId, createdByUserId: row.createdByUserId, @@ -419,7 +419,7 @@ export function documentService(db: Db) { return { ...existing, body: existing.latestBody, - latestRevisionId: existing.latestRevisionId ?? "", + latestRevisionId: existing.latestRevisionId ?? null, }; }); }, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index ecff7c3c..51d4dcb4 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -5,11 +5,13 @@ import { assets, companies, companyMemberships, + documents, goals, heartbeatRuns, issueAttachments, issueLabels, issueComments, + issueDocuments, issueReadStates, issues, labels, @@ -790,6 +792,10 @@ export function issueService(db: Db) { .select({ assetId: issueAttachments.assetId }) .from(issueAttachments) .where(eq(issueAttachments.issueId, id)); + const issueDocumentIds = await tx + .select({ documentId: issueDocuments.documentId }) + .from(issueDocuments) + .where(eq(issueDocuments.issueId, id)); const removedIssue = await tx .delete(issues) @@ -803,6 +809,12 @@ export function issueService(db: Db) { .where(inArray(assets.id, attachmentAssetIds.map((row) => row.assetId))); } + if (removedIssue && issueDocumentIds.length > 0) { + await tx + .delete(documents) + .where(inArray(documents.id, issueDocumentIds.map((row) => row.documentId))); + } + if (!removedIssue) return null; const [enriched] = await withIssueLabels(tx, [removedIssue]); return enriched; diff --git a/ui/src/components/IssueDocumentsSection.tsx b/ui/src/components/IssueDocumentsSection.tsx index 985ed75d..b367f6f9 100644 --- a/ui/src/components/IssueDocumentsSection.tsx +++ b/ui/src/components/IssueDocumentsSection.tsx @@ -27,6 +27,7 @@ type DraftState = { }; const DOCUMENT_AUTOSAVE_DEBOUNCE_MS = 900; +const DOCUMENT_KEY_PATTERN = /^[a-z0-9][a-z0-9_-]*$/; function renderBody(body: string, className?: string) { return {body}; @@ -51,6 +52,7 @@ export function IssueDocumentsSection({ const [confirmDeleteKey, setConfirmDeleteKey] = useState(null); const [error, setError] = useState(null); const [draft, setDraft] = useState(null); + const [autosaveDocumentKey, setAutosaveDocumentKey] = useState(null); const autosaveDebounceRef = useRef | null>(null); const { state: autosaveState, @@ -100,9 +102,23 @@ export function IssueDocumentsSection({ }, [documents]); const hasRealPlan = sortedDocuments.some((doc) => doc.key === "plan"); + const newDocumentKeyError = + draft?.isNew && draft.key.trim().length > 0 && !DOCUMENT_KEY_PATTERN.test(draft.key.trim()) + ? "Use lowercase letters, numbers, -, or _, and start with a letter or number." + : null; + + const resetAutosaveState = useCallback(() => { + setAutosaveDocumentKey(null); + reset(); + }, [reset]); + + const markDocumentDirty = useCallback((key: string) => { + setAutosaveDocumentKey(key); + markDirty(); + }, [markDirty]); const beginNewDocument = () => { - reset(); + resetAutosaveState(); setDraft({ key: "", title: "", @@ -116,7 +132,7 @@ export function IssueDocumentsSection({ const beginEdit = (key: string) => { const doc = sortedDocuments.find((entry) => entry.key === key); if (!doc) return; - reset(); + resetAutosaveState(); setDraft({ key: doc.key, title: doc.title ?? "", @@ -131,7 +147,7 @@ export function IssueDocumentsSection({ if (autosaveDebounceRef.current) { clearTimeout(autosaveDebounceRef.current); } - reset(); + resetAutosaveState(); setDraft(null); setError(null); }; @@ -152,7 +168,15 @@ export function IssueDocumentsSection({ setError("Document body cannot be empty"); } if (options?.trackAutosave) { - reset(); + resetAutosaveState(); + } + return false; + } + + if (!DOCUMENT_KEY_PATTERN.test(normalizedKey)) { + setError("Document key must start with a letter or number and use only lowercase letters, numbers, -, or _."); + if (options?.trackAutosave) { + resetAutosaveState(); } return false; } @@ -168,7 +192,7 @@ export function IssueDocumentsSection({ setDraft((value) => (value?.key === normalizedKey ? null : value)); } if (options?.trackAutosave) { - reset(); + resetAutosaveState(); } return true; } @@ -197,6 +221,7 @@ export function IssueDocumentsSection({ try { if (options?.trackAutosave) { + setAutosaveDocumentKey(normalizedKey); await runSave(save); } else { await save(); @@ -206,7 +231,7 @@ export function IssueDocumentsSection({ setError(err instanceof Error ? err.message : "Failed to save document"); return false; } - }, [invalidateIssueDocuments, reset, runSave, sortedDocuments, upsertDocument]); + }, [invalidateIssueDocuments, resetAutosaveState, runSave, sortedDocuments, upsertDocument]); const handleDraftBlur = async (event: React.FocusEvent) => { if (event.currentTarget.contains(event.relatedTarget as Node | null)) return; @@ -248,11 +273,11 @@ export function IssueDocumentsSection({ (existing.title ?? "") !== draft.title; if (!hasChanges) { if (autosaveState !== "saved") { - reset(); + resetAutosaveState(); } return; } - markDirty(); + markDocumentDirty(draft.key); if (autosaveDebounceRef.current) { clearTimeout(autosaveDebounceRef.current); } @@ -265,7 +290,7 @@ export function IssueDocumentsSection({ clearTimeout(autosaveDebounceRef.current); } }; - }, [autosaveState, commitDraft, draft, markDirty, reset, sortedDocuments]); + }, [autosaveState, commitDraft, draft, markDocumentDirty, resetAutosaveState, sortedDocuments]); const documentBodyShellClassName = "mt-3 overflow-hidden rounded-md border border-border bg-background"; const documentBodyPaddingClassName = "px-3 py-3"; @@ -297,6 +322,9 @@ export function IssueDocumentsSection({ } placeholder="Document key" /> + {newDocumentKeyError && ( +

{newDocumentKeyError}

+ )} {!isPlanKey(draft.key) && ( { - markDirty(); + markDocumentDirty(doc.key); setDraft((current) => current ? { ...current, title: event.target.value } : current); }} placeholder="Optional title" @@ -434,7 +462,7 @@ export function IssueDocumentsSection({ { - markDirty(); + markDocumentDirty(doc.key); setDraft((current) => { if (current && current.key === doc.key && !current.isNew) { return { ...current, body }; @@ -463,7 +491,7 @@ export function IssueDocumentsSection({ autosaveState === "error" ? "text-destructive" : "text-muted-foreground" } ${activeDraft ? "opacity-100" : "opacity-0"}`} > - {activeDraft + {activeDraft && autosaveDocumentKey === doc.key ? autosaveState === "saving" ? "Autosaving..." : autosaveState === "saved"