Super-admin org CRUD review fixes + name-length validation

__tests__/components/admin/organization-archived-byline.test.tsx

@@ -0,0 +1,65 @@
+import { render, screen } from "@testing-library/react";
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { OrganizationArchivedByline } from "@/components/ui/admin/organization-archived-byline";
+import type { Organization } from "@/types/organization";
+import { defaultUser } from "@/types/user";
+
+const mockUseUser = vi.fn();
+vi.mock("@/lib/api/users", () => ({
+  useUser: (...args: unknown[]) => mockUseUser(...args),
+}));
+
+function makeOrg(overrides: Partial<Organization> = {}): Organization {
+  return {
+    id: "org-1",
+    name: "Acme",
+    slug: "acme",
+    logo: "",
+    archived_at: "2026-01-01T00:00:00.000Z",
+    ...overrides,
+  } as Organization;
+}
+
+describe("OrganizationArchivedByline", () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it("shows the resolved archiver name when the lookup succeeds", () => {
+    mockUseUser.mockReturnValue({
+      user: { display_name: "Ada Admin", first_name: "Ada", last_name: "Admin" },
+      isLoading: false,
+      isError: undefined,
+    });
+    render(<OrganizationArchivedByline organization={makeOrg({ archived_by: "u-1" })} />);
+    expect(screen.getByText(/Archived .* by Ada Admin/)).toBeInTheDocument();
+  });
+
+  it("shows '…' while the archiver lookup is in flight", () => {
+    mockUseUser.mockReturnValue({
+      user: defaultUser(),
+      isLoading: true,
+      isError: undefined,
+    });
+    render(<OrganizationArchivedByline organization={makeOrg({ archived_by: "u-1" })} />);
+    expect(screen.getByText(/by …/)).toBeInTheDocument();
+  });
+
+  it("degrades to 'a former admin' when the archiver lookup errors", () => {
+    mockUseUser.mockReturnValue({
+      user: defaultUser(),
+      isLoading: false,
+      isError: new Error("403"),
+    });
+    render(<OrganizationArchivedByline organization={makeOrg({ archived_by: "u-1" })} />);
+    expect(screen.getByText(/by a former admin/)).toBeInTheDocument();
+  });
+
+  it("reads 'a former admin' when archived_by is absent (no lookup)", () => {
+    mockUseUser.mockReturnValue({
+      user: defaultUser(),
+      isLoading: false,
+      isError: undefined,
+    });
+    render(<OrganizationArchivedByline organization={makeOrg()} />);
+    expect(screen.getByText(/by a former admin/)).toBeInTheDocument();
+  });
+});

__tests__/components/admin/organization-form-dialog.test.tsx

@@ -77,6 +77,22 @@ describe("OrganizationFormDialog", () => {
     expect(mockToastSuccess).toHaveBeenCalled();
   });
 
+  it("sends the trimmed name (what is validated is what is sent)", async () => {
+    render(
+      <OrganizationFormDialog open onOpenChange={vi.fn()} onSaved={vi.fn()} />
+    );
+
+    fireEvent.change(screen.getByLabelText("Name"), {
+      target: { value: "  Padded Org  " },
+    });
+    fireEvent.click(screen.getByRole("button", { name: "Create organization" }));
+
+    await waitFor(() => expect(mockCreate).toHaveBeenCalledTimes(1));
+    expect(mockCreate).toHaveBeenCalledWith(
+      expect.objectContaining({ name: "Padded Org" })
+    );
+  });
+
   it("updates an existing organization when given one", async () => {
     const onSaved = vi.fn();
     render(
@@ -146,6 +162,77 @@ describe("OrganizationFormDialog", () => {
     expect(onOpenChange).not.toHaveBeenCalledWith(false);
   });
 
+  it("blocks submit with an inline error on an empty name (no request)", async () => {
+    render(
+      <OrganizationFormDialog open onOpenChange={vi.fn()} onSaved={vi.fn()} />
+    );
+
+    fireEvent.change(screen.getByLabelText("Name"), {
+      target: { value: "   " },
+    });
+    fireEvent.click(screen.getByRole("button", { name: "Create organization" }));
+
+    await waitFor(() =>
+      expect(
+        screen.getByText("Organization name must not be empty.")
+      ).toBeInTheDocument()
+    );
+    expect(mockCreate).not.toHaveBeenCalled();
+    expect(mockToastError).not.toHaveBeenCalled();
+  });
+
+  it("blocks submit with an inline error on a name over 255 chars (no request)", async () => {
+    render(
+      <OrganizationFormDialog open onOpenChange={vi.fn()} onSaved={vi.fn()} />
+    );
+
+    fireEvent.change(screen.getByLabelText("Name"), {
+      target: { value: "a".repeat(256) },
+    });
+    fireEvent.click(screen.getByRole("button", { name: "Create organization" }));
+
+    await waitFor(() =>
+      expect(
+        screen.getByText(
+          "Organization name must be at most 255 characters (got 256)."
+        )
+      ).toBeInTheDocument()
+    );
+    expect(mockCreate).not.toHaveBeenCalled();
+    expect(mockToastError).not.toHaveBeenCalled();
+  });
+
+  it("shows an inline name error on a server 422 validation_error (no toast)", async () => {
+    // A name that passes the client check but the server rejects (e.g. a
+    // grapheme the BE counts differently); proves the 422 fallback wires in.
+    mockCreate.mockRejectedValueOnce(
+      makeApiError(422, {
+        status_code: 422,
+        error: "validation_error",
+        message: "Organization name must be at most 255 characters (got 812).",
+      })
+    );
+    const onOpenChange = vi.fn();
+    render(
+      <OrganizationFormDialog open onOpenChange={onOpenChange} onSaved={vi.fn()} />
+    );
+
+    fireEvent.change(screen.getByLabelText("Name"), {
+      target: { value: "Valid Enough" },
+    });
+    fireEvent.click(screen.getByRole("button", { name: "Create organization" }));
+
+    await waitFor(() =>
+      expect(
+        screen.getByText(
+          "Organization name must be at most 255 characters (got 812)."
+        )
+      ).toBeInTheDocument()
+    );
+    expect(mockToastError).not.toHaveBeenCalled();
+    expect(onOpenChange).not.toHaveBeenCalledWith(false);
+  });
+
   it("shows an inline name error on a rename collision (update path)", async () => {
     mockUpdate.mockRejectedValueOnce(
       makeApiError(409, {

__tests__/components/admin/organization-row.test.tsx

@@ -17,10 +17,11 @@ vi.mock("sonner", () => ({
 const mockArchive = vi.fn();
 const mockUnarchive = vi.fn();
 vi.mock("@/lib/api/organizations", () => ({
-  OrganizationApi: {
+  useOrganizationArchiveMutation: () => ({
     archive: (...args: unknown[]) => mockArchive(...args),
     unarchive: (...args: unknown[]) => mockUnarchive(...args),
-  },
+    isLoading: false,
+  }),
   // child dialogs call this hook even while closed
   useOrganizationMutation: () => ({
     create: vi.fn(),
@@ -38,6 +39,7 @@ vi.mock("@/lib/api/users", () => ({
       last_name: "Admin",
     },
     isLoading: false,
+    isError: undefined,
   }),
 }));
 

__tests__/lib/api/organization-errors.test.ts

@@ -4,6 +4,9 @@ import {
   organizationArchivedMessage,
   organizationNameTakenMessage,
   organizationNotEmptyMessage,
+  organizationNameInvalidMessage,
+  validateOrganizationName,
+  ORGANIZATION_NAME_MAX_LENGTH,
 } from "@/lib/api/organization-errors";
 
 function apiError(status: number, data: unknown): EntityApiError {
@@ -57,3 +60,75 @@ describe("organization error discriminators", () => {
     expect(organizationNotEmptyMessage(nameTaken)).toBeNull();
   });
 });
+
+describe("organizationNameInvalidMessage (422 validation_error)", () => {
+  it("returns the backend message on a 422 validation_error", () => {
+    const err = apiError(422, {
+      error: "validation_error",
+      message: "Organization name must not be empty.",
+    });
+    expect(organizationNameInvalidMessage(err)).toBe(
+      "Organization name must not be empty."
+    );
+  });
+
+  it("falls back to a default when the 422 has no message", () => {
+    const err = apiError(422, { error: "validation_error" });
+    expect(organizationNameInvalidMessage(err)).toBe(
+      "Please enter a valid organization name."
+    );
+  });
+
+  it("ignores a validation_error that isn't a 422", () => {
+    const err = apiError(409, { error: "validation_error", message: "x" });
+    expect(organizationNameInvalidMessage(err)).toBeNull();
+  });
+
+  it("ignores a 422 that isn't validation_error, and non-errors", () => {
+    const other = apiError(422, { error: "cannot_link_completed_goal", message: "x" });
+    expect(organizationNameInvalidMessage(other)).toBeNull();
+    expect(organizationNameInvalidMessage(new Error("boom"))).toBeNull();
+    expect(organizationNameInvalidMessage(undefined)).toBeNull();
+  });
+
+  it("does not collide with the 409 name-taken discriminator", () => {
+    const nameTaken = apiError(409, {
+      error: "organization_name_taken",
+      message: "taken",
+    });
+    expect(organizationNameInvalidMessage(nameTaken)).toBeNull();
+  });
+});
+
+describe("validateOrganizationName", () => {
+  it("accepts a normal name", () => {
+    expect(validateOrganizationName("Acme")).toBeNull();
+  });
+
+  it("rejects empty / whitespace-only names", () => {
+    expect(validateOrganizationName("")).toBe(
+      "Organization name must not be empty."
+    );
+    expect(validateOrganizationName("   ")).toBe(
+      "Organization name must not be empty."
+    );
+  });
+
+  it(`accepts exactly ${ORGANIZATION_NAME_MAX_LENGTH} chars and rejects one more`, () => {
+    expect(
+      validateOrganizationName("a".repeat(ORGANIZATION_NAME_MAX_LENGTH))
+    ).toBeNull();
+    expect(
+      validateOrganizationName("a".repeat(ORGANIZATION_NAME_MAX_LENGTH + 1))
+    ).toBe(
+      `Organization name must be at most ${ORGANIZATION_NAME_MAX_LENGTH} characters (got ${
+        ORGANIZATION_NAME_MAX_LENGTH + 1
+      }).`
+    );
+  });
+
+  it("trims before measuring length", () => {
+    const padded = `  ${"a".repeat(ORGANIZATION_NAME_MAX_LENGTH)}  `;
+    expect(validateOrganizationName(padded)).toBeNull();
+  });
+});

src/components/ui/admin/organization-archived-byline.tsx

@@ -10,14 +10,17 @@ interface OrganizationArchivedBylineProps {
 
 /**
  * "Archived {date} by {name}" byline for an archived org. Resolves archived_by
- * (a user id) to a display name via GET /users/{id}; renders only when the org
- * is archived, so active rows never trigger the lookup. A missing archived_by on
- * an archived org means the archiver was since deleted (FK ON DELETE SET NULL).
+ * (a user id) to a display name via GET /users/{id} — the only lookup available,
+ * since archivers are platform SuperAdmins not scoped to any one org's user list.
+ * Renders only when the org is archived, so active rows never trigger the lookup.
+ * Shows "…" only while the lookup is in flight; degrades to "a former admin" when
+ * archived_by is absent (FK ON DELETE SET NULL) or the lookup fails/returns no name.
  */
 export function OrganizationArchivedByline({
   organization,
 }: OrganizationArchivedBylineProps) {
-  const { user } = useUser(organization.archived_by ?? "");
+  const hasArchiver = Boolean(organization.archived_by);
+  const { user, isLoading, isError } = useUser(organization.archived_by ?? "");
 
   const when = organization.archived_at
     ? DateTime.fromISO(organization.archived_at)
@@ -26,15 +29,16 @@ export function OrganizationArchivedByline({
   const resolvedName =
     user.display_name.trim() || `${user.first_name} ${user.last_name}`.trim();
 
-  const archiver = !organization.archived_by
-    ? "a former admin"
-    : resolvedName || "…";
+  const archiverLabel =
+    !hasArchiver || isError
+      ? "a former admin"
+      : resolvedName || (isLoading ? "…" : "a former admin");
 
   return (
     <p className="text-xs text-muted-foreground tabular-nums">
       Archived
       {when?.isValid ? ` ${when.toLocaleString(DateTime.DATE_MED)}` : ""} by{" "}
-      {archiver}
+      {archiverLabel}
     </p>
   );
 }

src/components/ui/admin/organization-form-dialog.tsx

@@ -15,7 +15,11 @@ import {
 import { Label } from "@/components/ui/label";
 import { Input } from "@/components/ui/input";
 import { useOrganizationMutation } from "@/lib/api/organizations";
-import { organizationNameTakenMessage } from "@/lib/api/organization-errors";
+import {
+  organizationNameTakenMessage,
+  organizationNameInvalidMessage,
+  validateOrganizationName,
+} from "@/lib/api/organization-errors";
 import { Organization, defaultOrganization } from "@/types/organization";
 
 interface OrganizationFormDialogProps {
@@ -55,29 +59,41 @@ export function OrganizationFormDialog({
   const handleSubmit = async (e: FormEvent) => {
     e.preventDefault();
 
+    // Send exactly what we validate (and what the backend stores): the trimmed
+    // name — so a whitespace-padded value can't pass the pre-check yet differ
+    // from what hits the wire.
+    const name = formData.name.trim();
+    const invalidName = validateOrganizationName(name);
+    if (invalidName !== null) {
+      setNameError(invalidName);
+      return;
+    }
+
     try {
       if (isEdit) {
         await update(organization.id, {
           ...organization,
-          name: formData.name,
+          name,
           logo: formData.logo,
         });
-        toast.success(`Organization "${formData.name}" updated`);
+        toast.success(`Organization "${name}" updated`);
       } else {
         await create({
           ...defaultOrganization(),
-          name: formData.name,
+          name,
           logo: formData.logo,
         });
-        toast.success(`Organization "${formData.name}" created`);
+        toast.success(`Organization "${name}" created`);
       }
       onSaved();
       onOpenChange(false);
     } catch (error) {
       console.error("Error saving organization:", error);
-      const nameTaken = organizationNameTakenMessage(error);
-      if (nameTaken !== null) {
-        setNameError(nameTaken);
+      const nameError =
+        organizationNameTakenMessage(error) ??
+        organizationNameInvalidMessage(error);
+      if (nameError !== null) {
+        setNameError(nameError);
       } else {
         toast.error(
           isEdit