Skip to content

fix(explore): persist aliased column config by real key - #44955

Merged
EnxDev merged 17 commits into
apache:masterfrom
AmoghAtreya:fix/column-config-aliased-column-key
Oct 23, 2026
Merged

EnxDev merged 17 commits into
apache:masterfrom
AmoghAtreya:fix/column-config-aliased-column-key

Conversation

@AmoghAtreya

@AmoghAtreya AmoghAtreya commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

SUMMARY

Separates the real column key from its display alias in ColumnConfigControl.

Aliased columns such as __timestamp (“Time”) now store configuration under the real column key, ensuring edits persist when the configuration popover is reopened.

Fixes #44911.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable.

TESTING INSTRUCTIONS

  1. Run:
    npm test -- src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.test.tsx --maxWorkers=2
  2. Confirm all tests pass.
  3. Open a chart containing __timestamp.
  4. Edit the “Time” column configuration and reopen it.
  5. Confirm the setting persists under column_config.__timestamp.

ADDITIONAL INFORMATION

Co-authored-by: Cursor <cursoragent@cursor.com>
@bito-code-review

bito-code-review Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Agent Run #3a01dc

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 43bdabf..43bdabf
    • superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.test.tsx
    • superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx
    • superset-frontend/src/explore/components/controls/ColumnConfigControl/types.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - Manually triggers a full AI Review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. When setColumnConfig is called with col.name (which may be an alias like 'Time'), it overwrites the configuration for that key, potentially discarding existing settings stored under the actual column key (e.g., __timestamp).

To resolve this, ensure the onChange callback preserves existing configuration properties by merging the new config with the existing one. You can update the onChange handler in ColumnConfigControl.tsx as follows:

onChange={config => setColumnConfig(col.key, { ...col.config, ...config } as T)}

There are no other comments on this PR to address.

superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx

onChange={config => setColumnConfig(col.key, { ...col.config, ...config } as T)}

@codecov

codecov Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.44%. Comparing base (abc1b50) to head (0642b92).
⚠️ Report is 55 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #44955       +/-   ##
===========================================
- Coverage   84.01%   65.44%   -18.58%     
===========================================
  Files        3057     3057               
  Lines      201186   201229       +43     
  Branches    47028    47054       +26     
===========================================
- Hits       169032   131699    -37333     
- Misses      28914    66578    +37664     
+ Partials     3240     2952      -288     
Flag Coverage Δ
javascript 80.12% <100.00%> (+0.01%) ⬆️
postgres ?
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bito-code-review

bito-code-review Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Agent Run #89ee34

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 43bdabf..45de404
    • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.test.ts
    • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts
    • superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - Manually triggers a full AI Review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@SBIN2010
SBIN2010 requested a balanced review from Copilot October 5, 2026 17:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Fixes persistence of column configuration for aliased/display-named columns (e.g., __timestamp shown as “Time”) by separating the real column key from the display name and normalizing legacy stored keys.

Changes:

  • Add a key field to ColumnConfigInfo and ensure edits are saved under the real column key rather than the display alias.
  • Extend normalizeColumnConfigKeys to remap configs saved under display aliases back onto real column keys.
  • Add test coverage for aliased-column persistence and legacy alias-key migration.
File Description
superset-frontend/​src/​explore/​components/​controls/​ColumnConfigControl/​types.ts Adds real key to the column config info model.
superset-frontend/​src/​explore/​components/​controls/​ColumnConfigControl/​ColumnConfigControl.tsx Uses real column key for persisting config updates.
superset-frontend/​src/​explore/​components/​controls/​ColumnConfigControl/​ColumnConfigControl.test.tsx Adds tests for aliased key persistence + legacy alias migration behavior.
superset-frontend/​packages/​superset-ui-chart-controls/​src/​utils/​headerGroups.ts Normalizes legacy column_config entries saved under display aliases.
superset-frontend/​packages/​superset-ui-chart-controls/​src/​utils/​headerGroups.test.ts Adds tests for alias-to-real-key normalization behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts Outdated

@SBIN2010 SBIN2010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing new edits under col.key fixes the reported bug. The legacy remap still needs a tighter match (real columns named like the alias, and non-English saved keys), and chart rendering still reads the unmigrated key until the next edit.

Comment thread superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@netlify

netlify Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 92f3402
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6ad85f4ba6e57744e14dec1e
😎 Deploy Preview https://deploy-preview-44955--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@SBIN2010 SBIN2010 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chart rendering now keeps a real Time column off __timestamp. The control that persists column_config still normalizes without those known names, so the next edit moves that config onto __timestamp.

@bito-code-review

bito-code-review Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Agent Run #7e60cf

Actionable Suggestions - 0
Review Details
  • Files reviewed - 9 · Commit Range: 45de404..ff12102
    • superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.tsx
    • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.test.ts
    • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts
    • superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts
    • superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
    • superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
    • superset-frontend/plugins/plugin-chart-echarts/src/Radar/controlPanel.tsx
    • superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
    • superset-frontend/src/explore/components/controls/ColumnConfigControl/ColumnConfigControl.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - Manually triggers a full AI Review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
Comment thread superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
@codeant-ai-for-open-source

codeant-ai-for-open-source Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

CodeAnt PR Risk: Low Risk

  • The PR appears safe to merge; legacy display-name settings are normalized to stored column keys before edits, with regression tests covering preservation.

Assessed commit: 0642b920a4b7

Comment thread superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
@pull-request-size pull-request-size Bot added size/XL and removed size/L labels Oct 6, 2026
Comment thread superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts Outdated

@bito-code-review bito-code-review Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #17c4fa

Actionable Suggestions - 1
  • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts - 1
    • CWE-1321: Prevent prototype pollution via object key · Line 394-394
Review Details
  • Files reviewed - 10 · Commit Range: ff12102..4bbf797
    • superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
    • superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx
    • superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
    • superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx
    • superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
    • superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
    • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.test.ts
    • superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts
    • superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/isEqualColumns.ts
    • superset-frontend/plugins/plugin-chart-table/src/utils/isEqualColumns.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers an incremental AI Review.

  • /review full - Manually triggers a full AI Review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts Outdated
@github-actions github-actions Bot added the requires:rebase Requires rebasing on top of current master label Oct 14, 2026
@sadpandajoe

Copy link
Copy Markdown
Member

@AmoghAtreya looks like there are merge conflicts, mind taking care of those?

…liased-column-key

Co-authored-by: Cursor <cursoragent@cursor.com>
@AmoghAtreya

Copy link
Copy Markdown
Contributor Author

Thanks for the message @sadpandajoe. Merged latest master into this branch and resolved the conflict in plugin-chart-table/test/controlPanel.test.tsx. Kept the knownKeys Time-metric test from this PR and the new pagination/row-limit tests from master. Should be unconflicted now.

@github-actions github-actions Bot removed the requires:rebase Requires rebasing on top of current master label Oct 21, 2026
Comment thread superset-frontend/packages/superset-ui-chart-controls/src/utils/headerGroups.ts Outdated

@EnxDev EnxDev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read through this one and it holds up. Edits land on col.key, legacy Time entries migrate at render as well as in the control, and real or dropped Time columns and metrics stay off __timestamp. The earlier threads (ordering, knownKeys before comparison expansion, render-path tests) are all reflected in the diff with tests that would fail without them.

One small nit inline on getKnownColumnConfigKeys, nothing blocking.

@EnxDev
EnxDev merged commit 3f77b34 into apache:master Oct 23, 2026
80 checks passed
hughhhh added a commit that referenced this pull request Oct 23, 2026
…migration

The popover test added in #44843 asserted that a config stored under the
display alias `Time` is ignored. #44955 deliberately reads legacy
`Time`-keyed configs and migrates them onto `__timestamp`, so the
assertion is stale. Assert the intended behaviour instead: the legacy
config is read back and an edit is saved under `__timestamp`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 34d5617)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Explore: column config for aliased columns (e.g. Time) is saved under the display alias instead of the real column key

5 participants