diff --git a/__tests__/components/admin/organization-archived-byline.test.tsx b/__tests__/components/admin/organization-archived-byline.test.tsx new file mode 100644 index 00000000..55be4368 --- /dev/null +++ b/__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 { + 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(); + 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(); + 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(); + 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(); + expect(screen.getByText(/by a former admin/)).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/admin/organization-form-dialog.test.tsx b/__tests__/components/admin/organization-form-dialog.test.tsx index e5bcd981..fe1cc09b 100644 --- a/__tests__/components/admin/organization-form-dialog.test.tsx +++ b/__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( + + ); + + 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( + + ); + + 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( + + ); + + 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( + + ); + + 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, { diff --git a/__tests__/components/admin/organization-row.test.tsx b/__tests__/components/admin/organization-row.test.tsx index c307fad2..8b9974e6 100644 --- a/__tests__/components/admin/organization-row.test.tsx +++ b/__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, }), })); diff --git a/__tests__/lib/api/organization-errors.test.ts b/__tests__/lib/api/organization-errors.test.ts index 9615a8c0..97144b1c 100644 --- a/__tests__/lib/api/organization-errors.test.ts +++ b/__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(); + }); +}); diff --git a/src/components/ui/admin/organization-archived-byline.tsx b/src/components/ui/admin/organization-archived-byline.tsx index 828b6378..59ab7752 100644 --- a/src/components/ui/admin/organization-archived-byline.tsx +++ b/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 (

Archived {when?.isValid ? ` ${when.toLocaleString(DateTime.DATE_MED)}` : ""} by{" "} - {archiver} + {archiverLabel}

); } diff --git a/src/components/ui/admin/organization-form-dialog.tsx b/src/components/ui/admin/organization-form-dialog.tsx index 7851aa10..ab77ec28 100644 --- a/src/components/ui/admin/organization-form-dialog.tsx +++ b/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 diff --git a/src/components/ui/admin/organization-row.tsx b/src/components/ui/admin/organization-row.tsx index 5439d88a..7f7a3712 100644 --- a/src/components/ui/admin/organization-row.tsx +++ b/src/components/ui/admin/organization-row.tsx @@ -13,7 +13,7 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; -import { OrganizationApi } from "@/lib/api/organizations"; +import { useOrganizationArchiveMutation } from "@/lib/api/organizations"; import { Organization, isOrganizationArchived } from "@/types/organization"; import { OrganizationFormDialog } from "./organization-form-dialog"; import { DeleteOrganizationDialog } from "./delete-organization-dialog"; @@ -33,7 +33,8 @@ export function OrganizationRow({ }: OrganizationRowProps) { const [editOpen, setEditOpen] = useState(false); const [deleteOpen, setDeleteOpen] = useState(false); - const [isArchiving, setIsArchiving] = useState(false); + const { archive, unarchive, isLoading: isArchiving } = + useOrganizationArchiveMutation(); const archived = isOrganizationArchived(organization); @@ -43,13 +44,12 @@ export function OrganizationRow({ : organization.created_at; const handleArchiveToggle = async () => { - setIsArchiving(true); try { if (archived) { - await OrganizationApi.unarchive(organization.id); + await unarchive(organization.id); toast.success(`Organization "${organization.name}" unarchived`); } else { - await OrganizationApi.archive(organization.id); + await archive(organization.id); toast.success(`Organization "${organization.name}" archived`); } onChanged(); @@ -60,8 +60,6 @@ export function OrganizationRow({ ? "There was an error unarchiving the organization" : "There was an error archiving the organization" ); - } finally { - setIsArchiving(false); } }; diff --git a/src/components/ui/admin/organizations-admin-section.tsx b/src/components/ui/admin/organizations-admin-section.tsx index 1075fa17..63e0efb3 100644 --- a/src/components/ui/admin/organizations-admin-section.tsx +++ b/src/components/ui/admin/organizations-admin-section.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useMemo, useState } from "react"; import { Plus } from "lucide-react"; import { Button } from "@/components/ui/button"; import { Card, CardContent } from "@/components/ui/card"; @@ -24,8 +24,9 @@ export function OrganizationsAdminSection() { useAllOrganizations(status); const [createOpen, setCreateOpen] = useState(false); - const sorted = [...organizations].sort((a, b) => - a.name.localeCompare(b.name) + const sorted = useMemo( + () => [...organizations].sort((a, b) => a.name.localeCompare(b.name)), + [organizations] ); return ( diff --git a/src/components/ui/app-sidebar.tsx b/src/components/ui/app-sidebar.tsx index 2c12f241..1456ecec 100644 --- a/src/components/ui/app-sidebar.tsx +++ b/src/components/ui/app-sidebar.tsx @@ -183,7 +183,7 @@ export function AppSidebar({ ...props }: React.ComponentProps) { - {isAdminOrSuperAdmin(currentUserRoleState) && ( + {(isSuperAdmin || isAdminOrSuperAdmin(currentUserRoleState)) && ( ) { )} - - - - - Members - - - + {currentOrganizationId && ( + + + + + Members + + + + )} diff --git a/src/lib/api/entity-api.ts b/src/lib/api/entity-api.ts index 15228712..b1bf7f58 100644 --- a/src/lib/api/entity-api.ts +++ b/src/lib/api/entity-api.ts @@ -1,6 +1,11 @@ import { Id, EntityApiError, EMPTY_ARRAY } from "@/types/general"; import { useState } from "react"; -import useSWR, { KeyedMutator, SWRConfiguration, useSWRConfig } from "swr"; +import useSWR, { + KeyedMutator, + ScopedMutator, + SWRConfiguration, + useSWRConfig, +} from "swr"; import { sessionGuard } from "@/lib/auth/session-guard"; import axios, { type AxiosRequestConfig } from "axios"; @@ -429,6 +434,27 @@ export namespace EntityApi { }; }; + /** + * Invalidates every SWR cache entry whose key targets the given base URL — + * both single-entity string keys (from {@link useEntity}) and list tuple keys + * `[url, params]` (from {@link useEntityList}). Use for out-of-band mutations + * (e.g. sub-action POSTs) that don't flow through {@link useEntityMutation}. + * + * @param mutate The global mutator from `useSWRConfig` + * @param baseUrl The entity base URL to match cache keys against + */ + export const invalidateEntityCache = ( + mutate: ScopedMutator, + baseUrl: string + ) => + mutate((key) => { + if (typeof key === "string") return key.includes(baseUrl); + if (Array.isArray(key) && typeof key[0] === "string") { + return key[0].includes(baseUrl); + } + return false; + }); + /** * A generic hook for entity mutations that manages loading and error states * and handles cache invalidation. @@ -464,15 +490,7 @@ export namespace EntityApi { try { const result = await operation(); - // Invalidate both single-entity caches (string keys from `useEntity`) - // and list caches (tuple keys `[url, params]` from `useEntityList`). - mutate((key) => { - if (typeof key === "string") return key.includes(baseUrl); - if (Array.isArray(key) && typeof key[0] === "string") { - return key[0].includes(baseUrl); - } - return false; - }); + invalidateEntityCache(mutate, baseUrl); return result; } catch (err) { // Handle both EntityApiError and regular Error types diff --git a/src/lib/api/organization-errors.ts b/src/lib/api/organization-errors.ts index 4dc8ed2b..7fa0cc3c 100644 --- a/src/lib/api/organization-errors.ts +++ b/src/lib/api/organization-errors.ts @@ -37,3 +37,39 @@ export const organizationArchivedMessage = (error: unknown): string | null => "organization_archived", "This organization is archived and can't accept new changes." ); + +/** Longest organization name the backend accepts (characters, trimmed). */ +export const ORGANIZATION_NAME_MAX_LENGTH = 255; + +/** + * A `422 validation_error` from an organization create/rename — an empty/ + * whitespace or over-{@link ORGANIZATION_NAME_MAX_LENGTH}-char name. Returns the + * backend's message, else null. `validation_error` is a SHARED discriminator, so + * only call this from the org create/edit form (never treat it as a global org + * error). Distinct from the `organization_name_taken` 409 (a uniqueness collision). + */ +export const organizationNameInvalidMessage = (error: unknown): string | null => { + if ( + EntityApiError.isEntityApiError(error) && + error.status === 422 && + error.data?.error === "validation_error" + ) { + return typeof error.data?.message === "string" + ? error.data.message + : "Please enter a valid organization name."; + } + return null; +}; + +/** + * Client-side mirror of the backend name rule, so the dialog can flag a bad name + * before the round-trip. Returns an inline message, or null when the name is ok. + */ +export const validateOrganizationName = (name: string): string | null => { + const trimmed = name.trim(); + if (trimmed.length === 0) return "Organization name must not be empty."; + if (trimmed.length > ORGANIZATION_NAME_MAX_LENGTH) { + return `Organization name must be at most ${ORGANIZATION_NAME_MAX_LENGTH} characters (got ${trimmed.length}).`; + } + return null; +}; diff --git a/src/lib/api/organizations.ts b/src/lib/api/organizations.ts index ec6535b2..2767fb17 100644 --- a/src/lib/api/organizations.ts +++ b/src/lib/api/organizations.ts @@ -1,5 +1,7 @@ // Interacts with the Organizations endpoints +import { useState } from "react"; +import { useSWRConfig } from "swr"; import { siteConfig } from "@/site.config"; import { Id } from "@/types/general"; import { @@ -248,3 +250,37 @@ export const useOrganizationMutation = () => { deleteNested: OrganizationApi.deleteNested, }); }; + +/** + * Hook for the archive/unarchive lifecycle actions, which are sub-action POSTs + * rather than standard CRUD and so don't flow through {@link useOrganizationMutation}. + * Each call invalidates every organization cache — including the sibling + * active/archived/all {@link useAllOrganizations} keys — so switching status + * tabs after a toggle reflects the change without waiting for a stale-revalidate. + * + * @returns An object containing: + * * archive: Archives an organization by id + * * unarchive: Restores an archived organization by id + * * isLoading: Boolean indicating if a toggle is in progress + */ +export const useOrganizationArchiveMutation = () => { + const { mutate } = useSWRConfig(); + const [isLoading, setIsLoading] = useState(false); + + const run = async (operation: () => Promise) => { + setIsLoading(true); + try { + const result = await operation(); + EntityApi.invalidateEntityCache(mutate, ORGANIZATIONS_BASEURL); + return result; + } finally { + setIsLoading(false); + } + }; + + return { + archive: (id: Id) => run(() => OrganizationApi.archive(id)), + unarchive: (id: Id) => run(() => OrganizationApi.unarchive(id)), + isLoading, + }; +}; diff --git a/src/types/user.ts b/src/types/user.ts index ee54b020..3e83be8b 100644 --- a/src/types/user.ts +++ b/src/types/user.ts @@ -173,11 +173,13 @@ export function isAdminOrSuperAdmin(roleState: UserRoleState): boolean { * context. Use this to gate platform-wide features (e.g. the /admin section). * * @param roles - Array of UserRole assignments for the user - * @returns true if the user has a SuperAdmin role with organization_id === null + * @returns true if the user has a SuperAdmin role with no organization scope */ export function isSuperAdmin(roles: UserRole[]): boolean { + // Loose `== null` tolerates a role serialized with organization_id omitted + // (undefined) as well as explicit null, matching isOrganizationArchived. return roles.some( - (r) => r.role === Role.SuperAdmin && r.organization_id === null + (r) => r.role === Role.SuperAdmin && r.organization_id == null ); }