[HDX-4120] feat(api): support heatmap tiles in external dashboards API#2200
[HDX-4120] feat(api): support heatmap tiles in external dashboards API#2200alex-fedotyev wants to merge 6 commits intomainfrom
Conversation
Heatmap is the only builder-mode display type that did not round-trip through the external dashboards API. The serializer dropped it into the "unsupported" fall-through, so creating, fetching, and updating heatmap tiles via /api/v2/dashboards lost the config. This adds: - A heatmap select-item schema in zod.ts that exposes the literal aggFn 'heatmap', valueExpression, optional countExpression, alias, and heatmapScaleType. Heatmap is included in the builder discriminated union only; it is intentionally not added to the raw SQL union since heatmap rendering requires isBuilderChartConfig. - A heatmap chart config schema with optional groupBy and numberFormat. - Builder serialization that reads the heatmap-specific fields off config.select[0] and emits aggFn 'heatmap' on the external surface, passing through groupBy and numberFormat. - Builder deserialization that maps the external aggFn 'heatmap' back to the internal aggFn 'count' the editor form persists, preserving countExpression, alias, and heatmapScaleType on the select item. - OpenAPI JSDoc with HeatmapSelectItem and HeatmapBuilderChartConfig components, and heatmap added to the TileConfig discriminator. - Integration tests: heatmap added to the existing "round-trip all supported chart types" tests for both POST and PUT, plus an explicit rejection test for raw SQL heatmap tiles. - Regenerated openapi.json. Follow-up to #2107 (review feedback from @pulpdrew).
🦋 Changeset detectedLatest commit: e1d1d9d The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
review/tier-4
🔴 Tier 4 — CriticalTouches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD. Why this tier:
Review process: Deep review from a domain expert. Synchronous walkthrough may be required. Stats
|
E2E Test Results✅ All tests passed • 159 passed • 3 skipped • 1192s
Tests ran across 4 shards in parallel. |
PR ReviewPR #2200 — feat(api): support heatmap tiles in external dashboards API Overall this is a well-structured PR that closely follows existing patterns (mirrors the pie chart implementation). Test coverage is thorough and the OpenAPI docs are complete.
✅ No critical security issues. Validation logic, aggFn translation ( |
knip flagged the exported type as unused. Extracted the inline heatmap select-item construction into a small convertToExternalHeatmapSelectItem helper that mirrors the existing convertToExternalSelectItem pattern and returns the typed shape, kills the knip warning, and removes the duplicated nullish-spread bookkeeping at the call site. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reviewer flagged that convertToExternalHeatmapSelectItem used truthy checks (countExpression ? ...) while convertToInternalTileConfig used !== undefined. An empty-string countExpression/alias round-trip would silently drop the field. Use !== undefined consistently in both directions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Thanks for the review.
|
The HeatmapSeriesEditor in the UI binds a single SearchWhereInput to the
top-level `where` / `whereLanguage` and does not render any groupBy
input. The previous schema had it backwards: it carried groupBy (not
exposed in the UI, never set by the form) and was missing
where / whereLanguage (the only filter heatmap actually uses).
- zod: drop groupBy, add where + whereLanguage to
externalDashboardHeatmapChartConfigSchema
- conversion utils: serializer emits chart-level where + whereLanguage;
deserializer maps them onto BuilderSavedChartConfig.where /
whereLanguage instead of stuffing them into select-item aggCondition
- OpenAPI: HeatmapBuilderChartConfig has where + whereLanguage instead
of groupBy; description notes the row-level filter is at chart level
- tests: POST round-trip exercises sql where ("ServiceName = 'api'")
with heatmapScaleType: 'log'; PUT round-trip exercises lucene where
("service:api") with heatmapScaleType: 'linear'. Both languages and
both scale types are now covered end-to-end.
- regenerated openapi.json
…ression
Two more gaps found while auditing the schema against the heatmap UI surface:
1. The heatmap UI's source picker is restricted to SourceKind.Trace
(ChartEditorControls.tsx:103-107: allowedSourceKinds={[SourceKind.Trace]}).
The external API previously accepted any source kind, so a metric or log
source would round-trip cleanly through /api/v2/dashboards and produce a
tile that does not render in the UI. Adds getHeatmapTilesWithNonTraceSources
and wires it into both POST and PUT alongside the existing source /
connection validators, returning 400 with a descriptive message.
2. validateChartForm in the editor rejects empty valueExpression on heatmap
("Value expression is required for heatmap charts"). The Zod schema
accepted empty string. Tightens to z.string().min(1).max(10000) and
updates the OpenAPI minLength to match.
Tests added: rejection on non-Trace source (with assertion on the error
message), rejection on empty valueExpression, rejection on multi-item
select array.
Regenerated openapi.json.
1 participant
Summary
Heatmap was the only builder-mode display type that did not round-trip through the external dashboards API. The serializer dropped it into the "unsupported" fall-through, so creating, fetching, and updating heatmap tiles via
/api/v2/dashboardslost the config.This wires up heatmap end-to-end on the external API: a dedicated select-item schema, an explicit case in both serialization directions, OpenAPI JSDoc, and tests.
Follow-up to #2107 (review feedback from @pulpdrew, who asked whether we had a follow-up ticket to update the external API for the new visualization type).
What's in the diff
packages/api/src/utils/zod.ts): a heatmap select-item schema that exposes the literalaggFn: "heatmap"plusvalueExpression, optionalcountExpression,alias,heatmapScaleType; and a heatmap chart-config schema with optionalgroupByandnumberFormat. Heatmap is added to the builder discriminated union only.packages/api/src/routers/external-api/v2/utils/dashboards.ts):case DisplayType.Heatmap:fall-through with an explicit case that reads heatmap-specific fields offconfig.select[0]and emitsaggFn: "heatmap"on the external surface.case 'heatmap':mirroring the Pie pattern; maps the externalaggFn: "heatmap"back to the internalaggFn: "count"that the editor form persists, while preservingcountExpression,alias, andheatmapScaleTypeon the select item.isBuilderChartConfig.packages/api/src/routers/external-api/v2/dashboards.ts):HeatmapSelectItemandHeatmapBuilderChartConfigcomponents, andheatmapadded to theTileConfigoneOfand discriminator mapping.packages/api/src/routers/external-api/__tests__/dashboards.test.ts): heatmap added to the existing "round-trip all supported chart types" tests for both POST and PUT, plus an explicit rejection test confirming raw-SQL heatmap tiles return 400.packages/api/openapi.json: regenerated.Notes for review
DBDashboardPage.tsxrequiresisBuilderChartConfigfor heatmap rendering, so the raw-SQL fall-through stays.heatmapScaleTypeandcountExpressionare persisted on the per-select-item level (viaDerivedColumnSchemainpackages/common-utils/src/types.ts), not on the chart config root. The form binds them asseries.0.heatmapScaleType/series.0.countExpression. The schema and conversion utilities follow that."heatmap"↔ internal"count") keeps the saved Mongo document identical to what the editor form produces, so heatmap tiles created via the API render the same way as ones created via the UI.Tier
Lands as
review/tier-4because anything underpackages/api/src/routers/external-api/is on the critical-path list. Diff is ~250 prod lines (most of it OpenAPI JSDoc and Zod boilerplate); no schema migrations or auth changes.Test plan
make ci-lint(yarn lint, tsc --noEmit, OpenAPI lint)make ci-unit(common-utils + app)make dev-intrequires Docker BuildKit which isn't available on this host.