From 8d18130bd40a1c8a34764eaca068585738d3947d Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 17:25:16 +0100 Subject: [PATCH 1/8] feat(users): add core list filters --- src/core/users/filter.test.ts | 363 ++++++++++++++++++++++++++++++++++ src/core/users/filter.ts | 231 ++++++++++++++++++++++ 2 files changed, 594 insertions(+) create mode 100644 src/core/users/filter.test.ts create mode 100644 src/core/users/filter.ts diff --git a/src/core/users/filter.test.ts b/src/core/users/filter.test.ts new file mode 100644 index 00000000..0ecb3588 --- /dev/null +++ b/src/core/users/filter.test.ts @@ -0,0 +1,363 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + parseFilterValues, + hasClientFilters, + userFullName, + userGlobalRoleIds, + resolveRoleIds, + fetchAllUsers, + filterUsers, + buildRoleNameMap, + formatUserRoles, + USER_FILTER_SCAN_PAGE_SIZE, + type UserClientFilters, +} from './filter.js'; +import type { APIClient } from '../../api-client/api-client.js'; + +const mockClient = { + listUsers: vi.fn(), + listRoles: vi.fn(), +} as unknown as APIClient; + +beforeEach(() => vi.clearAllMocks()); + +describe('parseFilterValues', () => { + it('returns [] for undefined/empty/whitespace input', () => { + expect(parseFilterValues(undefined)).toEqual([]); + expect(parseFilterValues('')).toEqual([]); + expect(parseFilterValues(' ')).toEqual([]); + }); + + it('splits comma-separated values, trims, drops blanks', () => { + expect(parseFilterValues('Ancillaries, Finance , ,API User')).toEqual([ + 'Ancillaries', + 'Finance', + 'API User', + ]); + }); + + it('dedupes case-insensitively keeping first spelling', () => { + expect(parseFilterValues('Ancillaries,ancillaries,ANCILLARIES')).toEqual(['Ancillaries']); + }); +}); + +describe('hasClientFilters', () => { + it('is false when no client filter flags are set', () => { + expect(hasClientFilters({})).toBe(false); + expect(hasClientFilters({ search: 'x', sort: 'name' })).toBe(false); + }); + + it('is true when any client filter is set', () => { + expect(hasClientFilters({ department: 'Ancillaries' })).toBe(true); + expect(hasClientFilters({ role: 'API User' })).toBe(true); + expect(hasClientFilters({ jobTitle: 'CEO' })).toBe(true); + expect(hasClientFilters({ email: 'a@b.com' })).toBe(true); + expect(hasClientFilters({ name: 'Alice' })).toBe(true); + }); + + it('is false for empty-string values', () => { + expect(hasClientFilters({ department: '', role: ' ' })).toBe(false); + }); +}); + +describe('userFullName', () => { + it('joins first and last name with a space', () => { + expect(userFullName({ first_name: 'Alice', last_name: 'Smith' })).toBe('Alice Smith'); + }); + + it('omits missing parts', () => { + expect(userFullName({ first_name: 'Alice', last_name: '' })).toBe('Alice'); + expect(userFullName({ first_name: '', last_name: 'Smith' })).toBe('Smith'); + expect(userFullName({})).toBe(''); + }); +}); + +describe('userGlobalRoleIds', () => { + it('extracts role_ids from user_team_roles entries', () => { + expect( + userGlobalRoleIds({ + user_team_roles: [ + { user_id: 1, team_id: 10, role_ids: [5, 7] }, + { user_id: 1, team_id: 20, role_ids: [9] }, + ], + }) + ).toEqual([5, 7, 9]); + }); + + it('returns [] when user_team_roles is missing or malformed', () => { + expect(userGlobalRoleIds({})).toEqual([]); + expect(userGlobalRoleIds({ user_team_roles: 'nope' })).toEqual([]); + expect(userGlobalRoleIds({ user_team_roles: [{ role_ids: 'nope' }] })).toEqual([]); + }); + + it('tolerates entries without role_ids', () => { + expect( + userGlobalRoleIds({ + user_team_roles: [{ team_id: 10 }, { team_id: 20, role_ids: [3] }, { role_ids: [4, 6] }], + }) + ).toEqual([3, 4, 6]); + }); +}); + +describe('resolveRoleIds', () => { + it('uses numeric tokens directly as IDs', async () => { + const result = await resolveRoleIds(mockClient, ['1', '2', '3']); + expect(mockClient.listRoles).not.toHaveBeenCalled(); + expect(result).toEqual([1, 2, 3]); + }); + + it('resolves names exactly and case-insensitively via listRoles search', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([ + { id: 1, name: 'Admin' }, + { id: 2, name: 'API User' }, + { id: 3, name: 'api user backup' }, + ] as any); + + const result = await resolveRoleIds(mockClient, ['API User']); + expect(mockClient.listRoles).toHaveBeenCalledWith({ search: 'API User', items: 200 }); + expect(result).toEqual([2]); + }); + + it('mixes numeric IDs and resolved names', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([{ id: 2, name: 'API User' }] as any); + + const result = await resolveRoleIds(mockClient, ['5', 'API User']); + expect(result).toEqual([5, 2]); + }); + + it('dedupes resolved IDs', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([{ id: 2, name: 'API User' }] as any); + const result = await resolveRoleIds(mockClient, ['API User', '2', 'api user']); + expect(result).toEqual([2]); + }); + + it('throws when a name matches no role', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([{ id: 1, name: 'Admin' }] as any); + await expect(resolveRoleIds(mockClient, ['Nonexistent'])).rejects.toThrow( + /No role found matching name "Nonexistent"/ + ); + }); + + it('throws with candidate list when a name matches multiple roles', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([ + { id: 2, name: 'API User' }, + { id: 4, name: 'API User' }, + ] as any); + await expect(resolveRoleIds(mockClient, ['API User'])).rejects.toThrow( + /Multiple roles match name "API User":\s*2 API User\s*4 API User/ + ); + }); + + it('returns [] for empty values', async () => { + await expect(resolveRoleIds(mockClient, [])).resolves.toEqual([]); + }); +}); + +describe('fetchAllUsers', () => { + it('pages through listUsers at 200/page until a short page', async () => { + const page1 = Array.from({ length: 200 }, (_, i) => ({ id: i + 1 })); + const page2 = Array.from({ length: 200 }, (_, i) => ({ id: i + 201 })); + const page3 = Array.from({ length: 25 }, (_, i) => ({ id: i + 401 })); + vi.mocked(mockClient.listUsers) + .mockResolvedValueOnce(page1 as any) + .mockResolvedValueOnce(page2 as any) + .mockResolvedValueOnce(page3 as any); + + const result = await fetchAllUsers(mockClient, { includeArchived: true }); + + expect(mockClient.listUsers).toHaveBeenCalledTimes(3); + expect(mockClient.listUsers).toHaveBeenNthCalledWith(1, { + includeArchived: true, + page: 1, + items: USER_FILTER_SCAN_PAGE_SIZE, + }); + expect(mockClient.listUsers).toHaveBeenNthCalledWith(2, { + includeArchived: true, + page: 2, + items: USER_FILTER_SCAN_PAGE_SIZE, + }); + expect(mockClient.listUsers).toHaveBeenNthCalledWith(3, { + includeArchived: true, + page: 3, + items: USER_FILTER_SCAN_PAGE_SIZE, + }); + expect(result).toHaveLength(425); + }); + + it('stops after the first page if it is short', async () => { + vi.mocked(mockClient.listUsers).mockResolvedValueOnce([{ id: 1 }] as any); + const result = await fetchAllUsers(mockClient, {}); + expect(mockClient.listUsers).toHaveBeenCalledTimes(1); + expect(result).toEqual([{ id: 1 }]); + }); + + it('forwards native base options (search, sort, sort_asc, ids) to every page', async () => { + const page1 = Array.from({ length: 200 }, (_, i) => ({ id: i + 1 })); + const page2 = [{ id: 201 }]; + vi.mocked(mockClient.listUsers) + .mockResolvedValueOnce(page1 as any) + .mockResolvedValueOnce(page2 as any); + + await fetchAllUsers(mockClient, { search: 'alice', sort: 'email', sort_asc: true, ids: '1,2' }); + + expect(mockClient.listUsers).toHaveBeenNthCalledWith(1, { + search: 'alice', + sort: 'email', + sort_asc: true, + ids: '1,2', + page: 1, + items: USER_FILTER_SCAN_PAGE_SIZE, + }); + expect(mockClient.listUsers).toHaveBeenNthCalledWith(2, { + search: 'alice', + sort: 'email', + sort_asc: true, + ids: '1,2', + page: 2, + items: USER_FILTER_SCAN_PAGE_SIZE, + }); + }); +}); + +describe('filterUsers', () => { + const users: Array> = [ + { + id: 1, + first_name: 'Alice', + last_name: 'Smith', + email: 'alice@acme.com', + department: 'Ancillaries', + job_title: 'CEO', + user_team_roles: [{ role_ids: [2, 3] }], + }, + { + id: 2, + first_name: 'Bob', + last_name: 'Jones', + email: 'bob@acme.com', + department: 'Finance', + job_title: 'Analyst', + user_team_roles: [{ role_ids: [5] }], + }, + { + id: 3, + first_name: 'Carol', + last_name: 'Lee', + email: 'carol@acme.com', + department: 'Ancillaries', + job_title: 'Engineer', + user_team_roles: [{ role_ids: [2, 9] }], + }, + ]; + + it('matches department exactly (case-insensitive), OR within values', () => { + const filters: UserClientFilters = { department: ['Ancillaries', 'Finance'] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([1, 2, 3]); + }); + + it('AND across fields: department AND role', () => { + const filters: UserClientFilters = { department: ['Ancillaries'], roleIds: [3] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([1]); + }); + + it('matches email exactly', () => { + const filters: UserClientFilters = { email: ['bob@acme.com'] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([2]); + }); + + it('matches name on the full first+last name', () => { + const filters: UserClientFilters = { name: ['Alice Smith'] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([1]); + }); + + it('matches job title exactly', () => { + const filters: UserClientFilters = { jobTitle: ['CEO'] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([1]); + }); + + it('role matches if any of the user role_ids is in the requested set', () => { + const filters: UserClientFilters = { roleIds: [5, 9] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([2, 3]); + }); + + it('with contains=true, matches substrings for a single field', () => { + const filters: UserClientFilters = { department: ['ancill'] }; + // 'Finance' does not contain 'ancill'; 'Ancillaries' does. + expect(filterUsers(users, filters, { contains: true }).map((u) => u.id)).toEqual([1, 3]); + }); + + it('with contains=true, still ANDs across fields', () => { + const filters: UserClientFilters = { name: ['smi'], department: ['ancill'] }; + // Only Alice has both a name containing 'smi' and a department containing 'ancill'. + expect(filterUsers(users, filters, { contains: true }).map((u) => u.id)).toEqual([1]); + }); + + it('contains does not affect role matching (role stays ID-exact)', () => { + const filters: UserClientFilters = { roleIds: [2], name: ['smi'] }; + expect(filterUsers(users, filters, { contains: true }).map((u) => u.id)).toEqual([1]); + }); + + it('returns all users when a field has an empty value array', () => { + const filters: UserClientFilters = { department: [] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([1, 2, 3]); + }); + + it('returns the same user references (no mutation) when no role tagging', () => { + const filters: UserClientFilters = { department: ['Ancillaries'] }; + const result = filterUsers(users, filters); + expect(result[0]).toBe(users[0]); + }); + + it('is case-insensitive on exact department match', () => { + const filters: UserClientFilters = { department: ['ancillaries'] }; + expect(filterUsers(users, filters).map((u) => u.id)).toEqual([1, 3]); + }); +}); + +describe('buildRoleNameMap', () => { + it('scans all roles pages at 200/page and maps id -> name', async () => { + const page1 = Array.from({ length: 200 }, (_, i) => ({ id: i + 1, name: `Role ${i + 1}` })); + const page2 = [{ id: 201, name: 'Role 201' }]; + vi.mocked(mockClient.listRoles) + .mockResolvedValueOnce(page1 as any) + .mockResolvedValueOnce(page2 as any); + + const map = await buildRoleNameMap(mockClient); + expect(mockClient.listRoles).toHaveBeenCalledTimes(2); + expect(mockClient.listRoles).toHaveBeenNthCalledWith(1, { page: 1, items: 200 }); + expect(mockClient.listRoles).toHaveBeenNthCalledWith(2, { page: 2, items: 200 }); + expect(map.get(1)).toBe('Role 1'); + expect(map.get(201)).toBe('Role 201'); + expect(map.size).toBe(201); + }); +}); + +describe('formatUserRoles', () => { + const roleNames = new Map([ + [2, 'API User'], + [3, 'Admin'], + ]); + + it('maps user role IDs to names and joins with ", "', () => { + expect(formatUserRoles({ user_team_roles: [{ role_ids: [2, 3] }] }, roleNames)).toBe( + 'API User, Admin' + ); + }); + + it('falls back to # for unknown role IDs', () => { + expect(formatUserRoles({ user_team_roles: [{ role_ids: [2, 99] }] }, roleNames)).toBe( + 'API User, #99' + ); + }); + + it('returns an empty string when the user has no roles', () => { + expect(formatUserRoles({}, roleNames)).toBe(''); + expect(formatUserRoles({ user_team_roles: [] }, roleNames)).toBe(''); + }); + + it('sorts role names deterministically by role ID', () => { + expect(formatUserRoles({ user_team_roles: [{ role_ids: [3, 2] }] }, roleNames)).toBe( + 'API User, Admin' + ); + }); +}); diff --git a/src/core/users/filter.ts b/src/core/users/filter.ts new file mode 100644 index 00000000..e67e0047 --- /dev/null +++ b/src/core/users/filter.ts @@ -0,0 +1,231 @@ +import type { APIClient } from '../../api-client/api-client.js'; + +/** Page size used when scanning every user to apply client-side filters. */ +export const USER_FILTER_SCAN_PAGE_SIZE = 200; + +export interface UserClientFilters { + department?: string[]; + roleIds?: number[]; + jobTitle?: string[]; + email?: string[]; + name?: string[]; +} + +export interface ListUsersScanOptions { + includeArchived?: boolean; + search?: string; + sort?: string; + sort_asc?: boolean; + ids?: string; +} + +/** + * Parse a comma-separated filter flag into a trimmed, blank-dropped, + * case-insensitively deduped list, keeping the first-seen spelling. + */ +export function parseFilterValues(input?: string): string[] { + if (!input) return []; + const seen = new Set(); + const out: string[] = []; + for (const raw of input.split(',')) { + const v = raw.trim(); + if (!v) continue; + const key = v.toLowerCase(); + if (seen.has(key)) continue; + seen.add(key); + out.push(v); + } + return out; +} + +/** + * True iff any client-side filter flag is set to a non-empty string. + * Native flags like `search`/`sort` are not client filters. + */ +export function hasClientFilters(options: { + department?: string; + role?: string; + jobTitle?: string; + email?: string; + name?: string; +}): boolean { + return Boolean( + options.department?.trim() || + options.role?.trim() || + options.jobTitle?.trim() || + options.email?.trim() || + options.name?.trim() + ); +} + +/** Join a user's first and last name with a space, omitting missing parts. */ +export function userFullName(user: Record): string { + return [user.first_name, user.last_name] + .filter((x) => typeof x === 'string' && x.length > 0) + .join(' '); +} + +/** + * Collect role IDs from a user's `user_team_roles` entries. Tolerant of + * missing or malformed data; returns [] when there is nothing to extract. + */ +export function userGlobalRoleIds(user: Record): number[] { + const entries = user.user_team_roles; + if (!Array.isArray(entries)) return []; + const ids: number[] = []; + for (const entry of entries) { + if (!entry || typeof entry !== 'object') continue; + const roleIds = (entry as Record).role_ids; + if (!Array.isArray(roleIds)) continue; + for (const id of roleIds) { + const n = Number(id); + if (!Number.isFinite(n)) continue; + ids.push(n); + } + } + return ids; +} + +function matchesField(candidate: string, values: string[] | undefined, contains: boolean): boolean { + if (!values || values.length === 0) return true; + const c = candidate.toLowerCase(); + for (const v of values) { + const want = v.toLowerCase(); + if (contains ? c.includes(want) : c === want) return true; + } + return false; +} + +/** + * Resolve role flag values to role IDs. Numeric tokens are used directly; names + * are matched exactly and case-insensitively against `listRoles({ search })`. + * Errors on no or multiple exact matches. Result is deduped, preserving + * first-seen order. + */ +export async function resolveRoleIds(client: APIClient, values: string[]): Promise { + const ids: number[] = []; + const seen = new Set(); + for (const value of values) { + const id = Number(value); + if (Number.isFinite(id) && String(id) === value.trim()) { + if (!seen.has(id)) { + seen.add(id); + ids.push(id); + } + continue; + } + const roles = (await client.listRoles({ + search: value, + items: USER_FILTER_SCAN_PAGE_SIZE, + })) as Array>; + const exact = roles.filter((r) => String(r.name ?? '').toLowerCase() === value.toLowerCase()); + if (exact.length === 0) { + throw new Error( + `No role found matching name "${value}". Use a numeric role ID, or check the name.` + ); + } + if (exact.length > 1) { + const candidates = exact.map((r) => ` ${r.id} ${r.name}`).join('\n'); + throw new Error( + `Multiple roles match name "${value}":\n${candidates}\nUse a numeric role ID to disambiguate.` + ); + } + const resolvedId = Number(exact[0]!.id); + if (!seen.has(resolvedId)) { + seen.add(resolvedId); + ids.push(resolvedId); + } + } + return ids; +} + +/** + * Fetch every user by paging through `listUsers` at a fixed page size until a + * short page is returned. Native `baseOptions` are honored on every page. + */ +export async function fetchAllUsers( + client: APIClient, + baseOptions: ListUsersScanOptions +): Promise>> { + const all: Array> = []; + let page = 1; + for (;;) { + const batch = (await client.listUsers({ + ...baseOptions, + page, + items: USER_FILTER_SCAN_PAGE_SIZE, + })) as Array>; + all.push(...batch); + if (batch.length < USER_FILTER_SCAN_PAGE_SIZE) break; + page++; + } + return all; +} + +/** + * Filter users by client predicates: OR within a field's values, AND across + * fields. `department`/`jobTitle`/`email`/`name` match exactly by default and + * as substrings when `contains` is true; `roleIds` matches by exact ID + * (unaffected by `contains`). Returns the matching user objects unchanged. + */ +export function filterUsers( + users: Array>, + filters: UserClientFilters, + options: { contains?: boolean } = {} +): Array> { + const contains = Boolean(options.contains); + return users.filter((u) => { + if ( + filters.department && + !matchesField(String(u.department ?? ''), filters.department, contains) + ) + return false; + if (filters.jobTitle && !matchesField(String(u.job_title ?? ''), filters.jobTitle, contains)) + return false; + if (filters.email && !matchesField(String(u.email ?? ''), filters.email, contains)) + return false; + if (filters.name && !matchesField(userFullName(u), filters.name, contains)) return false; + if (filters.roleIds && filters.roleIds.length > 0) { + const userRoleIds = userGlobalRoleIds(u); + if (!filters.roleIds.some((id) => userRoleIds.includes(id))) return false; + } + return true; + }); +} + +/** + * Scan all roles at 200/page and return a Map of role ID -> role name, for + * rendering the roles column of filtered users. + */ +export async function buildRoleNameMap(client: APIClient): Promise> { + const map = new Map(); + let page = 1; + for (;;) { + const roles = (await client.listRoles({ + page, + items: USER_FILTER_SCAN_PAGE_SIZE, + })) as Array>; + for (const r of roles) { + const id = Number(r.id); + if (Number.isFinite(id)) map.set(id, String(r.name ?? `#${id}`)); + } + if (roles.length < USER_FILTER_SCAN_PAGE_SIZE) break; + page++; + } + return map; +} + +/** + * Render a user's global-team role names, sorted by role ID ascending and + * joined with ", ". Unknown IDs fall back to "#"; a user with no roles + * renders as an empty string. + */ +export function formatUserRoles( + user: Record, + roleNames: Map +): string { + const ids = userGlobalRoleIds(user) + .slice() + .sort((a, b) => a - b); + return ids.map((id) => roleNames.get(id) ?? `#${id}`).join(', '); +} From 451babe4734bcd8883506519ad22714e0504a4eb Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 17:42:04 +0100 Subject: [PATCH 2/8] feat(users): pass native list filters --- src/api-client/api-client.test.ts | 50 +++++++++++++++++++++++++++++++ src/api-client/api-client.ts | 13 +++++++- src/core/users/list.ts | 18 ++++++++++- src/core/users/users.test.ts | 26 ++++++++++++++++ 4 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/api-client/api-client.test.ts b/src/api-client/api-client.test.ts index 56565523..739ae386 100644 --- a/src/api-client/api-client.test.ts +++ b/src/api-client/api-client.test.ts @@ -429,6 +429,56 @@ describe.skipIf(isLiveMode)('APIClient core', () => { expect(await client.listUsers()).toHaveLength(1); }); + it('should serialize native list filters as query params', async () => { + let capturedUrl: string | undefined; + server.use( + http.get(`${BASE_URL}/users`, ({ request }) => { + capturedUrl = request.url; + return HttpResponse.json({ users: [{ id: 1, email: 'a@b.c' }] }); + }) + ); + await client.listUsers({ + search: 'alice', + sort: 'email', + sort_asc: true, + ids: '1,2,3', + }); + expect(capturedUrl).toBeDefined(); + const url = new URL(capturedUrl!); + expect(url.searchParams.get('search')).toBe('alice'); + expect(url.searchParams.get('sort')).toBe('email'); + expect(url.searchParams.get('sort_asc')).toBe('true'); + expect(url.searchParams.get('ids')).toBe('1,2,3'); + }); + + it('should send include_archived when includeArchived is true', async () => { + let capturedUrl: string | undefined; + server.use( + http.get(`${BASE_URL}/users`, ({ request }) => { + capturedUrl = request.url; + return HttpResponse.json({ users: [] }); + }) + ); + await client.listUsers({ includeArchived: true }); + expect(new URL(capturedUrl!).searchParams.get('include_archived')).toBe('1'); + }); + + it('should not send native filter params when omitted', async () => { + let capturedUrl: string | undefined; + server.use( + http.get(`${BASE_URL}/users`, ({ request }) => { + capturedUrl = request.url; + return HttpResponse.json({ users: [] }); + }) + ); + await client.listUsers(); + const url = new URL(capturedUrl!); + expect(url.searchParams.get('sort')).toBeNull(); + expect(url.searchParams.get('sort_asc')).toBeNull(); + expect(url.searchParams.get('ids')).toBeNull(); + expect(url.searchParams.get('search')).toBeNull(); + }); + it('should reset user password', async () => { server.use( http.put(`${BASE_URL}/users/1/reset-password`, () => diff --git a/src/api-client/api-client.ts b/src/api-client/api-client.ts index bd55e1ee..625badae 100644 --- a/src/api-client/api-client.ts +++ b/src/api-client/api-client.ts @@ -778,7 +778,15 @@ export class APIClient { } async listUsers( - options: { includeArchived?: boolean; search?: string; items?: number; page?: number } = {} + options: { + includeArchived?: boolean; + search?: string; + sort?: string; + sort_asc?: boolean; + ids?: string; + items?: number; + page?: number; + } = {} ): Promise { const params: Record = { items: String(options.items ?? 100), @@ -786,6 +794,9 @@ export class APIClient { }; if (options.includeArchived) params.include_archived = '1'; if (options.search) params.search = options.search; + if (options.sort) params.sort = options.sort; + if (options.sort_asc !== undefined) params.sort_asc = String(options.sort_asc); + if (options.ids) params.ids = options.ids; const response = await this.request('GET', '/users', { params }); return this.validateListResponse(response, 'users', 'listUsers'); } diff --git a/src/core/users/list.ts b/src/core/users/list.ts index ec2e43c9..3d1e0514 100644 --- a/src/core/users/list.ts +++ b/src/core/users/list.ts @@ -5,16 +5,32 @@ export interface ListUsersParams { items?: number | undefined; page?: number | undefined; includeArchived?: boolean | undefined; + search?: string | undefined; + sort?: string | undefined; + sortAsc?: boolean | undefined; + ids?: string | undefined; } export async function listUsers( client: APIClient, params: ListUsersParams ): Promise> { - const opts: { includeArchived?: boolean; items?: number; page?: number } = {}; + const opts: { + includeArchived?: boolean; + items?: number; + page?: number; + search?: string; + sort?: string; + sort_asc?: boolean; + ids?: string; + } = {}; if (params.includeArchived !== undefined) opts.includeArchived = params.includeArchived; if (params.items !== undefined) opts.items = params.items; if (params.page !== undefined) opts.page = params.page; + if (params.search !== undefined) opts.search = params.search; + if (params.sort !== undefined) opts.sort = params.sort; + if (params.sortAsc !== undefined) opts.sort_asc = params.sortAsc; + if (params.ids !== undefined) opts.ids = params.ids; const data = await client.listUsers(opts); return { data, diff --git a/src/core/users/users.test.ts b/src/core/users/users.test.ts index 8f3a81df..42c498d0 100644 --- a/src/core/users/users.test.ts +++ b/src/core/users/users.test.ts @@ -118,6 +118,32 @@ describe('listUsers', () => { expect(mockClient.listUsers).toHaveBeenCalledWith({ includeArchived: true }); }); + + it('should forward native search/sort/sortAsc/ids to the API client', async () => { + mockClient.listUsers.mockResolvedValue([]); + + await listUsers(mockClient, { + search: 'alice', + sort: 'email', + sortAsc: true, + ids: '1,2', + }); + + expect(mockClient.listUsers).toHaveBeenCalledWith({ + search: 'alice', + sort: 'email', + sort_asc: true, + ids: '1,2', + }); + }); + + it('should not forward undefined native filters', async () => { + mockClient.listUsers.mockResolvedValue([]); + + await listUsers(mockClient, { items: 10, page: 2 }); + + expect(mockClient.listUsers).toHaveBeenCalledWith({ items: 10, page: 2 }); + }); }); describe('resetUserPassword', () => { From b767ad970c2752df27cf7fc6a864ba961ae8c364 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 17:54:03 +0100 Subject: [PATCH 3/8] feat(users): wire list filters into users list command --- src/commands/users/index.ts | 127 ++++++++++++++++++++++++++---- src/commands/users/users.test.ts | 129 +++++++++++++++++++++++++++++++ 2 files changed, 242 insertions(+), 14 deletions(-) diff --git a/src/commands/users/index.ts b/src/commands/users/index.ts index e2791433..1fa03ad3 100644 --- a/src/commands/users/index.ts +++ b/src/commands/users/index.ts @@ -13,7 +13,11 @@ import { type GlobalOptions, } from '../../lib/utils/api-helper.js'; import { parseUserId } from '../../lib/utils/validators.js'; -import { addPaginationOptions, printPaginationFooter } from '../../lib/utils/pagination.js'; +import { + addPaginationOptions, + printPaginationFooter, + printFilteredFooter, +} from '../../lib/utils/pagination.js'; import { renderInlineImage, supportsInlineImages } from '../../lib/utils/terminal-image.js'; import type { UserId } from '../../lib/api/branded-types.js'; import { @@ -30,6 +34,16 @@ import { updateUser as coreUpdateUser, archiveUser as coreArchiveUser, } from '../../core/users/index.js'; +import { + parseFilterValues, + hasClientFilters, + resolveRoleIds, + fetchAllUsers, + filterUsers, + buildRoleNameMap, + formatUserRoles, + type UserClientFilters, +} from '../../core/users/filter.js'; import { resetPasswordCommand } from './reset-password.js'; import { userApiKeysCommand } from './api-keys.js'; @@ -71,6 +85,23 @@ const listCommand = addPaginationOptions( new Command('list') .description('List all users') .option('--include-archived', 'include archived users') + .option('--search ', 'server-side fuzzy search across name/email/department/job_title') + .option('--sort ', 'sort by field (e.g. email, created_at)') + .option('--asc', 'sort in ascending order') + .option('--desc', 'sort in descending order') + .option('--ids ', 'filter by user IDs (comma-separated)') + .option( + '--department ', + 'filter by department, exact match (comma-separated; use --contains for substring)' + ) + .option('--role ', 'filter by role name or ID (comma-separated; scans all users)') + .option('--job-title ', 'filter by job title, exact match (comma-separated)') + .option('--email ', 'filter by email, exact match (comma-separated)') + .option('--name ', 'filter by full name, exact match (comma-separated)') + .option( + '--contains', + 'match --department/--job-title/--email/--name as substrings (case-insensitive)' + ) .option( '--show-avatars [cols]', 'display avatars inline, optional width in columns (default: 3)', @@ -82,12 +113,58 @@ const listCommand = addPaginationOptions( const client = await getAPIClientFromOptions(globalOptions); const { show = [], exclude = [], showOnly } = globalOptions; - const result = await coreListUsers(client, { - includeArchived: options.includeArchived, - items: options.items, - page: options.page, + const clientFiltersActive = hasClientFilters({ + department: options.department, + role: options.role, + jobTitle: options.jobTitle, + email: options.email, + name: options.name, }); - const users = result.data; + + const sortAsc = options.asc ? true : options.desc ? false : undefined; + const rolesColumnActive = clientFiltersActive || Boolean(options.search || options.ids); + + // Resolve client-side filter values up front (role names -> IDs). + const filters: UserClientFilters = {}; + if (options.department) filters.department = parseFilterValues(options.department); + if (options.role) + filters.roleIds = await resolveRoleIds(client, parseFilterValues(options.role)); + if (options.jobTitle) filters.jobTitle = parseFilterValues(options.jobTitle); + if (options.email) filters.email = parseFilterValues(options.email); + if (options.name) filters.name = parseFilterValues(options.name); + + let users: unknown[]; + let total: number | undefined; + + if (clientFiltersActive) { + // Scan all pages honoring native filters, then apply client predicates and + // paginate the matched set client-side (consistent with --metric on + // experiments list, PR #47). The API has no server-side department/role + // filter, so this is the only way to satisfy exact + multi-value asks. + const scanOptions = { + ...(options.includeArchived && { includeArchived: true }), + ...(options.search && { search: options.search }), + ...(options.sort && { sort: options.sort }), + ...(sortAsc !== undefined && { sort_asc: sortAsc }), + ...(options.ids && { ids: options.ids }), + }; + const all = await fetchAllUsers(client, scanOptions); + const matched = filterUsers(all, filters, { contains: options.contains }); + total = matched.length; + const start = (options.page - 1) * options.items; + users = matched.slice(start, start + options.items); + } else { + const result = await coreListUsers(client, { + includeArchived: options.includeArchived, + items: options.items, + page: options.page, + ...(options.search && { search: options.search }), + ...(options.sort && { sort: options.sort }), + ...(sortAsc !== undefined && { sortAsc }), + ...(options.ids && { ids: options.ids }), + }); + users = result.data; + } const wantAvatars = options.showAvatars !== undefined && @@ -96,6 +173,24 @@ const listCommand = addPaginationOptions( globalOptions.output !== 'json' && globalOptions.output !== 'yaml'; + // Roles column: shown when a filter is active (client or native search/ids), + // resolved from a single up-front GET /roles lookup. + let roleNames: Map | undefined; + if ( + rolesColumnActive && + !globalOptions.raw && + globalOptions.output !== 'json' && + globalOptions.output !== 'yaml' + ) { + roleNames = await buildRoleNameMap(client); + } + + const summarizeWithRoles = (u: Record): Record => { + const row = summarizeUserRow(u); + if (roleNames) row.roles = formatUserRoles(u, roleNames); + return row; + }; + if (wantAvatars) { const avatarWidth = typeof options.showAvatars === 'number' ? options.showAvatars : 3; const endpoint = resolveEndpoint(globalOptions); @@ -129,7 +224,7 @@ const listCommand = addPaginationOptions( ); const rows = (users as Array>).map((u) => - applyShowExclude(summarizeUserRow(u), u, show, exclude, showOnly) + applyShowExclude(summarizeWithRoles(u), u, show, exclude, showOnly) ); const keys = rows.length > 0 ? Object.keys(rows[0]!) : []; @@ -178,17 +273,21 @@ const listCommand = addPaginationOptions( const data = globalOptions.raw ? users : (users as Array>).map((u) => - applyShowExclude(summarizeUserRow(u), u, show, exclude, showOnly) + applyShowExclude(summarizeWithRoles(u), u, show, exclude, showOnly) ); printFormatted(data, globalOptions); } - printPaginationFooter( - users.length, - options.items, - options.page, - globalOptions.output as string - ); + if (clientFiltersActive && total !== undefined) { + printFilteredFooter(total, globalOptions.output as string); + } else { + printPaginationFooter( + users.length, + options.items, + options.page, + globalOptions.output as string + ); + } }) ); diff --git a/src/commands/users/users.test.ts b/src/commands/users/users.test.ts index e36c3fd4..ccbf364e 100644 --- a/src/commands/users/users.test.ts +++ b/src/commands/users/users.test.ts @@ -24,6 +24,7 @@ describe('users command', () => { const mockClient = { listUsers: vi.fn().mockResolvedValue([{ id: 1, email: 'test@test.com' }]), + listRoles: vi.fn().mockResolvedValue([]), getUser: vi.fn().mockResolvedValue({ id: 1, email: 'test@test.com' }), createUser: vi.fn().mockResolvedValue({ id: 99 }), updateUser: vi.fn().mockResolvedValue({}), @@ -70,6 +71,134 @@ describe('users command', () => { }); }); + it('should forward native --search/--sort/--asc/--ids to the API without scanning', async () => { + await usersCommand.parseAsync([ + 'node', + 'test', + 'list', + '--search', + 'alice', + '--sort', + 'email', + '--asc', + '--ids', + '1,2,3', + ]); + + expect(mockClient.listUsers).toHaveBeenCalledTimes(1); + expect(mockClient.listUsers).toHaveBeenCalledWith({ + includeArchived: undefined, + items: 20, + page: 1, + search: 'alice', + sort: 'email', + sort_asc: true, + ids: '1,2,3', + }); + }); + + it('should forward --desc as sort_asc false', async () => { + await usersCommand.parseAsync(['node', 'test', 'list', '--sort', 'email', '--desc']); + + expect(mockClient.listUsers).toHaveBeenCalledWith( + expect.objectContaining({ sort: 'email', sort_asc: false }) + ); + }); + + it('should scan all pages and filter client-side for --department', async () => { + const page1 = Array.from({ length: 200 }, (_, i) => ({ id: i + 1, department: 'Ancillaries' })); + const page2 = [{ id: 201, department: 'Finance' }]; + vi.mocked(mockClient.listUsers) + .mockResolvedValueOnce(page1 as any) + .mockResolvedValueOnce(page2 as any); + vi.mocked(mockClient.listRoles).mockResolvedValue([] as any); + + await usersCommand.parseAsync(['node', 'test', 'list', '--department', 'Ancillaries']); + + // Two scan pages at 200 items, no native pagination footer path. + expect(mockClient.listUsers).toHaveBeenCalledTimes(2); + expect(mockClient.listUsers).toHaveBeenNthCalledWith(1, { page: 1, items: 200 }); + expect(mockClient.listUsers).toHaveBeenNthCalledWith(2, { page: 2, items: 200 }); + // Roles lookup ran (roles column shows for client filters). + expect(mockClient.listRoles).toHaveBeenCalledTimes(1); + // Only the 200 Ancillaries users are passed to printFormatted (page 1 of 200). + expect(printFormatted).toHaveBeenCalledTimes(1); + const printed = vi.mocked(printFormatted).mock.calls[0]![0] as Array>; + expect(printed).toHaveLength(20); + expect(printed.every((r) => r.department === 'Ancillaries')).toBe(true); + // Filtered footer prints the full matched count, not a page footer. + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('200 results (filtered)')); + }); + + it('should resolve role names and filter by role id', async () => { + const page1 = [ + { id: 1, first_name: 'Alice', user_team_roles: [{ role_ids: [2] }] }, + { id: 2, first_name: 'Bob', user_team_roles: [{ role_ids: [5] }] }, + ]; + vi.mocked(mockClient.listUsers).mockResolvedValueOnce(page1 as any); + // First listRoles call resolves the name; second builds the role name map. + vi.mocked(mockClient.listRoles) + .mockResolvedValueOnce([ + { id: 1, name: 'Admin' }, + { id: 2, name: 'API User' }, + ] as any) + .mockResolvedValueOnce([{ id: 2, name: 'API User' }] as any); + + await usersCommand.parseAsync(['node', 'test', 'list', '--role', 'API User']); + + expect(mockClient.listRoles).toHaveBeenCalledTimes(2); + expect(mockClient.listRoles).toHaveBeenNthCalledWith(1, { search: 'API User', items: 200 }); + const printed = vi.mocked(printFormatted).mock.calls[0]![0] as Array>; + expect(printed).toHaveLength(1); + expect(printed[0]!.id).toBe(1); + expect(printed[0]!.roles).toBe('API User'); + }); + + it('should error when a role name matches nothing', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValueOnce([{ id: 1, name: 'Admin' }] as any); + + await expect( + usersCommand.parseAsync(['node', 'test', 'list', '--role', 'Nonexistent']) + ).rejects.toThrow('process.exit: 1'); + // The underlying error is logged before the wrapped exit. + const logged = consoleErrorSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + expect(logged).toMatch(/No role found matching name "Nonexistent"/); + }); + + it('should respect --contains for substring department matching', async () => { + vi.mocked(mockClient.listUsers).mockResolvedValueOnce([ + { id: 1, department: 'Ancillaries' }, + { id: 2, department: 'Finance' }, + ] as any); + vi.mocked(mockClient.listRoles).mockResolvedValue([] as any); + + await usersCommand.parseAsync(['node', 'test', 'list', '--department', 'Anc', '--contains']); + + const printed = vi.mocked(printFormatted).mock.calls[0]![0] as Array>; + expect(printed.map((r) => r.id)).toEqual([1, 2]); + }); + + it('should keep raw users untouched (no roles column) with --raw and a filter', async () => { + const rawUser = { + id: 1, + email: 'a@b.c', + department: 'Ancillaries', + user_team_roles: [{ role_ids: [2] }], + }; + vi.mocked(mockClient.listUsers).mockResolvedValueOnce([rawUser] as any); + + vi.mocked(getGlobalOptions).mockReturnValue({ output: 'table', raw: true } as any); + + await usersCommand.parseAsync(['node', 'test', 'list', '--department', 'Ancillaries']); + + expect(printFormatted).toHaveBeenCalledTimes(1); + const printed = vi.mocked(printFormatted).mock.calls[0]![0]; + // Raw output is the original user object, not a summarized row with a roles key. + expect(printed).toEqual([rawUser]); + // No roles lookup needed in raw mode. + expect(mockClient.listRoles).not.toHaveBeenCalled(); + }); + it('should get user by id', async () => { await usersCommand.parseAsync(['node', 'test', 'get', '1']); From 95668910ec627d0be5c0f4ba8d6023f8e9f47a19 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 17:55:08 +0100 Subject: [PATCH 4/8] chore(release): bump cli to 1.13.0 Document department/role/job-title/email/name filters on abs users list. --- README.md | 15 +++++++++++++++ package.json | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 03d879b9..388b4ac8 100644 --- a/README.md +++ b/README.md @@ -1012,12 +1012,27 @@ Aliases: `users`, `user` ```bash abs users list abs users list --include-archived +abs users list --search alice # server-side fuzzy search across name/email/department/job_title +abs users list --sort email --asc # sort by a field +abs users list --ids 1,2,3 # filter by user IDs abs users get 123 abs users create --email user@example.com --name "Jane Doe" abs users update 123 --name "Jane Smith" abs users archive 123 abs users archive 123 --unarchive +# Filter users by department, role, job title, email, or name (multi-value, exact by default) +abs users list --department Ancillaries # everyone in Ancillaries +abs users list --department Ancillaries,Finance # Ancillaries OR Finance +abs users list --role "API User" # everyone with the API User role (name or numeric ID) +abs users list --role "API User",Admin --department Ancillaries # role AND department +abs users list --job-title CEO --department Ancillaries +abs users list --email alice@acme.com,bob@acme.com +abs users list --name "Alice Smith" +abs users list --department anc --contains # substring match (case-insensitive) for department/job-title/email/name +# When a filter is active, a `roles` column is shown; the API has no server-side +# department/role filter, so client filters scan all users (200/page). + # Reset password abs users reset-password 123 diff --git a/package.json b/package.json index bd973ef4..420468ec 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@absmartly/cli", - "version": "1.12.0", + "version": "1.13.0", "description": "ABSmartly CLI - A/B Testing and Feature Flags command-line tool for AI agents and humans", "type": "module", "main": "./dist/index.js", From ba897528961acdb769847143edeb94087d8ea60e Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 19:57:37 +0100 Subject: [PATCH 5/8] fix(users): validate numeric --role IDs; cover client-filter pagination Address PR review: - resolveRoleIds now validates numeric role IDs via a batched listRoles({ids}) lookup and errors on unknown IDs (symmetric with name resolution), instead of silently matching zero users on a typo'd ID. - Document why userGlobalRoleIds uses Record: user_team_roles is API-populated but absent from the generated OpenAPI schema. - Add command test for page 2 of a client-filtered matched set spanning two scan pages, plus numeric-ID validation tests. --- src/commands/users/users.test.ts | 34 ++++++++++++++++++ src/core/users/filter.test.ts | 30 +++++++++++----- src/core/users/filter.ts | 60 ++++++++++++++++++++++++-------- 3 files changed, 102 insertions(+), 22 deletions(-) diff --git a/src/commands/users/users.test.ts b/src/commands/users/users.test.ts index ccbf364e..1c90b2aa 100644 --- a/src/commands/users/users.test.ts +++ b/src/commands/users/users.test.ts @@ -130,6 +130,40 @@ describe('users command', () => { expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('200 results (filtered)')); }); + it('should paginate the client-filtered matched set (--page 2)', async () => { + // 250 matching users spread across two scan pages (200 + 50); --items 20 --page 2 + // should return matched[20..40). + const page1 = Array.from({ length: 200 }, (_, i) => ({ id: i + 1, department: 'Ancillaries' })); + const page2 = Array.from({ length: 50 }, (_, i) => ({ + id: i + 201, + department: 'Ancillaries', + })); + vi.mocked(mockClient.listUsers) + .mockResolvedValueOnce(page1 as any) + .mockResolvedValueOnce(page2 as any); + vi.mocked(mockClient.listRoles).mockResolvedValue([] as any); + + await usersCommand.parseAsync([ + 'node', + 'test', + 'list', + '--department', + 'Ancillaries', + '--items', + '20', + '--page', + '2', + ]); + + const printed = vi.mocked(printFormatted).mock.calls[0]![0] as Array>; + // Second page of 20: ids 21..40. + expect(printed).toHaveLength(20); + expect(printed[0]!.id).toBe(21); + expect(printed[19]!.id).toBe(40); + // Footer still reports the full matched count. + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('250 results (filtered)')); + }); + it('should resolve role names and filter by role id', async () => { const page1 = [ { id: 1, first_name: 'Alice', user_team_roles: [{ role_ids: [2] }] }, diff --git a/src/core/users/filter.test.ts b/src/core/users/filter.test.ts index 0ecb3588..da1f7260 100644 --- a/src/core/users/filter.test.ts +++ b/src/core/users/filter.test.ts @@ -100,12 +100,24 @@ describe('userGlobalRoleIds', () => { }); describe('resolveRoleIds', () => { - it('uses numeric tokens directly as IDs', async () => { + it('validates numeric tokens via a batched listRoles({ ids }) lookup', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([ + { id: 1, name: 'A' }, + { id: 2, name: 'B' }, + { id: 3, name: 'C' }, + ] as any); const result = await resolveRoleIds(mockClient, ['1', '2', '3']); - expect(mockClient.listRoles).not.toHaveBeenCalled(); + expect(mockClient.listRoles).toHaveBeenCalledWith({ ids: '1,2,3', items: 200 }); expect(result).toEqual([1, 2, 3]); }); + it('throws when a numeric role ID does not exist', async () => { + vi.mocked(mockClient.listRoles).mockResolvedValue([{ id: 1, name: 'A' }] as any); + await expect(resolveRoleIds(mockClient, ['1', '9999'])).rejects.toThrow( + /No role found with ID 9999/ + ); + }); + it('resolves names exactly and case-insensitively via listRoles search', async () => { vi.mocked(mockClient.listRoles).mockResolvedValue([ { id: 1, name: 'Admin' }, @@ -118,14 +130,14 @@ describe('resolveRoleIds', () => { expect(result).toEqual([2]); }); - it('mixes numeric IDs and resolved names', async () => { + it('mixes numeric IDs and resolved names (numerics first)', async () => { vi.mocked(mockClient.listRoles).mockResolvedValue([{ id: 2, name: 'API User' }] as any); - const result = await resolveRoleIds(mockClient, ['5', 'API User']); - expect(result).toEqual([5, 2]); + const result = await resolveRoleIds(mockClient, ['2', 'API User']); + expect(result).toEqual([2]); }); - it('dedupes resolved IDs', async () => { + it('dedupes resolved IDs across numeric and name tokens', async () => { vi.mocked(mockClient.listRoles).mockResolvedValue([{ id: 2, name: 'API User' }] as any); const result = await resolveRoleIds(mockClient, ['API User', '2', 'api user']); expect(result).toEqual([2]); @@ -148,8 +160,10 @@ describe('resolveRoleIds', () => { ); }); - it('returns [] for empty values', async () => { - await expect(resolveRoleIds(mockClient, [])).resolves.toEqual([]); + it('returns [] for empty values without calling the API', async () => { + const result = await resolveRoleIds(mockClient, []); + expect(mockClient.listRoles).not.toHaveBeenCalled(); + expect(result).toEqual([]); }); }); diff --git a/src/core/users/filter.ts b/src/core/users/filter.ts index e67e0047..49796459 100644 --- a/src/core/users/filter.ts +++ b/src/core/users/filter.ts @@ -68,6 +68,12 @@ export function userFullName(user: Record): string { /** * Collect role IDs from a user's `user_team_roles` entries. Tolerant of * missing or malformed data; returns [] when there is nothing to extract. + * + * Typed as `Record` (not the exported `User` type) on purpose: + * `user_team_roles`/`role_ids` are populated by the live `GET /users` response + * (the backend User model's default include reshapes team roles into + * `{ team_id, role_ids: number[] }[]`) but are absent from the generated + * OpenAPI schema, so a typed field access is not available here. */ export function userGlobalRoleIds(user: Record): number[] { const entries = user.user_team_roles; @@ -97,23 +103,52 @@ function matchesField(candidate: string, values: string[] | undefined, contains: } /** - * Resolve role flag values to role IDs. Numeric tokens are used directly; names - * are matched exactly and case-insensitively against `listRoles({ search })`. - * Errors on no or multiple exact matches. Result is deduped, preserving - * first-seen order. + * Resolve role flag values to role IDs. Numeric tokens are validated to exist + * (via a single batched `listRoles({ ids })` lookup); names are matched exactly + * and case-insensitively against `listRoles({ search })`. Errors on unknown IDs + * and on no/multiple name matches. Result is deduped, preserving first-seen + * order. */ export async function resolveRoleIds(client: APIClient, values: string[]): Promise { const ids: number[] = []; const seen = new Set(); + + const add = (id: number): void => { + if (!seen.has(id)) { + seen.add(id); + ids.push(id); + } + }; + + const numericIds: number[] = []; + const names: string[] = []; for (const value of values) { const id = Number(value); if (Number.isFinite(id) && String(id) === value.trim()) { - if (!seen.has(id)) { - seen.add(id); - ids.push(id); - } - continue; + numericIds.push(id); + } else { + names.push(value); + } + } + + // Validate numeric IDs in one batch so a typo'd ID errors instead of + // silently matching zero users, symmetric with name resolution below. + if (numericIds.length > 0) { + const found = (await client.listRoles({ + ids: numericIds.join(','), + items: USER_FILTER_SCAN_PAGE_SIZE, + })) as Array>; + const foundIds = new Set(found.map((r) => Number(r.id))); + const missing = numericIds.filter((id) => !foundIds.has(id)); + if (missing.length > 0) { + throw new Error( + `No role found with ID ${missing.join(', ')}. Check the role ID(s) with \`abs roles list\`.` + ); } + for (const id of numericIds) add(id); + } + + for (const value of names) { const roles = (await client.listRoles({ search: value, items: USER_FILTER_SCAN_PAGE_SIZE, @@ -130,12 +165,9 @@ export async function resolveRoleIds(client: APIClient, values: string[]): Promi `Multiple roles match name "${value}":\n${candidates}\nUse a numeric role ID to disambiguate.` ); } - const resolvedId = Number(exact[0]!.id); - if (!seen.has(resolvedId)) { - seen.add(resolvedId); - ids.push(resolvedId); - } + add(Number(exact[0]!.id)); } + return ids; } From 50136a19b4444e06c8c808d017f662df420e2c7f Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 20:06:36 +0100 Subject: [PATCH 6/8] fix(users): scope roles column, paginate filtered footer, reject empty filters Address second round of PR review: - Roles column (and its GET /roles scan) now only fires for client field filters, not plain --search/--ids. - printFilteredFooter shows 'page X/Y' + 'Next: --page N+1' when the matched set spans multiple pages, matching the --metric footer parity claim. - A filter flag that parses to zero usable values (e.g. --department ' , ') now errors instead of silently returning the full list. - Clarify in comments that /users only returns global-team roles (backend include filters is_global_team) and that role resolution scans all roles. Verified the two review 'critical' shape/scope claims against the captured live /users payload (absmartly-api-mocks live-payloads/users-list.json): role_ids is the array form and the entry is the global team, so the existing code is correct. --- .superpowers/sdd/progress.md | 28 +++++++++++++++++ README.md | 5 +-- src/commands/users/index.ts | 32 +++++++++++++------ src/commands/users/users.test.ts | 27 +++++++++++++--- src/core/users/filter.ts | 10 ++++-- src/lib/utils/pagination.test.ts | 53 +++++++++++++++++++++++++++++++- src/lib/utils/pagination.ts | 15 +++++++-- 7 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 .superpowers/sdd/progress.md diff --git a/.superpowers/sdd/progress.md b/.superpowers/sdd/progress.md new file mode 100644 index 00000000..5f0ef22f --- /dev/null +++ b/.superpowers/sdd/progress.md @@ -0,0 +1,28 @@ +# Users List Filters — Progress Ledger + +Branch/worktree: worktree-feat-users-list-filters at .claude/worktrees/feat-users-list-filters +Plan: docs/superpowers/plans/2026-07-03-users-list-filters.md +Spec: docs/superpowers/specs/2026-07-03-users-list-filters-design.md + +## Tasks +- Task 1 (core filter module): IN PROGRESS — dispatched agent coreFilterAgent2 (sonnet, isolated worktree) +- Task 2 (API passthrough): IN PROGRESS — dispatched agent apiPassthroughAgent2 (sonnet, isolated worktree) +- Task 3 (command wiring): PENDING — depends on 1 + 2; will integrate then dispatch or do inline +- Task 4 (docs, bump, verify): PENDING + +## Integration plan +After both agents return: cherry-pick/merge their commits into this session worktree (they touch disjoint files), run full `npm test` + `npm run typecheck`, then start Task 3. + +## Notes +- First dispatch (coreFilterAgent + apiPassthroughAgent) hit 429 rate limit; re-dispatched as coreFilterAgent2 + apiPassthroughAgent2 after user said limit cleared. +- Agents were told to commit in their own isolated worktrees; merge their commit SHAs here once reported. + +## FINAL — completed inline (parallel agents kept hitting 429 rate limits) +- Task 1: complete — commit 8d18130 feat(users): add core list filters (38 filter tests) +- Task 2: complete — commit 451babe feat(users): pass native list filters (154 api/core tests) +- Task 3: complete — commit b767ad9 feat(users): wire list filters into users list command (14 command tests) +- Task 4: complete — commit 9566891 chore(release): bump cli to 1.13.0 (README + version) + +Verification: typecheck OK, lint OK, full suite 216 files / 2665 tests pass. +Branch: worktree-feat-users-list-filters at .claude/worktrees/feat-users-list-filters +Base: 3e6dce3 (origin/main) diff --git a/README.md b/README.md index 388b4ac8..bd43abb1 100644 --- a/README.md +++ b/README.md @@ -1030,8 +1030,9 @@ abs users list --job-title CEO --department Ancillaries abs users list --email alice@acme.com,bob@acme.com abs users list --name "Alice Smith" abs users list --department anc --contains # substring match (case-insensitive) for department/job-title/email/name -# When a filter is active, a `roles` column is shown; the API has no server-side -# department/role filter, so client filters scan all users (200/page). +# When a field filter is active, a `roles` column (global-team roles) is shown; +# the API has no server-side department/role filter, so client filters scan all +# users (200/page) and paginate the matched set. # Reset password abs users reset-password 123 diff --git a/src/commands/users/index.ts b/src/commands/users/index.ts index 1fa03ad3..3fc142c8 100644 --- a/src/commands/users/index.ts +++ b/src/commands/users/index.ts @@ -122,16 +122,29 @@ const listCommand = addPaginationOptions( }); const sortAsc = options.asc ? true : options.desc ? false : undefined; - const rolesColumnActive = clientFiltersActive || Boolean(options.search || options.ids); + // The roles column is about the field filters (it explains why rows matched a + // role/department query); native --search/--ids don't warrant a full roles scan. + const rolesColumnActive = clientFiltersActive; + + // Parse a filter flag, erroring if it was given but reduced to nothing (e.g. + // `--department " , "`) rather than silently returning the full unfiltered list. + const parseFilterFlag = (flag: string, raw: string): string[] => { + const values = parseFilterValues(raw); + if (values.length === 0) { + throw new Error(`${flag} was given no usable values (got "${raw}").`); + } + return values; + }; // Resolve client-side filter values up front (role names -> IDs). const filters: UserClientFilters = {}; - if (options.department) filters.department = parseFilterValues(options.department); + if (options.department) + filters.department = parseFilterFlag('--department', options.department); if (options.role) - filters.roleIds = await resolveRoleIds(client, parseFilterValues(options.role)); - if (options.jobTitle) filters.jobTitle = parseFilterValues(options.jobTitle); - if (options.email) filters.email = parseFilterValues(options.email); - if (options.name) filters.name = parseFilterValues(options.name); + filters.roleIds = await resolveRoleIds(client, parseFilterFlag('--role', options.role)); + if (options.jobTitle) filters.jobTitle = parseFilterFlag('--job-title', options.jobTitle); + if (options.email) filters.email = parseFilterFlag('--email', options.email); + if (options.name) filters.name = parseFilterFlag('--name', options.name); let users: unknown[]; let total: number | undefined; @@ -173,8 +186,9 @@ const listCommand = addPaginationOptions( globalOptions.output !== 'json' && globalOptions.output !== 'yaml'; - // Roles column: shown when a filter is active (client or native search/ids), - // resolved from a single up-front GET /roles lookup. + // Roles column: shown when a client field filter is active, resolved from an + // up-front scan of all roles (200/page). Only global-team roles appear, since + // the /users list response only includes user_team_roles for the global team. let roleNames: Map | undefined; if ( rolesColumnActive && @@ -279,7 +293,7 @@ const listCommand = addPaginationOptions( } if (clientFiltersActive && total !== undefined) { - printFilteredFooter(total, globalOptions.output as string); + printFilteredFooter(total, globalOptions.output as string, options.page, options.items); } else { printPaginationFooter( users.length, diff --git a/src/commands/users/users.test.ts b/src/commands/users/users.test.ts index 1c90b2aa..a43e2795 100644 --- a/src/commands/users/users.test.ts +++ b/src/commands/users/users.test.ts @@ -105,6 +105,21 @@ describe('users command', () => { ); }); + it('should not scan roles for native --search alone (no client field filter)', async () => { + await usersCommand.parseAsync(['node', 'test', 'list', '--search', 'alice']); + + // Native path only; no roles column, so no GET /roles scan. + expect(mockClient.listRoles).not.toHaveBeenCalled(); + }); + + it('should error when a filter flag parses to zero usable values', async () => { + await expect( + usersCommand.parseAsync(['node', 'test', 'list', '--department', ' , ']) + ).rejects.toThrow('process.exit: 1'); + const logged = consoleErrorSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + expect(logged).toMatch(/--department was given no usable values/); + }); + it('should scan all pages and filter client-side for --department', async () => { const page1 = Array.from({ length: 200 }, (_, i) => ({ id: i + 1, department: 'Ancillaries' })); const page2 = [{ id: 201, department: 'Finance' }]; @@ -126,8 +141,10 @@ describe('users command', () => { const printed = vi.mocked(printFormatted).mock.calls[0]![0] as Array>; expect(printed).toHaveLength(20); expect(printed.every((r) => r.department === 'Ancillaries')).toBe(true); - // Filtered footer prints the full matched count, not a page footer. - expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('200 results (filtered)')); + // Filtered footer prints the full matched count plus page info. + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('200 results (filtered) — page 1/10') + ); }); it('should paginate the client-filtered matched set (--page 2)', async () => { @@ -160,8 +177,10 @@ describe('users command', () => { expect(printed).toHaveLength(20); expect(printed[0]!.id).toBe(21); expect(printed[19]!.id).toBe(40); - // Footer still reports the full matched count. - expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('250 results (filtered)')); + // Footer still reports the full matched count with page info. + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('250 results (filtered) — page 2/13') + ); }); it('should resolve role names and filter by role id', async () => { diff --git a/src/core/users/filter.ts b/src/core/users/filter.ts index 49796459..fbc40553 100644 --- a/src/core/users/filter.ts +++ b/src/core/users/filter.ts @@ -69,11 +69,15 @@ export function userFullName(user: Record): string { * Collect role IDs from a user's `user_team_roles` entries. Tolerant of * missing or malformed data; returns [] when there is nothing to extract. * + * Flattens `role_ids` across all `user_team_roles` entries. For the `GET /users` + * list response this is effectively the user's global-team roles: the backend + * includes `user_team_roles` filtered to the global team only, so no per-entry + * `team_id` check is needed here. + * * Typed as `Record` (not the exported `User` type) on purpose: * `user_team_roles`/`role_ids` are populated by the live `GET /users` response - * (the backend User model's default include reshapes team roles into - * `{ team_id, role_ids: number[] }[]`) but are absent from the generated - * OpenAPI schema, so a typed field access is not available here. + * but are absent from the generated OpenAPI schema, so a typed field access is + * not available here. */ export function userGlobalRoleIds(user: Record): number[] { const entries = user.user_team_roles; diff --git a/src/lib/utils/pagination.test.ts b/src/lib/utils/pagination.test.ts index 2ee4b297..3dc1d705 100644 --- a/src/lib/utils/pagination.test.ts +++ b/src/lib/utils/pagination.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { Command } from 'commander'; -import { addPaginationOptions, printPaginationFooter, printMetricFooter } from './pagination.js'; +import { + addPaginationOptions, + printPaginationFooter, + printMetricFooter, + printFilteredFooter, +} from './pagination.js'; describe('addPaginationOptions', () => { it('should add --items and --page options to a Command', () => { @@ -50,6 +55,52 @@ describe('printPaginationFooter', () => { }); }); +describe('printFilteredFooter', () => { + let logSpy: ReturnType; + + beforeEach(() => { + logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + logSpy.mockRestore(); + }); + + it('shows a plain count when page/items are omitted', () => { + printFilteredFooter(5); + const output = logSpy.mock.calls[0]![0] as string; + expect(output).toContain('5 results (filtered).'); + expect(output).not.toContain('page'); + }); + + it('shows a plain count when total fits on one page', () => { + printFilteredFooter(5, 'table', 1, 20); + const output = logSpy.mock.calls[0]![0] as string; + expect(output).toContain('5 results (filtered).'); + expect(output).not.toContain('page'); + }); + + it('shows page X/Y and a Next hint when total exceeds one page', () => { + printFilteredFooter(250, 'table', 2, 20); + const output = logSpy.mock.calls[0]![0] as string; + expect(output).toContain('250 results (filtered) — page 2/13.'); + expect(output).toContain('Next: --page 3'); + }); + + it('omits the Next hint on the last page', () => { + printFilteredFooter(40, 'table', 2, 20); + const output = logSpy.mock.calls[0]![0] as string; + expect(output).toContain('page 2/2'); + expect(output).not.toContain('Next:'); + }); + + it('prints nothing for json/yaml output', () => { + printFilteredFooter(250, 'json', 1, 20); + printFilteredFooter(250, 'yaml', 1, 20); + expect(logSpy).not.toHaveBeenCalled(); + }); +}); + describe('printMetricFooter', () => { let logSpy: ReturnType; diff --git a/src/lib/utils/pagination.ts b/src/lib/utils/pagination.ts index d4f56c4e..2c7143c0 100644 --- a/src/lib/utils/pagination.ts +++ b/src/lib/utils/pagination.ts @@ -21,9 +21,20 @@ export function printPaginationFooter( console.log(chalk.gray(footer)); } -export function printFilteredFooter(count: number, outputFormat?: string): void { +export function printFilteredFooter( + count: number, + outputFormat?: string, + page?: number, + items?: number +): void { if (outputFormat === 'json' || outputFormat === 'yaml') return; - console.log(chalk.gray(`${count} result${count === 1 ? '' : 's'} (filtered).`)); + let footer = `${count} result${count === 1 ? '' : 's'} (filtered).`; + if (page !== undefined && items !== undefined && count > items) { + const totalPages = Math.max(1, Math.ceil(count / items)); + footer = `${count} results (filtered) — page ${page}/${totalPages}.`; + if (page < totalPages) footer += ` Next: --page ${page + 1}`; + } + console.log(chalk.gray(footer)); } export function printMetricFooter( From 68e32b6c858e63f1a9956da02feb818a6fd655e4 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 20:06:46 +0100 Subject: [PATCH 7/8] chore: git-ignore .superpowers session scratch --- .gitignore | 1 + .superpowers/sdd/progress.md | 28 ---------------------------- 2 files changed, 1 insertion(+), 28 deletions(-) delete mode 100644 .superpowers/sdd/progress.md diff --git a/.gitignore b/.gitignore index 735b315b..868964fd 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,4 @@ docs/plans/ docs/ PLAN-*.md .worktrees/ +.superpowers/ diff --git a/.superpowers/sdd/progress.md b/.superpowers/sdd/progress.md deleted file mode 100644 index 5f0ef22f..00000000 --- a/.superpowers/sdd/progress.md +++ /dev/null @@ -1,28 +0,0 @@ -# Users List Filters — Progress Ledger - -Branch/worktree: worktree-feat-users-list-filters at .claude/worktrees/feat-users-list-filters -Plan: docs/superpowers/plans/2026-07-03-users-list-filters.md -Spec: docs/superpowers/specs/2026-07-03-users-list-filters-design.md - -## Tasks -- Task 1 (core filter module): IN PROGRESS — dispatched agent coreFilterAgent2 (sonnet, isolated worktree) -- Task 2 (API passthrough): IN PROGRESS — dispatched agent apiPassthroughAgent2 (sonnet, isolated worktree) -- Task 3 (command wiring): PENDING — depends on 1 + 2; will integrate then dispatch or do inline -- Task 4 (docs, bump, verify): PENDING - -## Integration plan -After both agents return: cherry-pick/merge their commits into this session worktree (they touch disjoint files), run full `npm test` + `npm run typecheck`, then start Task 3. - -## Notes -- First dispatch (coreFilterAgent + apiPassthroughAgent) hit 429 rate limit; re-dispatched as coreFilterAgent2 + apiPassthroughAgent2 after user said limit cleared. -- Agents were told to commit in their own isolated worktrees; merge their commit SHAs here once reported. - -## FINAL — completed inline (parallel agents kept hitting 429 rate limits) -- Task 1: complete — commit 8d18130 feat(users): add core list filters (38 filter tests) -- Task 2: complete — commit 451babe feat(users): pass native list filters (154 api/core tests) -- Task 3: complete — commit b767ad9 feat(users): wire list filters into users list command (14 command tests) -- Task 4: complete — commit 9566891 chore(release): bump cli to 1.13.0 (README + version) - -Verification: typecheck OK, lint OK, full suite 216 files / 2665 tests pass. -Branch: worktree-feat-users-list-filters at .claude/worktrees/feat-users-list-filters -Base: 3e6dce3 (origin/main) From f40c6658be4818e302d1fd8b2e6fbc5383e88432 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 3 Jul 2026 20:10:03 +0100 Subject: [PATCH 8/8] chore(release): bump cli to 1.14.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 420468ec..565ce4e5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@absmartly/cli", - "version": "1.13.0", + "version": "1.14.0", "description": "ABSmartly CLI - A/B Testing and Feature Flags command-line tool for AI agents and humans", "type": "module", "main": "./dist/index.js",