diff --git a/app/routes/settings/auth-keys/overview.tsx b/app/routes/settings/auth-keys/overview.tsx index 7f8a5b9..26389c7 100644 --- a/app/routes/settings/auth-keys/overview.tsx +++ b/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) => ( <> - {user.name || user.displayName || user.email || user.id} + {getUserDisplayName(user)} {index < missing.length - 1 ? ", " : ". "} ))} @@ -178,9 +177,7 @@ export default function Page({ {[ All, ...keys.map(({ user }) => ( - - {user.name || user.displayName || user.email || user.id} - + {getUserDisplayName(user)} )), ]} diff --git a/app/utils/user.ts b/app/utils/user.ts new file mode 100644 index 0000000..e79adfe --- /dev/null +++ b/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; +} diff --git a/tests/unit/utils/user.test.ts b/tests/unit/utils/user.test.ts new file mode 100644 index 0000000..69a7494 --- /dev/null +++ b/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 => ({ + 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"); + }); +});