review comment fixes

Author: Developing-Gamer

apps/backend/src/app/api/latest/internal/email-drafts/[id]/route.tsx

@@ -1,7 +1,9 @@
+import { Prisma } from "@/generated/prisma/client";
+import { templateThemeIdToThemeMode, themeModeToTemplateThemeId } from "@/lib/email-drafts";
 import { getPrismaClientForTenancy } from "@/prisma-client";
 import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
 import { templateThemeIdSchema, yupNumber, yupObject, yupString } from "@stackframe/stack-shared/dist/schema-fields";
-import { templateThemeIdToThemeMode, themeModeToTemplateThemeId } from "@/lib/email-drafts";
+import { StatusError } from "@stackframe/stack-shared/dist/utils/errors";
 
 export const GET = createSmartRouteHandler({
   metadata: { hidden: true },
@@ -25,7 +27,15 @@ export const GET = createSmartRouteHandler({
   }),
   async handler({ auth: { tenancy }, params }) {
     const prisma = await getPrismaClientForTenancy(tenancy);
-    const d = await prisma.emailDraft.findFirstOrThrow({ where: { tenancyId: tenancy.id, id: params.id } });
+    let d;
+    try {
+      d = await prisma.emailDraft.findFirstOrThrow({ where: { tenancyId: tenancy.id, id: params.id } });
+    } catch (error) {
+      if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2025") {
+        throw new StatusError(StatusError.NotFound, "No draft found with given id");
+      }
+      throw error;
+    }
     return {
       statusCode: 200,
       bodyType: "json",
@@ -94,9 +104,16 @@ export const DELETE = createSmartRouteHandler({
   }),
   async handler({ auth: { tenancy }, params }) {
     const prisma = await getPrismaClientForTenancy(tenancy);
-    await prisma.emailDraft.delete({
-      where: { tenancyId_id: { tenancyId: tenancy.id, id: params.id } },
-    });
+    try {
+      await prisma.emailDraft.delete({
+        where: { tenancyId_id: { tenancyId: tenancy.id, id: params.id } },
+      });
+    } catch (error) {
+      if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2025") {
+        throw new StatusError(StatusError.NotFound, "No draft found with given id");
+      }
+      throw error;
+    }
     return {
       statusCode: 200,
       bodyType: "json",

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsx

@@ -6,7 +6,6 @@ import { EmailThemeSelector } from "@/components/email-theme-selector";
 import { useRouterConfirm } from "@/components/router";
 import { Alert, AlertDescription, AlertTitle, Badge, Button, Card, CardContent, Select, SelectContent, SelectItem, SelectTrigger, SelectValue, Skeleton, Typography } from "@/components/ui";
 import { AssistantChat, CodeEditor, VibeCodeLayout, type ViewportMode, type WysiwygDebugInfo } from "@/components/vibe-coding";
-import { type WysiwygDebugInfo as EmailDebugInfo } from "@/components/email-preview";
 import { ToolCallContent, createChatAdapter, createHistoryAdapter } from "@/components/vibe-coding/chat-adapters";
 import { EmailDraftUI } from "@/components/vibe-coding/draft-tool-components";
 import { KnownErrors } from "@stackframe/stack-shared/dist/known-errors";
@@ -51,11 +50,15 @@ export default function PageClient({ draftId }: { draftId: string }) {
       await stackAdminApp.updateEmailDraft(draftId, { tsxSource: currentCode, themeId: selectedThemeId });
       setSaveAlert({ variant: "success", title: "Draft saved" });
     } catch (error) {
-      setSaveAlert({
-        variant: "destructive",
-        title: "Failed to save draft",
-        description: getErrorMessage(error),
-      });
+      if (error instanceof KnownErrors.EmailRenderingError) {
+        setSaveAlert({
+          variant: "destructive",
+          title: "Failed to save draft",
+          description: error.message,
+        });
+        return;
+      }
+      throw error;
     }
   };
 
@@ -81,7 +84,7 @@ export default function PageClient({ draftId }: { draftId: string }) {
   };
 
   const previewActions = null;
-  const isDirty = currentCode !== draft?.tsxSource || selectedThemeId !== draft.themeId;
+  const isDirty = draft ? (currentCode !== draft.tsxSource || selectedThemeId !== draft.themeId) : false;
 
   // Handle WYSIWYG edit commits - calls the AI endpoint to update source code
   const handleWysiwygEditCommit: OnWysiwygEditCommit = useCallback(async (data) => {

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsx

@@ -17,7 +17,7 @@ import {
 import { ToolCallContent } from "@/components/vibe-coding/chat-adapters";
 import { KnownErrors } from "@stackframe/stack-shared/dist/known-errors";
 import { runAsynchronously } from "@stackframe/stack-shared/dist/utils/promises";
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { AppEnabledGuard } from "../../app-enabled-guard";
 import { PageLayout } from "../../page-layout";
 import { useAdminApp } from "../../use-admin-app";
@@ -61,19 +61,25 @@ export default function PageClient(props: { templateId: string }) {
     setIsLoading(true);
 
     const fetchTemplate = async () => {
-      const allTemplates = await stackAdminApp.listEmailTemplates();
+      try {
+        const allTemplates = await stackAdminApp.listEmailTemplates();
 
-      if (cancelled) return;
+        if (cancelled) return;
 
-      const found = allTemplates.find((t) => t.id === props.templateId);
+        const found = allTemplates.find((t) => t.id === props.templateId);
 
-      if (found) {
-        setFetchedTemplate(found);
-        setCurrentCode(found.tsxSource);
-        setSelectedThemeId(found.themeId);
+        if (found) {
+          setFetchedTemplate(found);
+          setCurrentCode(found.tsxSource);
+          setSelectedThemeId(found.themeId);
+        }
+      } catch (error) {
+        const fetchError = error instanceof Error ? error : new Error(String(error));
+        setFetchError(fetchError);
+        throw fetchError;
+      } finally {
+        setIsLoading(false);
       }
-
-      setIsLoading(false);
     };
 
     runAsynchronously(fetchTemplate);
@@ -83,12 +89,16 @@ export default function PageClient(props: { templateId: string }) {
     };
   }, [templateFromHook, fetchedTemplate, stackAdminApp, props.templateId]);
 
+  const hasSyncedTemplateFromHook = useRef(false);
+
   // When the template appears in the hook (e.g., after cache updates), sync state
   useEffect(() => {
-    if (templateFromHook && !currentCode) {
+    if (!templateFromHook || hasSyncedTemplateFromHook.current) return;
+    if (!currentCode) {
       setCurrentCode(templateFromHook.tsxSource);
       setSelectedThemeId(templateFromHook.themeId);
     }
+    hasSyncedTemplateFromHook.current = true;
   }, [templateFromHook, currentCode]);
 
   useEffect(() => {

apps/dashboard/src/components/email-preview.tsx

@@ -572,9 +572,10 @@ const WYSIWYG_EDITOR_SCRIPT = `
 `;
 
 /**
- * Debug info for WYSIWYG editing
+ * Debug info for WYSIWYG editing.
+ * Renamed to avoid colliding with VibeCodeLayout's debug type with the same name.
  */
-export type WysiwygDebugInfo = {
+export type EmailPreviewDebugInfo = {
   renderedHtml?: string,
   editableRegions?: Record<string, unknown>,
 };
@@ -598,7 +599,7 @@ function EmailPreviewEditableContent({
   templateId?: string,
   templateTsxSource?: string,
   editableSource?: 'template' | 'theme' | 'both',
-  onDebugInfoChange?: (info: WysiwygDebugInfo) => void,
+  onDebugInfoChange?: (info: EmailPreviewDebugInfo) => void,
   onWysiwygEditCommit?: OnWysiwygEditCommit,
 }) {
   const stackAdminApp = useAdminApp();
@@ -756,7 +757,7 @@ type EmailPreviewProps =
     /** Callback when user commits a WYSIWYG edit. Should return updated source code. */
     onWysiwygEditCommit?: OnWysiwygEditCommit,
     /** Callback when debug info changes (edit mode only, dev mode only) */
-    onDebugInfoChange?: (info: WysiwygDebugInfo) => void,
+    onDebugInfoChange?: (info: EmailPreviewDebugInfo) => void,
   };
 
 export default function EmailPreview({

apps/dashboard/src/components/ui/dropdown-menu.tsx

@@ -5,9 +5,9 @@ import { CheckIcon, ChevronRightIcon, DotFilledIcon } from "@radix-ui/react-icon
 import { forwardRefIfNeeded } from "@stackframe/stack-shared/dist/utils/react";
 import React from "react";
 
+import { cn } from "@/lib/utils";
 import { throwErr } from "@stackframe/stack-shared/dist/utils/errors";
 import { runAsynchronouslyWithAlert } from "@stackframe/stack-shared/dist/utils/promises";
-import { cn } from "@/lib/utils";
 import { Spinner } from "./spinner";
 
 const DropdownMenuContext = React.createContext<{
@@ -120,6 +120,24 @@ const DropdownMenuItem = forwardRefIfNeeded<
   const { setOpen } = React.useContext(DropdownMenuContext) ?? throwErr("No DropdownMenuContext found");
   const [isLoading, setIsLoading] = React.useState(false);
 
+  // Share activation logic so keyboard onSelect matches mouse clicks.
+  const handleItemAction = (event: { preventDefault: () => void, stopPropagation: () => void }) => {
+    event.preventDefault();
+    event.stopPropagation();
+    const result = props.onClick?.(event as React.MouseEvent<HTMLDivElement, MouseEvent>);
+    if (result && typeof (result as Promise<void>).then === "function") {
+      setIsLoading(true);
+      runAsynchronouslyWithAlert(
+        Promise.resolve(result).finally(() => {
+          setIsLoading(false);
+          setOpen(false);
+        })
+      );
+    } else {
+      setOpen(false);
+    }
+  };
+
   return <DropdownMenuPrimitive.Item
     ref={ref}
     className={cn(
@@ -129,25 +147,8 @@ const DropdownMenuItem = forwardRefIfNeeded<
     )}
     {...props}
     disabled={isLoading || props.disabled}
-    onSelect={props.onClick ? (event) => {
-      event.preventDefault();
-    } : undefined}
-    onClick={props.onClick ? (e) => {
-      e.preventDefault();
-      e.stopPropagation();
-      const result = props.onClick?.(e);
-      if (result && typeof (result as Promise<void>).then === "function") {
-        setIsLoading(true);
-        runAsynchronouslyWithAlert(
-          Promise.resolve(result).finally(() => {
-            setIsLoading(false);
-            setOpen(false);
-          })
-        );
-      } else {
-        setOpen(false);
-      }
-    } : undefined}
+    onSelect={props.onClick ? handleItemAction : undefined}
+    onClick={props.onClick ? handleItemAction : undefined}
   >
     <div style={{ visibility: isLoading ? "visible" : "hidden", position: "absolute", inset: 0, display: "flex", justifyContent: "center", alignItems: "center" }}>
       <Spinner />

apps/e2e/tests/backend/endpoints/api/v1/internal/email-drafts.test.ts

@@ -460,12 +460,12 @@ export function EmailTemplate({ user, project }: Props) {
     }
   `);
 
-  // Verify it no longer exists - Prisma throws NotFoundError which becomes 500
+  // Verify it no longer exists - return 404 instead of a Prisma error
   const getRes2 = await niceBackendFetch(`/api/v1/internal/email-drafts/${draftId}`, {
     method: "GET",
     accessType: "admin",
   });
-  expect(getRes2.status).toBe(500);
+  expect(getRes2.status).toBe(404);
 
   // Verify it's not in the list
   const listRes = await niceBackendFetch("/api/v1/internal/email-drafts", {
@@ -492,6 +492,6 @@ it("should return error when deleting non-existent draft", async ({ expect }) =>
     method: "DELETE",
     accessType: "admin",
   });
-  // Prisma throws an error when the record doesn't exist
-  expect(deleteRes.status).toBe(500);
+  // Return a proper 404 so niceBackendFetch doesn't throw
+  expect(deleteRes.status).toBe(404);
 });