From 32b43469984dd27a4a7862dc93f9aca3e6f83b07 Mon Sep 17 00:00:00 2001 From: joalves Date: Tue, 14 Jul 2026 22:06:46 +0000 Subject: [PATCH] fix(users): send --role on `users update` instead of dropping it `abs users update --role ` accepted a --role option but silently ignored it: updateUser() in src/core/users/update.ts built the request body from params.name only and never added params.role. As a result `abs users update 256 --role 34` failed with "At least one update field is required", and even with a name the role change was never sent. The API expects roles as an array of `{ role_id: number }` objects for the user's global team. This is confirmed by the OpenAPI UpdateUserBody schema (`data.roles: { role_id: number }[]`) and the backend handler which does `data.roles.map(({ role_id }) => ...)`. A bare `roles: [34]` array is rejected (500), which is why passing the field naively fails. Fix: - update.ts now adds `roles: [{ role_id }]` to the request body when --role is provided, parsing the option to a positive integer role ID and counting it toward the "at least one field" guard so `--role X` alone works. - Introduce an `UpdateUserData` type derived from the OpenAPI UpdateUserBody schema and use it for the core `data` object and the api-client `updateUser(id, data)` signature (previously `Partial`, which has no `roles` field), so role updates are type-checked. Tested: extended src/core/users/users.test.ts (role-only, name+role, invalid-role) and src/commands/users/users.test.ts (--role wiring); both suites pass. `tsc` compiles clean. Verified end-to-end against the compiled output that `--role 34` sends `{ roles: [{ role_id: 34 }] }`. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/api-client/api-client.ts | 3 ++- src/api-client/types.ts | 3 +++ src/commands/users/users.test.ts | 8 ++++++++ src/core/users/update.ts | 15 ++++++++++++++- src/core/users/users.test.ts | 30 ++++++++++++++++++++++++++++++ src/lib/api/openapi-types.ts | 1 + 6 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/api-client/api-client.ts b/src/api-client/api-client.ts index 2fc9485..c31204d 100644 --- a/src/api-client/api-client.ts +++ b/src/api-client/api-client.ts @@ -14,6 +14,7 @@ import type { Segment, Team, User, + UpdateUserData, Metric, Application, Environment, @@ -755,7 +756,7 @@ export class APIClient { return this.validateEntityResponse(response, 'user', 'createUser'); } - async updateUser(id: UserId, data: Partial): Promise { + async updateUser(id: UserId, data: UpdateUserData): Promise { const response = await this.request('PUT', `/users/${id}`, { data: { data } }); return this.validateEntityResponse(response, 'user', 'updateUser'); } diff --git a/src/api-client/types.ts b/src/api-client/types.ts index a1a7e2f..ff5d736 100644 --- a/src/api-client/types.ts +++ b/src/api-client/types.ts @@ -9,6 +9,7 @@ import type { Segment as OpenAPISegment, Team as OpenAPITeam, User as OpenAPIUser, + UpdateUserData, Metric as OpenAPIMetric, MetricTag as OpenAPIMetricTag, MetricCategory as OpenAPIMetricCategory, @@ -217,6 +218,8 @@ export type User = Partial & { email: string; }; +export type { UpdateUserData }; + export type Metric = Partial & { id: MetricId; name: string; diff --git a/src/commands/users/users.test.ts b/src/commands/users/users.test.ts index e36c3fd..a0a9938 100644 --- a/src/commands/users/users.test.ts +++ b/src/commands/users/users.test.ts @@ -107,6 +107,14 @@ describe('users command', () => { }); }); + it('should update a user role via --role', async () => { + await usersCommand.parseAsync(['node', 'test', 'update', '1', '--role', '34']); + + expect(mockClient.updateUser).toHaveBeenCalledWith(1, { + roles: [{ role_id: 34 }], + }); + }); + it('should archive a user', async () => { await usersCommand.parseAsync(['node', 'test', 'archive', '1']); diff --git a/src/core/users/update.ts b/src/core/users/update.ts index 8b98f56..737a11c 100644 --- a/src/core/users/update.ts +++ b/src/core/users/update.ts @@ -1,5 +1,6 @@ import type { APIClient } from '../../api-client/api-client.js'; import type { UserId } from '../../lib/api/branded-types.js'; +import type { UpdateUserData } from '../../api-client/types.js'; import type { CommandResult } from '../types.js'; export interface UpdateUserParams { @@ -12,13 +13,25 @@ export async function updateUser( client: APIClient, params: UpdateUserParams ): Promise> { - const data: Record = {}; + const data: UpdateUserData = {}; if (params.name) { const parts = params.name.split(' '); data.first_name = parts[0] ?? ''; data.last_name = parts.slice(1).join(' '); } + if (params.role !== undefined) { + // The API expects roles as an array of { role_id } objects for the + // global team (see UpdateUserBody in the OpenAPI schema and the backend + // handler which does data.roles.map(({ role_id }) => ...)). A bare + // number array is rejected. --role carries a single role ID. + const roleId = Number(params.role); + if (!Number.isInteger(roleId) || roleId <= 0) { + throw new Error(`Invalid role: "${params.role}" is not a valid role ID`); + } + data.roles = [{ role_id: roleId }]; + } + if (Object.keys(data).length === 0) { throw new Error('At least one update field is required'); } diff --git a/src/core/users/users.test.ts b/src/core/users/users.test.ts index 8f3a81d..25338ce 100644 --- a/src/core/users/users.test.ts +++ b/src/core/users/users.test.ts @@ -149,4 +149,34 @@ describe('updateUser', () => { 'At least one update field is required' ); }); + + it('should update role alone as [{ role_id }]', async () => { + mockClient.updateUser.mockResolvedValue(undefined); + + const result = await updateUser(mockClient, { id: 5 as any, role: '34' }); + + expect(mockClient.updateUser).toHaveBeenCalledWith(5, { + roles: [{ role_id: 34 }], + }); + expect(result).toEqual({ data: undefined }); + }); + + it('should update name and role together', async () => { + mockClient.updateUser.mockResolvedValue(undefined); + + await updateUser(mockClient, { id: 7 as any, name: 'Jane Smith', role: '2' }); + + expect(mockClient.updateUser).toHaveBeenCalledWith(7, { + first_name: 'Jane', + last_name: 'Smith', + roles: [{ role_id: 2 }], + }); + }); + + it('should throw on an invalid role value', async () => { + await expect(updateUser(mockClient, { id: 5 as any, role: 'admin' })).rejects.toThrow( + 'Invalid role' + ); + expect(mockClient.updateUser).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/api/openapi-types.ts b/src/lib/api/openapi-types.ts index 77f7065..cce987d 100644 --- a/src/lib/api/openapi-types.ts +++ b/src/lib/api/openapi-types.ts @@ -44,6 +44,7 @@ export type GoalTag = components['schemas']['GoalTag']; export type Segment = components['schemas']['Segment']; export type Team = components['schemas']['Team']; export type User = components['schemas']['User']; +export type UpdateUserData = components['schemas']['UpdateUserBody']['data']; export type Metric = components['schemas']['Metric']; export type MetricTag = components['schemas']['MetricTag']; export type MetricCategory = components['schemas']['MetricCategory'];