Add user utility functions and tests for OIDC user filtering

app/routes/settings/auth-keys/overview.tsx

@@ -9,6 +9,7 @@ import Select from "~/components/Select";
 import TableList from "~/components/TableList";
 import { Capabilities } from "~/server/web/roles";
 import log from "~/utils/log";
+import { filterUsersWithValidIds, getUserDisplayName } from "~/utils/user";
 
 import type { Route } from "./+types/overview";
 
@@ -22,26 +23,24 @@ export async function loader({ request, context }: Route.LoaderArgs) {
 
   const users = await api.getUsers();
   const preAuthKeys = await Promise.all(
-    users
-      .filter((user) => user.id?.length > 0) // Filter out users without valid IDs
-      .map(async (user) => {
-        try {
-          const preAuthKeys = await api.getPreAuthKeys(user.id);
-          return {
-            success: true,
-            user,
-            preAuthKeys,
-          };
-        } catch (error) {
-          log.error("api", "GET /v1/preauthkey for %s: %o", user.name, error);
-          return {
-            success: false,
-            user,
-            error,
-            preAuthKeys: [],
-          };
-        }
-      }),
+    filterUsersWithValidIds(users).map(async (user) => {
+      try {
+        const preAuthKeys = await api.getPreAuthKeys(user.id);
+        return {
+          success: true,
+          user,
+          preAuthKeys,
+        };
+      } catch (error) {
+        log.error("api", "GET /v1/preauthkey for %s: %o", user.name, error);
+        return {
+          success: false,
+          user,
+          error,
+          preAuthKeys: [],
+        };
+      }
+    }),
   );
 
   const keys = preAuthKeys
@@ -147,7 +146,7 @@ export default function Page({
           An error occurred while fetching the authentication keys for the following users:{" "}
           {missing.map(({ user }, index) => (
             <>
-              <Code key={user.id}>{user.name || user.displayName || user.email || user.id}</Code>
+              <Code key={user.id}>{getUserDisplayName(user)}</Code>
               {index < missing.length - 1 ? ", " : ". "}
             </>
           ))}
@@ -178,9 +177,7 @@ export default function Page({
           {[
             <Select.Item key="__headplane_all">All</Select.Item>,
             ...keys.map(({ user }) => (
-              <Select.Item key={user.id}>
-                {user.name || user.displayName || user.email || user.id}
-              </Select.Item>
+              <Select.Item key={user.id}>{getUserDisplayName(user)}</Select.Item>
             )),
           ]}
         </Select>

app/utils/user.ts

@@ -0,0 +1,11 @@
+import type { User } from "~/types/User";
+
+// Filter users with valid IDs (OIDC users may not have a name)
+export function filterUsersWithValidIds(users: User[]): User[] {
+  return users.filter((user) => user.id?.length > 0);
+}
+
+// Get display name with fallback: name -> displayName -> email -> id
+export function getUserDisplayName(user: User): string {
+  return user.name || user.displayName || user.email || user.id;
+}

tests/unit/utils/user.test.ts

@@ -0,0 +1,101 @@
+import { describe, expect, test } from "vitest";
+
+import type { User } from "~/types/User";
+
+import { filterUsersWithValidIds, getUserDisplayName } from "~/utils/user";
+
+const makeUser = (overrides: Partial<User>): User => ({
+  id: "default-id",
+  name: "",
+  createdAt: "2024-01-01T00:00:00Z",
+  ...overrides,
+});
+
+describe("filterUsersWithValidIds", () => {
+  test("keeps users with valid ID and name", () => {
+    const users = [makeUser({ id: "123", name: "John" })];
+    const result = filterUsersWithValidIds(users);
+    expect(result).toHaveLength(1);
+    expect(result[0].id).toBe("123");
+  });
+
+  test("keeps users with valid ID but no name", () => {
+    const users = [makeUser({ id: "123", name: "" })];
+    const result = filterUsersWithValidIds(users);
+    expect(result).toHaveLength(1);
+  });
+
+  test("keeps users with valid ID and no optional fields", () => {
+    const users = [makeUser({ id: "123", name: "", displayName: undefined, email: undefined })];
+    const result = filterUsersWithValidIds(users);
+    expect(result).toHaveLength(1);
+  });
+
+  test("removes users with empty ID", () => {
+    const users = [makeUser({ id: "", name: "John" })];
+    const result = filterUsersWithValidIds(users);
+    expect(result).toHaveLength(0);
+  });
+
+  test("handles mix of valid and invalid", () => {
+    const users = [
+      makeUser({ id: "123", name: "John" }),
+      makeUser({ id: "", name: "Jane" }),
+      makeUser({ id: "456", name: "" }),
+    ];
+    const result = filterUsersWithValidIds(users);
+    expect(result).toHaveLength(2);
+    expect(result.map((u) => u.id)).toEqual(["123", "456"]);
+  });
+
+  test("returns empty for empty input", () => {
+    expect(filterUsersWithValidIds([])).toHaveLength(0);
+  });
+});
+
+describe("getUserDisplayName", () => {
+  test("uses name when set", () => {
+    const user = makeUser({ id: "123", name: "John" });
+    expect(getUserDisplayName(user)).toBe("John");
+  });
+
+  test("uses displayName when name is empty", () => {
+    const user = makeUser({ id: "123", name: "", displayName: "John Doe" });
+    expect(getUserDisplayName(user)).toBe("John Doe");
+  });
+
+  test("uses email when name and displayName are empty", () => {
+    const user = makeUser({ id: "123", name: "", displayName: "", email: "john@example.com" });
+    expect(getUserDisplayName(user)).toBe("john@example.com");
+  });
+
+  test("uses id when everything else is empty", () => {
+    const user = makeUser({ id: "123", name: "", displayName: "", email: "" });
+    expect(getUserDisplayName(user)).toBe("123");
+  });
+
+  test("uses id when optional fields are undefined", () => {
+    const user = makeUser({ id: "123", name: "", displayName: undefined, email: undefined });
+    expect(getUserDisplayName(user)).toBe("123");
+  });
+
+  test("prefers name over displayName", () => {
+    const user = makeUser({
+      id: "123",
+      name: "John",
+      displayName: "John Doe",
+      email: "john@example.com",
+    });
+    expect(getUserDisplayName(user)).toBe("John");
+  });
+
+  test("prefers displayName over email", () => {
+    const user = makeUser({
+      id: "123",
+      name: "",
+      displayName: "John Doe",
+      email: "john@example.com",
+    });
+    expect(getUserDisplayName(user)).toBe("John Doe");
+  });
+});