Skip to content

Commit 15c9230

Browse files
betegonclaude
andcommitted
fix(dashboard): address PR review comments on widget edit
1. Fix stale `fields` in mergeQueries — destructure to omit existing `fields` so prepareWidgetQueries recomputes from updated aggregates/ columns (bugbot comment #1) 2. Preserve widgetType without silent default — only set widgetType if explicitly provided via --dataset or already on the existing widget, preventing parseWidgetInput from defaulting to "spans" (bugbot #4) 3. Resolve aliases in paren passthrough — `spm()` now correctly resolves to `epm()` instead of being rejected by validation (bugbot #3) Comment #2 (drops interval/thresholds) is not a real issue — these fields are already stripped by the allowlist in stripWidgetServerFields before buildReplacement sees them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 753f145 commit 15c9230

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

src/commands/dashboard/widget/edit.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,12 @@ function mergeQueries(
6363
return; // signal: keep existing
6464
}
6565

66+
// Destructure to omit stale `fields` — prepareWidgetQueries will
67+
// recompute it from the updated aggregates/columns.
68+
const { fields: _staleFields, ...base } = existingQuery ?? {};
6669
return [
6770
{
68-
...existingQuery,
71+
...base,
6972
...(flags.query && { aggregates: flags.query.map(parseAggregate) }),
7073
...(flags.where !== undefined && { conditions: flags.where }),
7174
...(flags["group-by"] && { columns: flags["group-by"] }),
@@ -81,7 +84,6 @@ function buildReplacement(
8184
): DashboardWidget {
8285
const mergedQueries = mergeQueries(flags, existing.queries?.[0]);
8386

84-
const widgetType = flags.dataset ?? existing.widgetType;
8587
const limit = flags.limit !== undefined ? flags.limit : existing.limit;
8688

8789
const raw: Record<string, unknown> = {
@@ -90,8 +92,12 @@ function buildReplacement(
9092
queries: mergedQueries ?? existing.queries,
9193
layout: existing.layout,
9294
};
93-
if (widgetType) {
94-
raw.widgetType = widgetType;
95+
// Only set widgetType if explicitly provided via --dataset or already on the widget.
96+
// Avoids parseWidgetInput defaulting to "spans" for widgets without a widgetType.
97+
if (flags.dataset) {
98+
raw.widgetType = flags.dataset;
99+
} else if (existing.widgetType) {
100+
raw.widgetType = existing.widgetType;
95101
}
96102
if (limit !== undefined) {
97103
raw.limit = limit;

src/types/dashboard.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,11 @@ export const IsFilterValueSchema = z.enum(IS_FILTER_VALUES);
356356
*/
357357
export function parseAggregate(input: string): string {
358358
if (input.includes("(")) {
359-
return input;
359+
// Resolve aliases even in paren form: spm() → epm(), tpm(x) → epm(x)
360+
const parenIdx = input.indexOf("(");
361+
const fn = input.slice(0, parenIdx);
362+
const alias = AGGREGATE_ALIASES[fn];
363+
return alias ? `${alias}${input.slice(parenIdx)}` : input;
360364
}
361365
const colonIdx = input.indexOf(":");
362366
if (colonIdx > 0) {

0 commit comments

Comments
 (0)