Skip to content

Commit 73d370d

Browse files
committed
fix(dashboards): unblock filter autocomplete + drop drifty test expect
Two fixes for #2383 CI: 1. DashboardFiltersModal default-value picker no longer issues a query when the resolved table name is empty. A metric source with no metricType picked yet resolves `getMetricTableName` to `source.from.tableName`, which is typically empty on a metric source. Firing the autocomplete in that state produces a malformed `DESCRIBE {db}.{}` (empty Identifier substitution) that the ClickHouse proxy rejects with a 500. The e2e shard 2 trace shows exactly this 500 firing during filter editing, which left the dashboard filter chip dropdowns disabled long enough to time out the click-the-option waits. Gating on `tableName` keeps the picker idle until the form is configured enough to name a real table. 2. The external API HDX-4404 round-trip test asserted `whereLanguage: 'sql'` on the response for filters whose input payload omits `whereLanguage`. The Zod schema is `.optional()` with no default, so the wire format is undefined-in / undefined-out. The previous expectation made the test fail by asserting a default the schema never emits. Dropped the field from those three filter expects and from the PUT-flip filter expect; added a comment so the omission is intentional.
1 parent 21880e4 commit 73d370d

2 files changed

Lines changed: 16 additions & 5 deletions

File tree

packages/api/src/routers/external-api/__tests__/dashboards.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -962,13 +962,18 @@ describe('External API v2 Dashboards - old format', () => {
962962
// indicate schema drift) fail the assertion. The bot deep-review at
963963
// dashboards.test.ts:961 flagged toMatchObject for letting unexpected
964964
// fields slip through silently.
965+
//
966+
// whereLanguage is intentionally absent from these expectations: the
967+
// input payload omits it, the Zod schema is optional with no default,
968+
// so the round-trip is undefined-in / undefined-out. Adding the field
969+
// here would make the test fail by asserting a wire-format default
970+
// the schema never emits.
965971
expect(response.body.data.filters[0]).toEqual({
966972
id: response.body.data.filters[0].id,
967973
type: 'QUERY_EXPRESSION',
968974
name: 'Service (locked, read-only)',
969975
expression: 'ServiceName',
970976
sourceId: traceSource._id.toString(),
971-
whereLanguage: 'sql',
972977
constant: true,
973978
renderMode: 'readonly',
974979
});
@@ -978,7 +983,6 @@ describe('External API v2 Dashboards - old format', () => {
978983
name: 'Environment (hidden)',
979984
expression: 'environment',
980985
sourceId: traceSource._id.toString(),
981-
whereLanguage: 'sql',
982986
constant: true,
983987
renderMode: 'hidden',
984988
});
@@ -991,7 +995,6 @@ describe('External API v2 Dashboards - old format', () => {
991995
name: 'Region (default editable)',
992996
expression: 'region',
993997
sourceId: traceSource._id.toString(),
994-
whereLanguage: 'sql',
995998
});
996999

9971000
// GET round-trip.
@@ -1041,7 +1044,6 @@ describe('External API v2 Dashboards - old format', () => {
10411044
name: 'Region (now read-only)',
10421045
expression: 'region',
10431046
sourceId: traceSource._id.toString(),
1044-
whereLanguage: 'sql',
10451047
constant: true,
10461048
renderMode: 'readonly',
10471049
});

packages/app/src/DashboardFiltersModal.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,16 @@ const DashboardFilterEditForm = ({
364364
// default-value picker can query autocomplete values matching the
365365
// configured source / expression / WHERE. Memoize on the primitive
366366
// bits so we don't re-issue the same query on unrelated form edits.
367+
//
368+
// Gate on `tableName` (not just `sourceId`): a metric source with no
369+
// metricType picked yet resolves to `source.from.tableName`, which is
370+
// typically empty on a metric source. Firing the autocomplete in that
371+
// state produces a malformed DESCRIBE ({db}.{} with an empty Identifier
372+
// substitution) that the ClickHouse proxy rejects with a 500. The
373+
// query only makes sense once the form has enough information to name
374+
// a real table.
367375
const filterForValueQuery = useMemo(() => {
368-
if (!sourceId || !watchedExpression) return null;
376+
if (!sourceId || !watchedExpression || !tableName) return null;
369377
return {
370378
id: filter.id,
371379
type: 'QUERY_EXPRESSION' as const,
@@ -386,6 +394,7 @@ const DashboardFilterEditForm = ({
386394
watchedWhere,
387395
watchedWhereLanguage,
388396
metricType,
397+
tableName,
389398
]);
390399

391400
const [modalContentRef, setModalContentRef] = useState<HTMLElement | null>(

0 commit comments

Comments
 (0)