fix(dashboard): clean up JSON formatting and contribution suffix in V…#40683
Conversation
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #a00a35
Actionable Suggestions - 1
-
superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx - 1
- CWE-6262: Missing test coverage for JSON parsing · Line 41-57
Review Details
-
Files reviewed - 1 · Commit Range:
da0e100..da0e100- superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.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
|
I'll examine this soon. A bit busy at work atm. |
|
@aminghadersohi Thank you for the feedback! I have updated the logic to use .slice() to strictly target the suffix, integrated the getMetricLabel core helper to handle simple metrics, and fixed the catch block per your suggestions. Pushed the updates! |
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
MEDIUM
useGridResultTable.tsx:58—' (contribution)'is a UI-facing string with no i18n wrapper. Other UI strings in Superset uset(). Suggestion:suffix = ` (${t('contribution')})`- No tests added for the new logic.
useGridColumnsnow has non-trivial branching (contribution-suffix stripping, JSON parse +getMetricLabelfallback). Test files exist atDataTablesPane/test/and should cover: plain column,__contributioncolumn, JSON-metric column, JSON-metric + contribution, non-JSON display name.
NIT
-
useGridResultTable.tsx:48—let cleanHeader = rawHeaderis dead code; the value is unconditionally overwritten at line 70. Remove it or useconst cleanHeader = \${cleaned}${suffix}`` after the try/catch. -
useGridResultTable.tsx:53,56—'__contribution'appears twice; extract to a named constant.
Code Review Agent Run #533ffcActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
|
@aminghadersohi addressed the comments: added tests for the column label hook, extracted the strings, and added i18n support. Tests are passing locally, ready for your review. |
Code Review Agent Run #67aaddActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
All prior review comments addressed — the suffix stripping, getMetricLabel usage, i18n, test coverage, and code cleanup all look correct. Waiting on CI to fully run (maintainer is approving the workflow now).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40683 +/- ##
=======================================
Coverage 64.07% 64.07%
=======================================
Files 2664 2664
Lines 143829 143840 +11
Branches 33083 33085 +2
=======================================
+ Hits 92160 92171 +11
Misses 50060 50060
Partials 1609 1609
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rabumaabraham
force-pushed
the
fix-table-headers
branch
from
0191942 to
bae49a6
Compare
|
@rusackas the branch has been successfully rebased and all review comments are addressed and approved. Could you please approve the workflows so the checks can finish up for merge? Thank you! |
Code Review Agent Run #ce1c61Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
rusackas
added
the
merge-if-green
|
Running CI, will merge if/when it passes. |
SUMMARY
This PR fixes a bug where the "View as Table" modal displays raw JSON strings (e.g.,
{"label": "..."}) and raw suffixes (e.g.,__contribution) in column headers instead of clean, human-readable labels.This builds directly on top of the previous autonomous attempt to resolve the custom-labeled metrics bug, ensuring data row values populate correctly without breaking AG Grid field matching while fully cleaning up the header names.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before (Raw JSON)
After (Clean Headers)
TESTING INSTRUCTIONS
...) on the chart and select View as table.undefined.ADDITIONAL INFORMATION