fix(issue-documents): address greptile review

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/routes/issues.ts

@@ -443,8 +443,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
     }
 
     const actor = getActorInfo(req);
-    const before = await documentsSvc.getIssueDocumentByKey(issue.id, keyParsed.data);
-    const doc = await documentsSvc.upsertIssueDocument({
+    const result = await documentsSvc.upsertIssueDocument({
       issueId: issue.id,
       key: keyParsed.data,
       title: req.body.title ?? null,
@@ -455,6 +454,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
       createdByAgentId: actor.agentId ?? null,
       createdByUserId: actor.actorType === "user" ? actor.actorId : null,
     });
+    const doc = result.document;
 
     await logActivity(db, {
       companyId: issue.companyId,
@@ -462,7 +462,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
       actorId: actor.actorId,
       agentId: actor.agentId,
       runId: actor.runId,
-      action: before ? "issue.document_updated" : "issue.document_created",
+      action: result.created ? "issue.document_created" : "issue.document_updated",
       entityType: "issue",
       entityId: issue.id,
       details: {
@@ -474,7 +474,7 @@ export function issueRoutes(db: Db, storage: StorageService) {
       },
     });
 
-    res.status(before ? 200 : 201).json(doc);
+    res.status(result.created ? 201 : 200).json(doc);
   });
 
   router.get("/issues/:id/documents/:key/revisions", async (req, res) => {

server/src/services/documents.ts

@@ -297,15 +297,18 @@ export function documentService(db: Db) {
               .where(eq(issueDocuments.documentId, existing.id));
 
             return {
-              ...existing,
-              title: input.title ?? null,
-              format: input.format,
-              body: input.body,
-              latestRevisionId: revision.id,
-              latestRevisionNumber: nextRevisionNumber,
-              updatedByAgentId: input.createdByAgentId ?? null,
-              updatedByUserId: input.createdByUserId ?? null,
-              updatedAt: now,
+              created: false as const,
+              document: {
+                ...existing,
+                title: input.title ?? null,
+                format: input.format,
+                body: input.body,
+                latestRevisionId: revision.id,
+                latestRevisionNumber: nextRevisionNumber,
+                updatedByAgentId: input.createdByAgentId ?? null,
+                updatedByUserId: input.createdByUserId ?? null,
+                updatedAt: now,
+              },
             };
           }
 
@@ -360,21 +363,24 @@ export function documentService(db: Db) {
           });
 
           return {
-            id: document.id,
-            companyId: issue.companyId,
-            issueId: issue.id,
-            key,
-            title: document.title,
-            format: document.format,
-            body: document.latestBody,
-            latestRevisionId: revision.id,
-            latestRevisionNumber: 1,
-            createdByAgentId: document.createdByAgentId,
-            createdByUserId: document.createdByUserId,
-            updatedByAgentId: document.updatedByAgentId,
-            updatedByUserId: document.updatedByUserId,
-            createdAt: document.createdAt,
-            updatedAt: document.updatedAt,
+            created: true as const,
+            document: {
+              id: document.id,
+              companyId: issue.companyId,
+              issueId: issue.id,
+              key,
+              title: document.title,
+              format: document.format,
+              body: document.latestBody,
+              latestRevisionId: revision.id,
+              latestRevisionNumber: 1,
+              createdByAgentId: document.createdByAgentId,
+              createdByUserId: document.createdByUserId,
+              updatedByAgentId: document.updatedByAgentId,
+              updatedByUserId: document.updatedByUserId,
+              createdAt: document.createdAt,
+              updatedAt: document.updatedAt,
+            },
           };
         });
       } catch (error) {

ui/src/components/IssueDocumentsSection.tsx

@@ -31,6 +31,7 @@ type DraftState = {
 type DocumentConflictState = {
   key: string;
   serverDocument: IssueDocument;
+  localDraft: DraftState;
   showRemote: boolean;
 };
 
@@ -189,14 +190,15 @@ export function IssueDocumentsSection({
   const beginEdit = (key: string) => {
     const doc = sortedDocuments.find((entry) => entry.key === key);
     if (!doc) return;
+    const conflictedDraft = documentConflict?.key === key ? documentConflict.localDraft : null;
     setFoldedDocumentKeys((current) => current.filter((entry) => entry !== key));
     resetAutosaveState();
     setDocumentConflict((current) => current?.key === key ? current : null);
     setDraft({
-      key: doc.key,
-      title: doc.title ?? "",
-      body: doc.body,
-      baseRevisionId: doc.latestRevisionId,
+      key: conflictedDraft?.key ?? doc.key,
+      title: conflictedDraft?.title ?? doc.title ?? "",
+      body: conflictedDraft?.body ?? doc.body,
+      baseRevisionId: conflictedDraft?.baseRevisionId ?? doc.latestRevisionId,
       isNew: false,
     });
     setError(null);
@@ -306,6 +308,13 @@ export function IssueDocumentsSection({
           setDocumentConflict({
             key: normalizedKey,
             serverDocument: latestDocument,
+            localDraft: {
+              key: normalizedKey,
+              title: isPlanKey(normalizedKey) ? "" : normalizedTitle,
+              body: currentDraft.body,
+              baseRevisionId: currentDraft.baseRevisionId,
+              isNew: false,
+            },
             showRemote: true,
           });
           setFoldedDocumentKeys((current) => current.filter((key) => key !== normalizedKey));
@@ -338,10 +347,14 @@ export function IssueDocumentsSection({
   }, [documentConflict, resetAutosaveState]);
 
   const overwriteDocumentFromDraft = useCallback(async (key: string) => {
-    if (documentConflict?.key !== key || !draft || draft.key !== key || draft.isNew) return;
+    if (documentConflict?.key !== key) return;
+    const sourceDraft =
+      draft && draft.key === key && !draft.isNew
+        ? draft
+        : documentConflict.localDraft;
     await commitDraft(
       {
-        ...draft,
+        ...sourceDraft,
         baseRevisionId: documentConflict.serverDocument.latestRevisionId,
       },
       {
@@ -352,6 +365,17 @@ export function IssueDocumentsSection({
     );
   }, [commitDraft, documentConflict, draft]);
 
+  const keepConflictedDraft = useCallback((key: string) => {
+    if (documentConflict?.key !== key) return;
+    setDraft(documentConflict.localDraft);
+    setDocumentConflict((current) =>
+      current?.key === key
+        ? { ...current, showRemote: false }
+        : current,
+    );
+    setError(null);
+  }, [documentConflict]);
+
   const copyDocumentBody = useCallback(async (key: string, body: string) => {
     try {
       await navigator.clipboard.writeText(body);
@@ -727,13 +751,7 @@ export function IssueDocumentsSection({
                           <Button
                             variant="outline"
                             size="sm"
-                            onClick={() =>
-                              setDocumentConflict((current) =>
-                                current?.key === doc.key
-                                  ? { ...current, showRemote: false }
-                                  : current,
-                              )
-                            }
+                            onClick={() => keepConflictedDraft(doc.key)}
                           >
                             Keep my draft
                           </Button>