Skip to content

fix(dashboard): preserve certification fields when Certification panel is closed - #42957

Open
chkang83 wants to merge 2 commits into
apache:masterfrom
chkang83:fix/dashboard-preserve-certification-on-save
Open

fix(dashboard): preserve certification fields when Certification panel is closed#42957
chkang83 wants to merge 2 commits into
apache:masterfrom
chkang83:fix/dashboard-preserve-certification-on-save

Conversation

@chkang83

@chkang83 chkang83 commented Aug 10, 2026

Copy link
Copy Markdown

SUMMARY

Saving Dashboard Properties while the Certification accordion panel was never opened cleared certified_by / certification_details.
The Properties modal uses an accordion Collapse, so Certification FormItems are unregistered until that panel mounts. form.getFieldsValue() only returns registered fields, so those values became undefined and the PUT payload wrote null.
onFinish now calls form.getFieldsValue(true) to read store values for unregistered fields (already set via setFieldsValue on load). Added a regression test that saves without opening Certification and asserts the certification fields are preserved.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

2026-08-10.14.20.56.mp4
2026-08-10.14.24.11.mp4

TESTING INSTRUCTIONS

  1. Open a dashboard that already has Certified by / Certification details set.
  2. Open Dashboard properties.
  3. Do not expand the Certification section.
  4. Click Save.
  5. Re-open Dashboard properties → Certification and confirm the previous certification values are still present.
    Automated:
cd superset-frontend
npm test -- --testPathPatterns='dashboard/components/PropertiesModal/PropertiesModal.test' --testNamePattern='preserves certification fields'

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@chkang83
fix(dashboard): preserve certification fields when Certification pane…
a0411ae 
…l is closed

Accordion Collapse leaves certification FormItems unregistered until opened;
getFieldsValue(true) reads store values so save no longer clears certified_by.
@dosubot dosubot Bot added change:frontend Requires changing the frontend dashboard:properties Related to the properties of the Dashboard labels Aug 10, 2026
@bito-code-review

bito-code-review Bot commented Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review Agent Run #dfd89b

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: a0411ae..a0411ae
    • superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx
    • superset-frontend/src/dashboard/components/PropertiesModal/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

AI Code Review powered by Bito Logo

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.41%. Comparing base (f8925cc) to head (a0411ae).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #42957   +/-   ##
=======================================
  Coverage   66.41%   66.41%           
=======================================
  Files        2858     2858           
  Lines      161446   161446           
  Branches    37190    37190           
=======================================
  Hits       107222   107222           
  Misses      52187    52187           
  Partials     2037     2037
Flag Coverage Δ
javascript 73.27% <100.00%> (ø)

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.

@chkang83
fix(dashboard): narrow put body type in PropertiesModal certification…
56dca96 
… test

Assert the request body is a string before JSON.parse so TypeScript type-checking passes in CI.
@bito-code-review

bito-code-review Bot commented Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review Agent Run #471f62

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: a0411ae..56dca96
    • superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.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

AI Code Review powered by Bito Logo

@rusackas
rusackas requested review from aminghadersohi and a lite review from Copilot August 10, 2026 19:51
Copilot AI reviewed Aug 10, 2026
View reviewed changes

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.

Pull request overview

This PR fixes a regression in the dashboard Properties modal where saving without expanding the Certification accordion panel would unintentionally clear certified_by / certification_details, due to Ant Design Form only returning currently-registered (mounted) fields.

Changes:

  • Update the Properties modal submit handler to use form.getFieldsValue(true) so unmounted accordion fields retain their previously loaded values.
  • Add a regression test that saves without opening the Certification section and asserts certification fields are preserved in both onSubmit props and the PUT payload.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
superset-frontend/src/dashboard/components/PropertiesModal/index.tsx Reads form values using getFieldsValue(true) during save so unmounted Certification fields aren’t dropped from the payload.
superset-frontend/src/dashboard/components/PropertiesModal/PropertiesModal.test.tsx Adds a regression test covering the “save without opening Certification” scenario and verifies certification fields are preserved.

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

@rusackas
rusackas self-requested a review August 10, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@aminghadersohi aminghadersohi Awaiting requested review from aminghadersohi
@rusackas rusackas Awaiting requested review from rusackas

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

change:frontend Requires changing the frontend dashboard:properties Related to the properties of the Dashboard size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@chkang83