Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/large-suits-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@hyperdx/app": patch
---

Preserve conditional color rules for SQL and PromQl chart configs
10 changes: 9 additions & 1 deletion packages/api/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -2302,7 +2302,15 @@
},
"color": {
"$ref": "#/components/schemas/ChartPaletteToken",
"description": "Optional static color applied to the displayed number. Raw SQL number tiles do not support conditional colorRules.\n"
"description": "Optional static color applied to the displayed number."
},
"colorRules": {
"type": "array",
"maxItems": 10,
"description": "Ordered conditional color rules evaluated against the displayed value (last match wins). Falls back to color, then the default text color when no rule matches.\n",
"items": {
"$ref": "#/components/schemas/NumberTileColorCondition"
}
}
}
}
Expand Down
114 changes: 101 additions & 13 deletions packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3073,7 +3073,7 @@ describe('External API v2 Dashboards - new format', () => {
sqlTemplate,
sourceId,
numberFormat: { output: 'currency', currencySymbol: '$' },
// Raw SQL number tiles carry the static tile color (no colorRules).
// This fixture exercises the static color; raw SQL number tiles also support colorRules.
color: 'chart-purple',
},
};
Expand Down Expand Up @@ -4783,7 +4783,7 @@ describe('External API v2 Dashboards - new format', () => {
sqlTemplate,
sourceId,
numberFormat: { output: 'currency', currencySymbol: '$' },
// Raw SQL number tiles carry the static tile color (no colorRules).
// This fixture exercises the static color; raw SQL number tiles also support colorRules.
color: 'chart-purple',
},
};
Expand Down Expand Up @@ -5360,6 +5360,13 @@ describe('External API v2 Dashboards - new format', () => {
},
});

const postRawSqlTile = (config: Record<string, unknown>) =>
authRequest('post', BASE_URL).send({
name: 'Raw SQL number color dashboard',
tiles: [rawSqlNumberTile(config)],
tags: [],
});

// ── Positive: one per UI input ──────────────────────────────────────

it('round-trips a builder number tile with a static color', async () => {
Expand Down Expand Up @@ -5455,21 +5462,52 @@ describe('External API v2 Dashboards - new format', () => {
});
});

it('strips colorRules from a raw SQL number tile, keeping color', async () => {
it('round-trips colorRules for a raw SQL number tile', async () => {
const colorRules = [
{ operator: 'gt', value: 1, color: 'chart-red' },
{ operator: 'lte', value: 1, color: 'chart-green' },
];

const create = await authRequest('post', BASE_URL)
.send({
name: 'Raw SQL colorRules',
tiles: [
rawSqlNumberTile({
color: 'chart-blue',
colorRules: [{ operator: 'gt', value: 1, color: 'chart-red' }],
colorRules,
}),
],
tags: [],
})
.expect(200);
expect(create.body.data.tiles[0].config.color).toBe('chart-blue');
expect(create.body.data.tiles[0].config.colorRules).toBeUndefined();

expect(create.body.data.tiles[0].config).toMatchObject({
color: 'chart-blue',
colorRules,
});

const dashboardId = create.body.data.id;
const get = await authRequest('get', `${BASE_URL}/${dashboardId}`).expect(
200,
);

expect(get.body.data.tiles[0].config).toMatchObject({
color: 'chart-blue',
colorRules,
});

const update = await authRequest('put', `${BASE_URL}/${dashboardId}`)
.send({
name: get.body.data.name,
tiles: get.body.data.tiles,
tags: get.body.data.tags,
})
.expect(200);

expect(update.body.data.tiles[0].config).toMatchObject({
color: 'chart-blue',
colorRules,
});
});

// ── Negative: one per schema rejection rule ─────────────────────────
Expand Down Expand Up @@ -5499,6 +5537,16 @@ describe('External API v2 Dashboards - new format', () => {
expect(res.body.message).toContain('tiles.0.config.colorRules');
});

it('rejects more than 10 colorRules for a raw SQL number tile', async () => {
const colorRules = Array.from({ length: 11 }, (_, i) => ({
operator: 'gt',
value: i,
color: 'chart-blue',
}));
const res = await postRawSqlTile({ colorRules }).expect(400);
expect(res.body.message).toContain('tiles.0.config.colorRules');
});

it('rejects a between rule whose value is not a two-number tuple', async () => {
await postTile({
colorRules: [{ operator: 'between', value: 100, color: 'chart-blue' }],
Expand All @@ -5520,6 +5568,15 @@ describe('External API v2 Dashboards - new format', () => {
}
});

it('rejects unsupported colorRule operators for a raw SQL number tile', async () => {
for (const operator of ['contains', 'startsWith', 'endsWith', 'regex']) {
const res = await postRawSqlTile({
colorRules: [{ operator, value: 'error', color: 'chart-blue' }],
}).expect(400);
expect(res.body.message).toContain('tiles.0.config.colorRules');
}
});

it('rejects a per-rule color that is not a palette token', async () => {
const res = await postTile({
colorRules: [{ operator: 'gt', value: 1, color: 'red' }],
Expand All @@ -5531,6 +5588,17 @@ describe('External API v2 Dashboards - new format', () => {
}).expect(400);
});

it('rejects a per-rule color that is not a palette token for a raw SQL number tile', async () => {
const res = await postRawSqlTile({
colorRules: [{ operator: 'gt', value: 1, color: 'red' }],
}).expect(400);
expect(res.body.message).toContain('tiles.0.config.colorRules');

await postRawSqlTile({
colorRules: [{ operator: 'gt', value: 1, color: 'chart-1' }],
}).expect(400);
});

it('rejects a rule label longer than 40 characters', async () => {
await postTile({
colorRules: [
Expand Down Expand Up @@ -5629,13 +5697,7 @@ describe('External API v2 Dashboards - new format', () => {
});

it('normalizes a legacy numeric token on a raw SQL number tile to its hue name on read', async () => {
const create = await authRequest('post', BASE_URL)
.send({
name: 'Raw SQL legacy color',
tiles: [rawSqlNumberTile({ color: 'chart-blue' })],
tags: [],
})
.expect(200);
const create = await postRawSqlTile({ color: 'chart-blue' }).expect(200);
const dashboardId = create.body.data.id;

await Dashboard.updateOne(
Expand All @@ -5649,6 +5711,32 @@ describe('External API v2 Dashboards - new format', () => {
// chart-4 maps to chart-red.
expect(get.body.data.tiles[0].config.color).toBe('chart-red');
});

it('normalizes legacy raw SQL colorRule colors and drops unresolvable ones on read', async () => {
const create = await postRawSqlTile({
colorRules: [{ operator: 'gt', value: 1, color: 'chart-green' }],
}).expect(200);
const dashboardId = create.body.data.id;

await Dashboard.updateOne(
{ _id: dashboardId },
{
$set: {
'tiles.0.config.colorRules': [
{ operator: 'gt', value: 1, color: 'chart-1' },
{ operator: 'gt', value: 2, color: 'not-a-token' },
],
},
},
);

const get = await authRequest('get', `${BASE_URL}/${dashboardId}`).expect(
200,
);
expect(get.body.data.tiles[0].config.colorRules).toEqual([
{ operator: 'gt', value: 1, color: 'chart-green' },
]);
});
});

describe('Number tile background chart (HDX-1360)', () => {
Expand Down
11 changes: 9 additions & 2 deletions packages/api/src/routers/external-api/v2/dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1174,9 +1174,16 @@ const EXTERNAL_DASHBOARD_PROJECTION = {
* example: "number"
* color:
* $ref: '#/components/schemas/ChartPaletteToken'
* description: Optional static color applied to the displayed number.
* colorRules:
* type: array
* maxItems: 10
* description: >
* Optional static color applied to the displayed number. Raw
* SQL number tiles do not support conditional colorRules.
* Ordered conditional color rules evaluated against the displayed
* value (last match wins). Falls back to color, then the default
* text color when no rule matches.
* items:
* $ref: '#/components/schemas/NumberTileColorCondition'
*
* PieRawSqlChartConfig:
* description: Raw SQL configuration for a pie chart.
Expand Down
10 changes: 5 additions & 5 deletions packages/api/src/routers/external-api/v2/utils/dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,8 @@ const convertToExternalTileChartConfig = (
sqlTemplate: config.sqlTemplate,
sourceId: config.source,
numberFormat: config.numberFormat,
// Raw SQL number tiles carry the static tile color too (no
// colorRules; see the schema). Normalize a legacy token saved
// before the hue rename to its hue name on output.
color: resolveChartPaletteToken(config.color),
colorRules: toExternalColorRules(config.colorRules),
};
case DisplayType.Pie:
return {
Expand Down Expand Up @@ -685,12 +683,14 @@ export function convertToInternalTileConfig(
externalConfig.displayType === 'table'
? externalConfig.onClick
: undefined,
// Only the raw SQL number variant carries `color`; table and pie
// do not expose it. `_.omitBy(_.isNil)` below drops it when absent.
color:
externalConfig.displayType === 'number'
? externalConfig.color
: undefined,
colorRules:
externalConfig.displayType === 'number'
? externalConfig.colorRules
: undefined,
} satisfies RawSqlSavedChartConfig;
break;
default:
Expand Down
14 changes: 4 additions & 10 deletions packages/api/src/utils/zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,14 +321,8 @@ const externalDashboardTableRawSqlChartConfigSchema =
const externalDashboardNumberRawSqlChartConfigSchema =
externalDashboardRawSqlChartConfigBaseSchema.extend({
displayType: z.literal('number'),
// Raw SQL number tiles expose the same static tile color as builder
// number tiles: the editor gates the picker on displayType, not
// configType (`ChartDisplaySettingsDrawer`). `colorRules` is
// intentionally omitted here because the editor's save path
// (`convertFormStateToSavedChartConfig`) picks `color` but not
// `colorRules` for raw SQL configs, so persisted raw SQL number tiles
// never carry rules.
color: ChartPaletteTokenSchema.optional(),
colorRules: z.array(NumberTileColorConditionSchema).max(10).optional(),
});

const externalDashboardPieRawSqlChartConfigSchema =
Expand Down Expand Up @@ -368,9 +362,9 @@ const externalDashboardNumberChartConfigSchema = z.object({
// `configType === 'sql'`). The save path
// (`convertFormStateToSavedChartConfig`) persists `backgroundChart` only on
// the builder branch (the raw SQL / promql picks omit it), so it lives on
// the builder number schema only, like `colorRules`. `BackgroundChartSchema`
// is imported from common-utils so the external surface cannot drift from
// what the UI persists.
// the builder number schema only. `BackgroundChartSchema` is imported from
// common-utils so the external surface cannot drift from what the UI
// persists.
backgroundChart: BackgroundChartSchema.optional(),
});

Expand Down
46 changes: 46 additions & 0 deletions packages/app/src/components/ChartEditor/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,52 @@ describe('color round-trip (sql/promql Number tile)', () => {
expect((result as any).color).toBe('chart-error');
});

it('preserves colorRules through both conversions for sql Number tile', () => {
const form: ChartEditorFormState = {
configType: 'sql',
displayType: DisplayType.Number,
sqlTemplate: 'SELECT count() FROM logs',
connection: 'conn-1',
colorRules: [
{ operator: 'lt', value: 4, color: 'chart-error' },
{ operator: 'gte', value: 4, color: 'chart-success' },
],
series: [],
};

expect(convertFormStateToSavedChartConfig(form, undefined)).toMatchObject({
colorRules: form.colorRules,
});
expect(
convertFormStateToChartConfig(form, dateRange, undefined),
).toMatchObject({
colorRules: form.colorRules,
});
});

it('preserves colorRules through both conversions for promql Number tile', () => {
const form: ChartEditorFormState = {
configType: 'promql',
displayType: DisplayType.Number,
promqlExpression: 'up',
connection: 'conn-1',
colorRules: [
{ operator: 'lt', value: 1, color: 'chart-error' },
{ operator: 'gte', value: 1, color: 'chart-success' },
],
series: [],
};

expect(convertFormStateToSavedChartConfig(form, undefined)).toMatchObject({
colorRules: form.colorRules,
});
expect(
convertFormStateToChartConfig(form, dateRange, undefined),
).toMatchObject({
colorRules: form.colorRules,
});
});

it('omits color when not set on sql Number tile', () => {
const form: ChartEditorFormState = {
configType: 'sql',
Expand Down
4 changes: 4 additions & 0 deletions packages/app/src/components/ChartEditor/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export function convertFormStateToSavedChartConfig(
'displayType',
'numberFormat',
'color',
'colorRules',
'granularity',
'compareToPreviousPeriod',
'fillNulls',
Expand All @@ -180,6 +181,7 @@ export function convertFormStateToSavedChartConfig(
'displayType',
'numberFormat',
'color',
'colorRules',
Comment thread
greptile-apps[bot] marked this conversation as resolved.
'granularity',
'compareToPreviousPeriod',
'fillNulls',
Expand Down Expand Up @@ -238,6 +240,7 @@ export function convertFormStateToChartConfig(
'displayType',
'numberFormat',
'color',
'colorRules',
'granularity',
'compareToPreviousPeriod',
'fillNulls',
Expand All @@ -261,6 +264,7 @@ export function convertFormStateToChartConfig(
'displayType',
'numberFormat',
'color',
'colorRules',
'granularity',
'compareToPreviousPeriod',
'fillNulls',
Expand Down
Loading