Skip to content

Commit 2d1312b

Browse files
committed
fix(dashboard): address Seer and Bugbot review findings
- Fix validateQueryConstraints false positive: only check group-by+limit when user explicitly passes --group-by, not when merely changing --query on an existing grouped widget (which may have auto-defaulted columns) - Fix orphaned JSDoc: move buildWidgetFromFlags doc comment to its actual function definition after the validation helpers were inserted
1 parent 9c1271a commit 2d1312b

2 files changed

Lines changed: 13 additions & 10 deletions

File tree

src/commands/dashboard/resolve.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -316,15 +316,6 @@ export function resolveWidgetIndex(
316316
return matchIndex;
317317
}
318318

319-
/**
320-
* Build a widget from user-provided flag values.
321-
*
322-
* Shared between `dashboard widget add` and `dashboard widget edit`.
323-
* Parses aggregate shorthand, sort expressions, and validates via Zod schema.
324-
*
325-
* @param opts - Widget configuration from parsed flags
326-
* @returns Validated widget with computed query fields
327-
*/
328319
/**
329320
* Validate that a sort expression references an aggregate present in the query.
330321
* The Sentry API returns 400 when the sort field isn't in the widget's aggregates.
@@ -421,6 +412,15 @@ function warnUnknownAggregateFields(
421412
}
422413
}
423414

415+
/**
416+
* Build a widget from user-provided flag values.
417+
*
418+
* Shared between `dashboard widget add` and `dashboard widget edit`.
419+
* Parses aggregate shorthand, sort expressions, and validates via Zod schema.
420+
*
421+
* @param opts - Widget configuration from parsed flags
422+
* @returns Validated widget with computed query fields
423+
*/
424424
export function buildWidgetFromFlags(opts: {
425425
title: string;
426426
display: string;

src/commands/dashboard/widget/edit.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,10 @@ function validateQueryConstraints(
137137
mergedQueries: DashboardWidgetQuery[] | undefined,
138138
limit: number | null | undefined
139139
): void {
140-
if (flags["group-by"] || flags.query) {
140+
// Only validate when user explicitly passes --group-by, not when merely
141+
// changing --query on an existing grouped widget (which may have auto-defaulted
142+
// columns like ["issue"] with no limit)
143+
if (flags["group-by"]) {
141144
const columns =
142145
mergedQueries?.[0]?.columns ?? existing.queries?.[0]?.columns ?? [];
143146
validateGroupByRequiresLimit(columns, limit ?? undefined);

0 commit comments

Comments
 (0)