fix(dataset-editor): drop null warning_markdown from extra JSON serialisation

[mikebridge]

SUMMARY

Every Dataset Editor save silently rewrote every column's and metric's extra JSON-string field from \"{}\" to '{\"warning_markdown\":null}', even when no one touched any warning-related setting. The two forms are functionally equivalent for downstream consumers, so the bug has been latent in production for a long time. It became visible during the entity-versioning work, where the diff engine surfaced N+1 spurious change records per save — one per existing column whose extra got rewritten, plus the actual user-intended change.

Root cause

1. TableColumn.data (superset/connectors/sqla/models.py:1075-1094) serialises every column with warning_markdown exposed at the top level, derived from CertificationMixin.warning_markdown. That property parses extra JSON and returns Python None when the key is absent — which serialises to JSON null.
2. The API response delivers column.warning_markdown: null to the frontend.
3. buildExtraJsonObject (in DatasourceModal/index.tsx) re-serialises that null back into extra via JSON.stringify({ ..., warning_markdown: item?.warning_markdown }). JSON.stringify keeps explicit null; only undefined is dropped.
4. Save POSTs the rewritten extra; the backend stores it; the next load round-trips identically. The rewrite is permanent.

Fix

One expression change in buildExtraJsonObject: coerce empty/null warning_markdown to undefined before serialisation.

-    warning_markdown: item?.warning_markdown,
+    warning_markdown: item?.warning_markdown || undefined,

JSON.stringify then drops the key entirely when the value is empty/null. Matches the existing UI semantic where warning_markdown && <WarningIconWithTooltip /> already treats empty/null as absent. The fix applies uniformly to columns and metrics — both flow through the same helper.

The helper is also now exported (was module-local) so the new unit tests can call it directly.

BEFORE/AFTER

Pre-fix on the entity-versioning branch, adding one calculated column to a dataset with 8 existing columns produces 9 records in version_changes:

8 records: kind=column, path=[\"columns\",<name>], extra: \"{}\" → '{\"warning_markdown\":null}'
1 record:  kind=column, path=[\"columns\",\"num_new_york\"], from_value: null, to_value: <new column>

After this fix on the same branch, the same edit produces exactly 1 record — only the actual added column.

TESTING INSTRUCTIONS

1. Open the Dataset Editor on any dataset with several columns (e.g., `birth_names`).

2. Settings tab → edit Description → Save.

3. Query Postgres directly:

   SELECT column_name, extra FROM table_columns
   WHERE table_id = (SELECT id FROM tables WHERE table_name = 'birth_names');

   Expected: every column's `extra` is unchanged from before the save (still `'{}'` for columns that started that way). Pre-fix, every column's `extra` would have been rewritten to `'{"warning_markdown":null}'`.

4. Regression check — warning roundtrip: Metrics tab → expand a metric → set "⚠ test" in the Warning field → Save → reopen → confirm warning icon appears. Then expand the metric → clear the field → Save → reopen → confirm warning icon is gone.

5. Run the unit tests:

   cd superset-frontend && npm run test -- DatasourceModal
   # Test Suites: 3 passed, 3 total   Tests: 22 passed, 22 total (17 existing + 5 new)
   

Existing data healing

Rows whose `extra` is currently `'{"warning_markdown":null}'` (pre-fix noise from previous saves) heal naturally on their next save — the helper serialises them to `'{}'`. No data migration needed.

Out of scope

• Backend serialisation cleanup (`TableColumn.data` exposing `warning_markdown: null` instead of omitting absent keys). The frontend fix is necessary regardless and sufficient on its own; backend change has bigger API-contract implications.
• Lifting `warning_markdown` and `certification` fields out of `extra` JSON into proper columns. That's a schema-evolution project on its own.

ADDITIONAL INFORMATION

• Has associated issue: internal ticket sc-104972
• Required feature flags:
• Changes UI (behaviourally — the saved `extra` JSON differs by one omitted key when no warning is set)
• Includes DB Migration
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	6f0345f
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/69f1190383caed000860bad5
😎 Deploy Preview	https://deploy-preview-39706--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

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

[sfirke]

Not a part of the codebase I'm familiar with at all - but this seems like a clear improvement to me! Thanks for explaining in such detail.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.49%. Comparing base (549aff7) to head (40c2592).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #39706   +/-   ##
=======================================
  Coverage   64.49%   64.49%           
=======================================
  Files        2566     2566           
  Lines      133972   133973    +1     
  Branches    31124    31125    +1     
=======================================
+ Hits        86403    86404    +1     
  Misses      46074    46074           
  Partials     1495     1495           

Flag	Coverage Δ
javascript	66.59% <100.00%> (+<0.01%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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]

Code Review Agent Run #2f2784

Actionable Suggestions - 0

Review Details

• Files reviewed - 2 · Commit Range: 6f0345f..6f0345f

  • superset-frontend/src/components/Datasource/DatasourceModal/DatasourceModal.test.tsx
  • superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

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

• /review - 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[bito-code-review]

The current code sets warning_markdown to item?.warning_markdown || undefined, which omits the field in the JSON output when the value is falsy (null, empty string, etc.). Conditionally omitting the field entirely if falsy would produce the same result, as JSON.stringify excludes undefined properties.

[bito-code-review]

Code Review Agent Run #5d7feb

Actionable Suggestions - 0

Additional Suggestions - 1

• superset-frontend/src/components/Datasource/DatasourceModal/index.tsx - 1

  • Missing return type annotation · Line 74-76
    The buildExtraJsonObject function lacks an explicit return type hint, which violates the repository's type safety standards. Add `: string` to the function signature to improve consistency and enable better static type checking.
    Code suggestion

     @@ -74,3 +74,3 @@
    - export function buildExtraJsonObject(
    -   item: DatasetObject['metrics'][0] | DatasetObject['columns'][0],
    - ) {
    + export function buildExtraJsonObject(
    +   item: DatasetObject['metrics'][0] | DatasetObject['columns'][0],
    + ): string {

Review Details

• Files reviewed - 2 · Commit Range: 40c2592..40c2592

  • superset-frontend/src/components/Datasource/DatasourceModal/DatasourceModal.test.tsx
  • superset-frontend/src/components/Datasource/DatasourceModal/index.tsx
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

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

• /review - 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[richardfogaca]

LGTM 💯