diff --git a/src/api-client/api-client.test.ts b/src/api-client/api-client.test.ts index 5d6d0fba..56565523 100644 --- a/src/api-client/api-client.test.ts +++ b/src/api-client/api-client.test.ts @@ -1188,7 +1188,30 @@ describe.skipIf(isLiveMode)('APIClient core', () => { expect(await client.getPlatformConfig(1)).toBeDefined(); server.use(http.put(`${BASE_URL}/configs/1`, () => HttpResponse.json({ config: { id: 1 } }))); - await client.updatePlatformConfig(1, { key: 'val' }); + await client.updatePlatformConfig(1, { name: 'cfg', value: 'val' }); + }); + + it('should PUT the full config object under data, not the bare value', async () => { + let sentBody: Record = {}; + server.use( + http.put(`${BASE_URL}/configs/22`, async ({ request }) => { + sentBody = (await request.json()) as Record; + return HttpResponse.json({ + ok: true, + config: { id: 22, name: 'experiment_form_max_secondary_metrics', value: '30' }, + }); + }) + ); + + await client.updatePlatformConfig(22, { + name: 'experiment_form_max_secondary_metrics', + value: '30', + }); + + // Wire body must be {data: {name, value}} — not {data: '30'} or {data: {data: ...}} + expect(sentBody).toEqual({ + data: { name: 'experiment_form_max_secondary_metrics', value: '30' }, + }); }); }); diff --git a/src/commands/platformconfig/index.ts b/src/commands/platformconfig/index.ts index a2e01ec3..ba670c1c 100644 --- a/src/commands/platformconfig/index.ts +++ b/src/commands/platformconfig/index.ts @@ -46,7 +46,7 @@ const updateCommand = new Command('update') withErrorHandling(async (id: number, options) => { const globalOptions = getGlobalOptions(updateCommand); const client = await getAPIClientFromOptions(globalOptions); - const value = validateJSON(options.value, '--value') as Record; + const value = validateJSON(options.value, '--value'); const result = await updatePlatformConfig(client, { id, value }); console.log(chalk.green(`✓ Platform config ${id} updated`)); printFormatted(result.data, globalOptions); diff --git a/src/commands/platformconfig/platformconfig.test.ts b/src/commands/platformconfig/platformconfig.test.ts index 5a722710..2a0b5b1e 100644 --- a/src/commands/platformconfig/platformconfig.test.ts +++ b/src/commands/platformconfig/platformconfig.test.ts @@ -23,9 +23,9 @@ describe('platform-config command', () => { let processExitSpy: ReturnType; const mockClient = { - listPlatformConfigs: vi.fn().mockResolvedValue([{ id: 1, value: 'test' }]), - getPlatformConfig: vi.fn().mockResolvedValue({ id: 1, value: 'test' }), - updatePlatformConfig: vi.fn().mockResolvedValue({ id: 1, value: 'new' }), + listPlatformConfigs: vi.fn().mockResolvedValue([{ id: 1, name: 'cfg', value: 'test' }]), + getPlatformConfig: vi.fn().mockResolvedValue({ id: 1, name: 'cfg', value: 'test' }), + updatePlatformConfig: vi.fn().mockResolvedValue({ id: 1, name: 'cfg', value: 'new' }), }; beforeEach(() => { @@ -61,7 +61,26 @@ describe('platform-config command', () => { expect(printFormatted).toHaveBeenCalled(); }); - it('should update platform config', async () => { + it('should fetch the current config and PUT the merged payload (string value)', async () => { + await platformConfigCommand.parseAsync(['node', 'test', 'update', '1', '--value', '"30"']); + + expect(mockClient.getPlatformConfig).toHaveBeenCalledWith(1); + expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, { + name: 'cfg', + value: '30', + }); + }); + + it('should accept a numeric value via --value', async () => { + await platformConfigCommand.parseAsync(['node', 'test', 'update', '1', '--value', '42']); + + expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, { + name: 'cfg', + value: 42, + }); + }); + + it('should accept an object as --value', async () => { await platformConfigCommand.parseAsync([ 'node', 'test', @@ -71,6 +90,9 @@ describe('platform-config command', () => { '{"key":"val"}', ]); - expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, { key: 'val' }); + expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, { + name: 'cfg', + value: { key: 'val' }, + }); }); }); diff --git a/src/core/platformconfig/platformconfig.test.ts b/src/core/platformconfig/platformconfig.test.ts index 307e1c6a..ef37fb56 100644 --- a/src/core/platformconfig/platformconfig.test.ts +++ b/src/core/platformconfig/platformconfig.test.ts @@ -22,11 +22,81 @@ describe('platformconfig', () => { expect(result.data).toEqual({ id: 1, key: 'val' }); }); - it('should update platform config', async () => { - const value = { setting: true }; - mockClient.updatePlatformConfig.mockResolvedValue({ id: 1, setting: true }); - const result = await updatePlatformConfig(mockClient as any, { id: 1, value }); - expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, value); - expect(result.data).toEqual({ id: 1, setting: true }); + it('should fetch the current config, merge the new value, and PUT without id', async () => { + mockClient.getPlatformConfig.mockResolvedValue({ + id: 22, + name: 'experiment_form_max_secondary_metrics', + value: '20', + }); + mockClient.updatePlatformConfig.mockResolvedValue({ + id: 22, + name: 'experiment_form_max_secondary_metrics', + value: '30', + }); + + const result = await updatePlatformConfig(mockClient as any, { id: 22, value: '30' }); + + expect(mockClient.getPlatformConfig).toHaveBeenCalledWith(22); + // name comes from GET; id stays out of the body (URL path). + expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(22, { + name: 'experiment_form_max_secondary_metrics', + value: '30', + }); + expect(result.data).toEqual({ + id: 22, + name: 'experiment_form_max_secondary_metrics', + value: '30', + }); + }); + + it('should accept any JSON value for value (string, number, object)', async () => { + mockClient.getPlatformConfig.mockResolvedValue({ id: 1, name: 'numeric_cfg', value: 0 }); + mockClient.updatePlatformConfig.mockResolvedValue({ id: 1, name: 'numeric_cfg', value: 42 }); + + await updatePlatformConfig(mockClient as any, { id: 1, value: 42 }); + + expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, { + name: 'numeric_cfg', + value: 42, + }); + }); + + it('should throw a malformed-payload error if GET returns a non-object', async () => { + mockClient.getPlatformConfig.mockResolvedValue(null); + mockClient.updatePlatformConfig.mockClear(); + + await expect(updatePlatformConfig(mockClient as any, { id: 999, value: 'x' })).rejects.toThrow( + /malformed payload.*got null/ + ); + expect(mockClient.updatePlatformConfig).not.toHaveBeenCalled(); + }); + + it('should wrap GET errors with context and not attempt a PUT', async () => { + const cause = new Error('network down'); + mockClient.getPlatformConfig.mockRejectedValue(cause); + mockClient.updatePlatformConfig.mockClear(); + + await expect(updatePlatformConfig(mockClient as any, { id: 7, value: 'x' })).rejects.toThrow( + /failed to fetch existing config/ + ); + expect(mockClient.updatePlatformConfig).not.toHaveBeenCalled(); + }); + + it.each([ + ['null', null], + ['false', false], + ['zero', 0], + ['empty string', ''], + ['empty array', []], + ])('should accept falsy JSON value: %s', async (_label, falsyValue) => { + mockClient.getPlatformConfig.mockResolvedValue({ id: 1, name: 'cfg', value: 'old' }); + mockClient.updatePlatformConfig.mockResolvedValue({ id: 1, name: 'cfg', value: falsyValue }); + + await updatePlatformConfig(mockClient as any, { id: 1, value: falsyValue }); + + expect(mockClient.updatePlatformConfig).toHaveBeenCalledWith(1, { + name: 'cfg', + value: falsyValue, + }); }); }); diff --git a/src/core/platformconfig/platformconfig.ts b/src/core/platformconfig/platformconfig.ts index 224ed7c6..7b8a0283 100644 --- a/src/core/platformconfig/platformconfig.ts +++ b/src/core/platformconfig/platformconfig.ts @@ -20,13 +20,32 @@ export async function getPlatformConfig( export interface UpdatePlatformConfigParams { id: number; - value: Record; + value: unknown; } export async function updatePlatformConfig( client: APIClient, params: UpdatePlatformConfigParams ): Promise> { - const data = await client.updatePlatformConfig(params.id, params.value); + let current: unknown; + try { + current = await client.getPlatformConfig(params.id); + } catch (err) { + throw new Error(`Cannot update platform config ${params.id}: failed to fetch existing config`, { + cause: err, + }); + } + if (!current || typeof current !== 'object') { + const got = current === null ? 'null' : typeof current; + throw new Error( + `Cannot update platform config ${params.id}: GET returned malformed payload (expected object, got ${got})` + ); + } + // Last-writer-wins on every field except `value`: we read the current config and PUT it back + // with the new value. Concurrent writes to other fields between this GET and PUT are silently + // overwritten with the stale read. id is in the URL path; don't echo it in the body. + const { id: _id, ...rest } = current as Record; + const merged: Record = { ...rest, value: params.value }; + const data = await client.updatePlatformConfig(params.id, merged); return { data }; }