refactor: simplify dialog - drop unnecessary hooks, parallelize DB calls, extract constants

nicktrn · nicktrn · commit 9eaae9d7d89f · 2026-03-28T20:22:31.000Z
diff --git a/apps/webapp/app/components/admin/FeatureFlagsDialog.tsx b/apps/webapp/app/components/admin/FeatureFlagsDialog.tsx
@@ -1,5 +1,5 @@
 import { useFetcher } from "@remix-run/react";
-import { useCallback, useEffect, useMemo, useState } from "react";
+import { useEffect, useMemo, useState } from "react";
 import {
   Dialog,
   DialogContent,
@@ -14,6 +14,10 @@ import { Input } from "~/components/primitives/Input";
 import { cn } from "~/utils/cn";
 import type { FlagControlType } from "~/v3/featureFlags.server";
 
+const UNSET_VALUE = "__unset__";
+
+const HIDDEN_FLAGS = ["defaultWorkerInstanceGroupId"];
+
 type LoaderData = {
   org: { id: string; title: string; slug: string };
   orgFlags: Record<string, unknown>;
@@ -42,18 +46,15 @@ export function FeatureFlagsDialog({
   const loadFetcher = useFetcher<LoaderData>();
   const saveFetcher = useFetcher<ActionData>();
 
-  // Local state for edits - keyed by flag name, value is the override or undefined (unset)
   const [overrides, setOverrides] = useState<Record<string, unknown>>({});
   const [initialOverrides, setInitialOverrides] = useState<Record<string, unknown>>({});
 
-  // Load flags when dialog opens
   useEffect(() => {
     if (open && orgId) {
       loadFetcher.load(`/admin/api/orgs/${orgId}/feature-flags`);
     }
   }, [open, orgId]);
 
-  // Sync loaded data into local state
   useEffect(() => {
     if (loadFetcher.data) {
       const loaded = loadFetcher.data.orgFlags ?? {};
@@ -62,7 +63,6 @@ export function FeatureFlagsDialog({
     }
   }, [loadFetcher.data]);
 
-  // Close on successful save
   useEffect(() => {
     if (saveFetcher.data?.success) {
       onOpenChange(false);
@@ -73,45 +73,40 @@ export function FeatureFlagsDialog({
     return JSON.stringify(overrides) !== JSON.stringify(initialOverrides);
   }, [overrides, initialOverrides]);
 
-  const setFlagValue = useCallback((key: string, value: unknown) => {
+  const setFlagValue = (key: string, value: unknown) => {
     setOverrides((prev) => ({ ...prev, [key]: value }));
-  }, []);
+  };
 
-  const unsetFlag = useCallback((key: string) => {
+  const unsetFlag = (key: string) => {
     setOverrides((prev) => {
       const next = { ...prev };
       delete next[key];
       return next;
     });
-  }, []);
+  };
 
-  const handleSave = useCallback(() => {
+  const handleSave = () => {
     if (!orgId) return;
-
     const body = Object.keys(overrides).length === 0 ? null : overrides;
-
     saveFetcher.submit(JSON.stringify(body), {
       method: "POST",
       action: `/admin/api/orgs/${orgId}/feature-flags`,
       encType: "application/json",
     });
-  }, [orgId, overrides, saveFetcher]);
+  };
 
   const data = loadFetcher.data;
   const isLoading = loadFetcher.state === "loading";
   const isSaving = saveFetcher.state === "submitting";
 
-  // Build JSON preview
-  const jsonPreview = useMemo(() => {
-    if (Object.keys(overrides).length === 0) return "null";
-    return JSON.stringify(overrides, null, 2);
-  }, [overrides]);
+  const jsonPreview =
+    Object.keys(overrides).length === 0 ? "null" : JSON.stringify(overrides, null, 2);
 
-  // Sort flag keys alphabetically
-  const sortedFlagKeys = useMemo(() => {
-    if (!data) return [];
-    return Object.keys(data.controlTypes).sort();
-  }, [data]);
+  const sortedFlagKeys = data
+    ? Object.keys(data.controlTypes)
+        .filter((key) => !HIDDEN_FLAGS.includes(key))
+        .sort()
+    : [];
 
   return (
     <Dialog open={open} onOpenChange={onOpenChange}>
@@ -126,9 +121,7 @@ export function FeatureFlagsDialog({
             <div className="py-8 text-center text-sm text-text-dimmed">Loading flags...</div>
           ) : data ? (
             <div className="flex flex-col gap-1.5">
-              {sortedFlagKeys
-                .filter((key) => key !== "defaultWorkerInstanceGroupId")
-                .map((key) => {
+              {sortedFlagKeys.map((key) => {
                 const control = data.controlTypes[key];
                 const isOverridden = key in overrides;
                 const globalValue = data.globalFlags[key as keyof typeof data.globalFlags];
@@ -179,7 +172,7 @@ export function FeatureFlagsDialog({
                           value={isOverridden ? (overrides[key] as string) : undefined}
                           options={control.options}
                           onChange={(val) => {
-                            if (val === "__unset__") {
+                            if (val === UNSET_VALUE) {
                               unsetFlag(key);
                             } else {
                               setFlagValue(key, val);
@@ -210,7 +203,6 @@ export function FeatureFlagsDialog({
           ) : null}
         </div>
 
-        {/* JSON Preview */}
         {data && (
           <details className="mt-2">
             <summary className="cursor-pointer text-xs text-text-dimmed hover:text-text-bright">
@@ -271,21 +263,21 @@ function EnumControl({
   onChange: (val: string) => void;
   dimmed: boolean;
 }) {
-  const items = ["__unset__", ...options];
+  const items = [UNSET_VALUE, ...options];
 
   return (
     <Select
       variant="tertiary/small"
-      value={value ?? "__unset__"}
+      value={value ?? UNSET_VALUE}
       setValue={onChange}
       items={items}
-      text={(val) => (val === "__unset__" ? "unset" : val)}
+      text={(val) => (val === UNSET_VALUE ? "unset" : val)}
       className={cn(dimmed && "opacity-50")}
     >
       {(items) =>
         items.map((item) => (
           <SelectItem key={item} value={item}>
-            {item === "__unset__" ? "unset" : item}
+            {item === UNSET_VALUE ? "unset" : item}
           </SelectItem>
         ))
       }
diff --git a/apps/webapp/app/models/admin.server.ts b/apps/webapp/app/models/admin.server.ts
@@ -133,7 +133,6 @@ export async function adminGetOrganizations(userId: string, { page, search }: Se
       v2Enabled: true,
       v3Enabled: true,
       deletedAt: true,
-      featureFlags: true,
       members: {
         select: {
           user: {
diff --git a/apps/webapp/app/routes/admin.api.orgs.$orgId.feature-flags.ts b/apps/webapp/app/routes/admin.api.orgs.$orgId.feature-flags.ts
@@ -22,15 +22,18 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
 
   const { orgId } = ParamsSchema.parse(params);
 
-  const organization = await prisma.organization.findUnique({
-    where: { id: orgId },
-    select: {
-      id: true,
-      title: true,
-      slug: true,
-      featureFlags: true,
-    },
-  });
+  const [organization, globalFlags] = await Promise.all([
+    prisma.organization.findUnique({
+      where: { id: orgId },
+      select: {
+        id: true,
+        title: true,
+        slug: true,
+        featureFlags: true,
+      },
+    }),
+    getGlobalFlags(),
+  ]);
 
   if (!organization) {
     throw new Response("Organization not found", { status: 404 });
@@ -41,7 +44,6 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
     : ({ success: false } as const);
 
   const orgFlags = orgFlagsResult.success ? orgFlagsResult.data : {};
-  const globalFlags = await getGlobalFlags();
   const controlTypes = getAllFlagControlTypes();
 
   return json({
@@ -63,39 +65,33 @@ export async function action({ request, params }: ActionFunctionArgs) {
   }
 
   const { orgId } = ParamsSchema.parse(params);
-
-  const organization = await prisma.organization.findUnique({
-    where: { id: orgId },
-    select: { id: true },
-  });
-
-  if (!organization) {
-    throw new Response("Organization not found", { status: 404 });
-  }
-
   const body = await request.json();
 
-  // body is the full overrides object (or null to clear all)
-  if (body === null || (typeof body === "object" && Object.keys(body).length === 0)) {
+  const featureFlags =
+    body === null || (typeof body === "object" && Object.keys(body).length === 0)
+      ? Prisma.JsonNull
+      : (() => {
+          const validationResult = validatePartialFeatureFlags(body as Record<string, unknown>);
+          if (!validationResult.success) {
+            throw json(
+              { error: "Invalid feature flags", details: validationResult.error.issues },
+              { status: 400 }
+            );
+          }
+          return validationResult.data;
+        })();
+
+  try {
     await prisma.organization.update({
       where: { id: orgId },
-      data: { featureFlags: Prisma.JsonNull },
+      data: { featureFlags },
     });
-    return json({ success: true });
+  } catch (e) {
+    if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2025") {
+      throw new Response("Organization not found", { status: 404 });
+    }
+    throw e;
   }
 
-  const validationResult = validatePartialFeatureFlags(body as Record<string, unknown>);
-  if (!validationResult.success) {
-    return json(
-      { error: "Invalid feature flags", details: validationResult.error.issues },
-      { status: 400 }
-    );
-  }
-
-  await prisma.organization.update({
-    where: { id: orgId },
-    data: { featureFlags: validationResult.data },
-  });
-
   return json({ success: true });
 }
diff --git a/apps/webapp/app/routes/admin.orgs.tsx b/apps/webapp/app/routes/admin.orgs.tsx
@@ -49,12 +49,11 @@ export default function AdminDashboardRoute() {
   const { organizations, filters, page, pageCount } = useTypedLoaderData<typeof loader>();
 
   const [flagsOrgId, setFlagsOrgId] = useState<string | null>(null);
-  const [flagsOrgTitle, setFlagsOrgTitle] = useState("");
   const [flagsOpen, setFlagsOpen] = useState(false);
+  const flagsOrgTitle = organizations.find((o) => o.id === flagsOrgId)?.title ?? "";
 
-  const openFlagsDialog = (orgId: string, orgTitle: string) => {
+  const openFlagsDialog = (orgId: string) => {
     setFlagsOrgId(orgId);
-    setFlagsOrgTitle(orgTitle);
     setFlagsOpen(true);
   };
 
@@ -126,23 +125,23 @@ export default function AdminDashboardRoute() {
                     <TableCell>{org.deletedAt ? "☠️" : ""}</TableCell>
                     <TableCell isSticky={true}>
                       <div className="flex items-center gap-2">
-                      <Button
-                        variant="tertiary/small"
-                        onClick={() => openFlagsDialog(org.id, org.title)}
-                      >
-                        Flags
-                      </Button>
-                      <LinkButton
-                        to={`/@/orgs/${org.slug}`}
-                        variant="tertiary/small"
-                        shortcut={
-                          organizations.length === 1
-                            ? { modifiers: ["mod"], key: "enter", enabledOnInputElements: true }
-                            : undefined
-                        }
-                      >
-                        Impersonate
-                      </LinkButton>
+                        <Button
+                          variant="tertiary/small"
+                          onClick={() => openFlagsDialog(org.id)}
+                        >
+                          Flags
+                        </Button>
+                        <LinkButton
+                          to={`/@/orgs/${org.slug}`}
+                          variant="tertiary/small"
+                          shortcut={
+                            organizations.length === 1
+                              ? { modifiers: ["mod"], key: "enter", enabledOnInputElements: true }
+                              : undefined
+                          }
+                        >
+                          Impersonate
+                        </LinkButton>
                       </div>
                     </TableCell>
                   </TableRow>
