From f21bd4434b5e59d496c43d5c02a8d8b644a6d2c4 Mon Sep 17 00:00:00 2001 From: Jonas Alves Date: Fri, 25 Sep 2026 18:37:55 +0100 Subject: [PATCH 1/2] feat(metrics): add --datasource-id to metrics create/version custom_sql metrics pinned to a non-default datasource (e.g. BigQuery) had no CLI way to set or carry forward datasource_id on a new version. --- src/commands/metrics/index.ts | 6 +++ src/commands/metrics/metrics.test.ts | 63 ++++++++++++++++++++++++++++ src/core/metrics/payload.ts | 2 + 3 files changed, 71 insertions(+) diff --git a/src/commands/metrics/index.ts b/src/commands/metrics/index.ts index 25b6bc9..7f92d0c 100644 --- a/src/commands/metrics/index.ts +++ b/src/commands/metrics/index.ts @@ -173,6 +173,11 @@ function addMetricFieldOptions(cmd: Command): Command { ) .option('--custom-sql ', 'custom SQL (required for custom_sql type)') .option('--custom-statistics-type ', 'custom statistics type (continuous, binomial)') + .option( + '--datasource-id ', + 'datasource ID the custom SQL runs against (custom_sql)', + parseInt + ) .option('--vr-lookback-interval ', 'VR lookback interval (1w, 2w, 3w, 4w)') .option('--relation-kind ', 'goal relation kind (refund, replacement)') .option('--relation-refund-operation ', 'refund operation (add, subtract)') @@ -264,6 +269,7 @@ async function resolveMetricFieldsFromOptions( activityInterval: options.activityInterval as string | undefined, customSql: options.customSql as string | undefined, customStatisticsType: options.customStatisticsType as string | undefined, + datasourceId: options.datasourceId as number | undefined, vrLookbackInterval: options.vrLookbackInterval as string | undefined, relationKind: options.relationKind as string | undefined, relationRefundOperation: options.relationRefundOperation as string | undefined, diff --git a/src/commands/metrics/metrics.test.ts b/src/commands/metrics/metrics.test.ts index f221fc0..37f942c 100644 --- a/src/commands/metrics/metrics.test.ts +++ b/src/commands/metrics/metrics.test.ts @@ -388,6 +388,34 @@ describe('metrics command', () => { ); }); + it('should create a custom_sql metric pinned to a datasource via --datasource-id', async () => { + await metricsCommand.parseAsync([ + 'node', + 'test', + 'create', + '--name', + 'BQ conversions', + '--type', + 'custom_sql', + '--description', + 'BigQuery conversions', + '--custom-sql', + 'SELECT 1', + '--custom-statistics-type', + 'binomial', + '--datasource-id', + '10', + ]); + + expect(mockClient.createMetric).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'custom_sql', + custom_sql: 'SELECT 1', + datasource_id: 10, + }) + ); + }); + it('should create a goal_ratio metric with numerator and denominator types', async () => { await metricsCommand.parseAsync([ 'node', @@ -807,6 +835,41 @@ describe('metrics command', () => { expect(mockClient.activateMetric).not.toHaveBeenCalled(); }); + it('should pass --datasource-id through to the version payload', async () => { + await metricsCommand.parseAsync([ + 'node', + 'test', + 'version', + '1', + '--reason', + 'pin to bigquery', + '--datasource-id', + '10', + ]); + + expect(mockClient.createMetricVersion).toHaveBeenCalledWith( + 1, + { datasource_id: 10 }, + 'pin to bigquery' + ); + }); + + it('should omit datasource_id from the version payload when --datasource-id is not passed', async () => { + await metricsCommand.parseAsync([ + 'node', + 'test', + 'version', + '1', + '--reason', + 'rename', + '--name', + 'x', + ]); + + const payload = mockClient.createMetricVersion.mock.calls[0]![1] as Record; + expect(payload).not.toHaveProperty('datasource_id'); + }); + it('should support `new-version` alias', async () => { await metricsCommand.parseAsync([ 'node', diff --git a/src/core/metrics/payload.ts b/src/core/metrics/payload.ts index 26c6f3e..78e56a4 100644 --- a/src/core/metrics/payload.ts +++ b/src/core/metrics/payload.ts @@ -27,6 +27,7 @@ export interface MetricFields { activityInterval?: string | undefined; customSql?: string | undefined; customStatisticsType?: string | undefined; + datasourceId?: number | undefined; vrLookbackInterval?: string | undefined; relationKind?: string | undefined; relationRefundOperation?: string | undefined; @@ -81,6 +82,7 @@ export function buildMetricPayload(fields: MetricFields): Record Date: Mon, 28 Sep 2026 17:10:43 +0100 Subject: [PATCH 2/2] fix(metrics): use safe integer parser for --datasource-id Commander passes the previous option value as the parser's second argument, and parseInt treats that as a radix, so a repeated --datasource-id flag or a value like "10x" could silently resolve to the wrong datasource ID. Use the existing parseDatasourceId validator (used elsewhere for the same reason) instead of raw parseInt. --- src/commands/metrics/index.ts | 4 +-- src/commands/metrics/metrics.test.ts | 52 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/commands/metrics/index.ts b/src/commands/metrics/index.ts index 7f92d0c..ee8c1c4 100644 --- a/src/commands/metrics/index.ts +++ b/src/commands/metrics/index.ts @@ -7,7 +7,7 @@ import { printResult, withErrorHandling, } from '../../lib/utils/api-helper.js'; -import { parseMetricId } from '../../lib/utils/validators.js'; +import { parseMetricId, parseDatasourceId } from '../../lib/utils/validators.js'; import type { MetricId } from '../../lib/api/branded-types.js'; import { summarizeMetricRow } from '../../api-client/entity-summary.js'; import { createListCommand } from '../../lib/utils/list-command.js'; @@ -176,7 +176,7 @@ function addMetricFieldOptions(cmd: Command): Command { .option( '--datasource-id ', 'datasource ID the custom SQL runs against (custom_sql)', - parseInt + parseDatasourceId ) .option('--vr-lookback-interval ', 'VR lookback interval (1w, 2w, 3w, 4w)') .option('--relation-kind ', 'goal relation kind (refund, replacement)') diff --git a/src/commands/metrics/metrics.test.ts b/src/commands/metrics/metrics.test.ts index 37f942c..8727b8a 100644 --- a/src/commands/metrics/metrics.test.ts +++ b/src/commands/metrics/metrics.test.ts @@ -416,6 +416,58 @@ describe('metrics command', () => { ); }); + it('should use the last --datasource-id when the flag is repeated, not a radix-confused value', async () => { + await metricsCommand.parseAsync([ + 'node', + 'test', + 'create', + '--name', + 'BQ conversions', + '--type', + 'custom_sql', + '--description', + 'BigQuery conversions', + '--custom-sql', + 'SELECT 1', + '--custom-statistics-type', + 'binomial', + '--datasource-id', + '16', + '--datasource-id', + '10', + ]); + + expect(mockClient.createMetric).toHaveBeenCalledWith( + expect.objectContaining({ + datasource_id: 10, + }) + ); + }); + + it('should reject a --datasource-id with trailing non-numeric characters', async () => { + await expect( + metricsCommand.parseAsync([ + 'node', + 'test', + 'create', + '--name', + 'BQ conversions', + '--type', + 'custom_sql', + '--description', + 'BigQuery conversions', + '--custom-sql', + 'SELECT 1', + '--custom-statistics-type', + 'binomial', + '--datasource-id', + '10x', + ]) + ).rejects.toThrow(); + + expect(mockClient.createMetric).not.toHaveBeenCalled(); + }); + it('should create a goal_ratio metric with numerator and denominator types', async () => { await metricsCommand.parseAsync([ 'node',