mirror of
https://github.com/supabase/supabase.git
synced 2026-10-06 18:05:11 +03:00
032c71c5bfa1f4425d2f93072e28fa7d4e9a8321
1
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
32f17f994d |
fix(studio): key query chart series by position, not column name (#50270)
## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Bug fix. Hardening ahead of any decision to enable session replay. ### What's inside - ~47 lines of logic: positional series keys, the X-key collision guard, and the rewiring of rows, cumulative results and axis width ([QueryResultChart.tsx](https://github.com/supabase/supabase/pull/50270/changes#diff-dae994728613498678bcc06a3ea5176b36708e06b531cfc7eddae3e588fff528)) - ~123 lines of tests, one case per chart type, cumulative setting and edge case ([QueryResultChart.test.tsx](https://github.com/supabase/supabase/pull/50270/changes#diff-6e1c92ad725bac0324db4d079f3d0cd061b8f6e2aecdc21bcdc5969c90dc3c59)) ## What is the current behavior? Session replay is disabled in every environment, and no recordings exist. This is about what a recording *would* contain if it were ever switched on: charting a query result would put the customer's own column names into it. `ChartContainer` writes every chart config key into a `<style>` element as `--color-<key>`. rrweb records `<style>` text verbatim: ```js u = "STYLE" === parentTagName || void 0 h = "SCRIPT" === parentTagName || void 0 !u && !h && o && r && (o = maskTextFn ? maskTextFn(o, parent) : o.replace(/\S/g, "*")) ``` The `!u` guard means `maskTextFn` never sees stylesheet text. It is a text node, so `maskAttributeFn` does not see it either. CSS written into the DOM is a channel no masking hook reaches. `QueryResultChart` keyed its config by the column names picked for the Y axis, which come from the customer's own SQL results. With recording enabled, a query charting a column named `customer_email` would produce `--color-customer_email` in the captured DOM. Linear [GROWTH-1229](https://linear.app/supabase/issue/GROWTH-1229). #48818 masks the attribute channel. This channel is text, so that PR does not reach it. ## What is the new behavior? Y series are keyed by position (`series_0`, `series_1`), so no customer string reaches the config keys. The column name still goes through as `config[key].label`, which `chart.tsx` renders into the tooltip and legend as text. Text nodes outside `<style>` are masked by `maskReplayText`, so the name is safe there and the chart stays readable. The X column keeps its own name. Only config keys reach the `<style>` and the X column is never one, so renaming it would buy nothing, and it would cost the `timestamp` handling: `ChartBar` and `ChartLine` branch on `xKey === 'timestamp'` to format tooltip dates and render the date-range footer. Keeping the original name needs one guard, since an X column literally named `series_0` would share a row key with the first Y series. `xKeyFor` appends underscores until the two are distinct. ## Additional context ### Testing Eleven tests. They assert the rendered `<style>` contains `--color-series_0` and does not contain the column name, across both chart types and both cumulative settings, plus the two-series case, the `timestamp` column and the collision guard. Three are verified to catch the defect they cover: the style-key assertion fails against the unfixed component, the `timestamp` assertion fails when the X key is pinned to a constant, and the `xKeyFor` cases fail when the guard's body is removed. `vitest run components/interfaces/Explorer` passes: 170 tests across 16 files. The tooltip and legend path isn't asserted, because recharts doesn't render either one at the 0x0 size jsdom gives the container. That the label is what renders there comes from reading `chart.tsx`. It is unverified at runtime. ### Remaining exposure Every other `ChartContainer` caller passes literal keys (`--color-error`, `--color-ok_count`), and `UnifiedLogs` filters a static config by a fixed level set, so this was the only caller feeding it customer strings. `chart-bar.tsx:119-124` and `chart-line.tsx` still fall back to building a config from whatever `dataKeys` they're given, so a future caller can reintroduce this without touching `QueryResultChart`. A general guard would have to live in `ChartContainer` or in the replay config. CSSOM writes (`insertRule`, `replace`, `replaceSync`, adopted stylesheets) also emit CSS outside both masking hooks. This PR does not close that path. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved query result charts to preserve the source column name used for the X-axis. - Prevented X-axis names from conflicting with generated series identifiers. - Ensured bar and line charts consistently bind data to the correct X-axis and series. - Preserved timestamp-based chart behavior, including the date-range footer. - Chart series styling now uses stable positional identifiers, ensuring colors remain correctly assigned across configured series. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |