fix(onboarding): address final review findings — atomicity, scheduling guard, apply-all recovery, client validation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

src/features/onboarding/actions/apply-db-settings.action.ts

@@ -93,6 +93,7 @@ export const applyOnboardingDbSettingsAction = userAction
     };
 
     const applyScheduling = async () => {
+      if (backupMethod === undefined) return;
       // Direct DB update — intentionally skips the side effect in
       // updateDatabaseBackupPolicyAction that deletes retention policy on null.
       const cronValue =
@@ -105,37 +106,41 @@ export const applyOnboardingDbSettingsAction = userAction
 
     const applyNotifications = async () => {
       if (!notificationPolicies) return;
-      await db
-        .delete(drizzleDb.schemas.alertPolicy)
-        .where(eq(drizzleDb.schemas.alertPolicy.databaseId, databaseId));
+      await db.transaction(async (tx) => {
+        await tx
+          .delete(drizzleDb.schemas.alertPolicy)
+          .where(eq(drizzleDb.schemas.alertPolicy.databaseId, databaseId));
 
-      if (notificationPolicies.length > 0) {
-        await db.insert(drizzleDb.schemas.alertPolicy).values(
-          notificationPolicies.map((p) => ({
-            databaseId,
-            notificationChannelId: p.channelId,
-            eventKinds: p.eventKinds as any,
-            enabled: p.enabled,
-          }))
-        );
-      }
+        if (notificationPolicies.length > 0) {
+          await tx.insert(drizzleDb.schemas.alertPolicy).values(
+            notificationPolicies.map((p) => ({
+              databaseId,
+              notificationChannelId: p.channelId,
+              eventKinds: p.eventKinds as any,
+              enabled: p.enabled,
+            }))
+          );
+        }
+      });
     };
 
     const applyStorage = async () => {
       if (!storagePolicies) return;
-      await db
-        .delete(drizzleDb.schemas.storagePolicy)
-        .where(eq(drizzleDb.schemas.storagePolicy.databaseId, databaseId));
+      await db.transaction(async (tx) => {
+        await tx
+          .delete(drizzleDb.schemas.storagePolicy)
+          .where(eq(drizzleDb.schemas.storagePolicy.databaseId, databaseId));
 
-      if (storagePolicies.length > 0) {
-        await db.insert(drizzleDb.schemas.storagePolicy).values(
-          storagePolicies.map((p) => ({
-            databaseId,
-            storageChannelId: p.channelId,
-            enabled: p.enabled,
-          }))
-        );
-      }
+        if (storagePolicies.length > 0) {
+          await tx.insert(drizzleDb.schemas.storagePolicy).values(
+            storagePolicies.map((p) => ({
+              databaseId,
+              storageChannelId: p.channelId,
+              enabled: p.enabled,
+            }))
+          );
+        }
+      });
     };
 
     if (section === "retention" || section === "all") await applyRetention();

src/features/onboarding/steps/step-db-settings.tsx

@@ -497,7 +497,7 @@ const NotificationsSection = ({
         </Button>
         <Button
           type="button"
-          disabled={isPending}
+          disabled={isPending || policies.some((p) => !p.channelId || p.eventKinds.length === 0)}
           onClick={() => onSave(policies)}
           className="ml-auto"
         >
@@ -666,7 +666,7 @@ const StorageSection = ({
         </Button>
         <Button
           type="button"
-          disabled={isPending}
+          disabled={isPending || policies.some((p) => !p.channelId)}
           onClick={() => onSave(policies)}
           className="ml-auto"
         >
@@ -803,26 +803,42 @@ export const StepDbSettings = () => {
 
     const handleApplyToAll = async () => {
       const otherDbIds = databaseIds.filter((id) => id !== dbId);
+      const succeededIds: string[] = [];
+
       for (const targetId of otherDbIds) {
-        await applyMutation.mutateAsync({
-          databaseId: targetId,
-          section: "all",
-          retention: settings.retention,
-          backupMethod: settings.backupMethod,
-          backupCron: settings.backupCron,
-          notificationPolicies: settings.notificationPolicies as any,
-          storagePolicies: settings.storagePolicies,
+        try {
+          await applyMutation.mutateAsync({
+            databaseId: targetId,
+            section: "all",
+            retention: settings.retention,
+            backupMethod: settings.backupMethod,
+            backupCron: settings.backupCron,
+            notificationPolicies: settings.notificationPolicies,
+            storagePolicies: settings.storagePolicies,
+          });
+          succeededIds.push(targetId);
+        } catch {
+          // mutation onError already shows a toast
+        }
+      }
+
+      if (succeededIds.length > 0) {
+        const updatedSettings = { ...dbSettings };
+        succeededIds.forEach((id) => {
+          updatedSettings[id] = { ...(dbSettings[id] ?? {}), ...settings };
+        });
+        await updateContext({
+          flowData: { ...state?.context.flowData, dbSettings: updatedSettings },
         });
       }
-      const updatedSettings = { ...dbSettings };
-      otherDbIds.forEach((id) => {
-        updatedSettings[id] = { ...(dbSettings[id] ?? {}), ...settings };
-      });
-      await updateContext({
-        flowData: { ...state?.context.flowData, dbSettings: updatedSettings },
-      });
-      toast.success("Settings applied to all databases.");
-      setPhase({ kind: "grid" });
+
+      if (succeededIds.length === otherDbIds.length) {
+        toast.success("Settings applied to all databases.");
+        setPhase({ kind: "grid" });
+      } else if (succeededIds.length > 0) {
+        toast.warning(`Settings applied to ${succeededIds.length} of ${otherDbIds.length} databases.`);
+        setPhase({ kind: "grid" });
+      }
     };
 
     const SECTIONS: {