Skip to content

feat(parallel-coordinates): migrate para chart to v1 chart data API - #41716

Merged
rusackas merged 2 commits into
remove-legacy-viz-pipelinefrom
legacy-viz-t1-para
Jul 3, 2026
Merged

feat(parallel-coordinates): migrate para chart to v1 chart data API#41716
rusackas merged 2 commits into
remove-legacy-viz-pipelinefrom
legacy-viz-t1-para

Conversation

@rusackas

@rusackas rusackas commented Jul 3, 2026

Copy link
Copy Markdown
Member

SUMMARY

Tier 1 of #41714 (targets remove-legacy-viz-pipeline): migrate Parallel Coordinates (para) off explore_json onto /api/v1/chart/data.

  • New buildQuery.ts mirroring the legacy ParallelCoordinatesViz.query_obj: series → groupby column, secondary (color) metric included via extractQueryFields' secondary_metric alias, sort metric appended to the select list, orderby applied when order_desc is set.
  • useLegacyApi: true removed (and the "Legacy" tag dropped from the viz gallery metadata).
  • transformProps unchanged — the legacy get_data() was a plain df.to_dict("records") pass-through, which is exactly what the v1 endpoint returns.
  • No DB migration needed: viz_type is unchanged, and legacy saved-chart form data (granularity_sqla, time_range) is handled by extractExtras, covered by a test.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

No visual change — same renderer, same data shape, different endpoint.

TESTING INSTRUCTIONS

  • npm run test -- plugins/legacy-plugin-chart-parallel-coordinates — 7 new buildQuery tests covering series/metrics extraction, secondary metric, sort-metric append/dedup, order_desc, adhoc sort metrics, and legacy granularity_sqla handling.
  • Manual: open any saved Parallel Coordinates chart; the network tab should show POST /api/v1/chart/data instead of /superset/explore_json/, with identical rendering.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
  • Introduces new feature or API
  • Removes existing feature or API

🤖 Generated with Claude Code

@dosubot dosubot Bot added api Related to the REST API change:frontend Requires changing the frontend viz:charts Namespace | Anything related to viz types labels Jul 3, 2026
@bito-code-review

bito-code-review Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Bito Automatic Review Skipped - Branch Excluded

Bito didn't auto-review because the source or target branch is excluded from automatic reviews.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the branch exclusion settings here, or contact your Bito workspace admin at evan@preset.io.

@github-actions github-actions Bot added plugins and removed api Related to the REST API labels Jul 3, 2026
@bito-code-review

Copy link
Copy Markdown
Contributor

The flagged issue is correct. The current implementation of buildQuery in superset-frontend/plugins/legacy-plugin-chart-parallel-coordinates/src/buildQuery.ts spreads ...baseQueryObject without explicitly overriding orderby, which allows residual sorting from form_data (like order_by_cols) to persist. To resolve this, you should explicitly set orderby to undefined when it is empty, ensuring only the intended sorting rules are applied.

Here is the corrected implementation for the return block:

    return [
      {
        ...baseQueryObject,
        metrics,
        orderby: orderby.length > 0 ? orderby : undefined,
      },
    ];

I have checked the PR comments, and there are no other pending review comments to address. Would you like me to proceed with any other changes?

superset-frontend/plugins/legacy-plugin-chart-parallel-coordinates/src/buildQuery.ts

return [
      {
        ...baseQueryObject,
        metrics,
        orderby: orderby.length > 0 ? orderby : undefined,
      },
    ];

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.62%. Comparing base (c6242a6) to head (0d50722).

Files with missing lines Patch % Lines
...acy-plugin-chart-parallel-coordinates/src/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                      Coverage Diff                       @@
##           remove-legacy-viz-pipeline   #41716      +/-   ##
==============================================================
- Coverage                       64.62%   64.62%   -0.01%     
==============================================================
  Files                            2684     2685       +1     
  Lines                          148431   148446      +15     
  Branches                        34249    34253       +4     
==============================================================
+ Hits                            95930    95938       +8     
- Misses                          50739    50746       +7     
  Partials                         1762     1762              
Flag Coverage Δ
javascript 69.39% <93.33%> (-0.01%) ⬇️

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.

claude added 2 commits July 2, 2026 22:02
Adds a buildQuery that mirrors the legacy ParallelCoordinatesViz
query_obj: series becomes the groupby column, the secondary (color)
metric rides along via extractQueryFields' secondary_metric alias,
and the sort metric is appended to the select list with ordering
applied when order_desc is set. The legacy get_data was a plain
records pass-through, so transformProps is unchanged.

Saved charts keep working without a DB migration: viz_type is
unchanged and legacy granularity_sqla/time_range form data flows
through extractExtras.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The legacy pipeline never honored order_by_cols for this chart, so the
migrated buildQuery owns the orderby field explicitly rather than
inheriting whatever extractQueryFields pulled from residual form data.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rusackas
rusackas force-pushed the legacy-viz-t1-para branch from da83575 to 0d50722 Compare July 3, 2026 05:02
@rusackas
rusackas merged commit 5b84293 into remove-legacy-viz-pipeline Jul 3, 2026
57 checks passed
@rusackas
rusackas deleted the legacy-viz-t1-para branch July 3, 2026 05:20
rusackas added a commit that referenced this pull request Jul 7, 2026
rusackas added a commit that referenced this pull request Jul 7, 2026
rusackas added a commit that referenced this pull request Jul 8, 2026
rusackas added a commit that referenced this pull request Jul 10, 2026
rusackas added a commit that referenced this pull request Jul 13, 2026
rusackas added a commit that referenced this pull request Jul 14, 2026
rusackas added a commit that referenced this pull request Jul 15, 2026
rusackas added a commit that referenced this pull request Jul 15, 2026
rusackas added a commit that referenced this pull request Jul 16, 2026
rusackas added a commit that referenced this pull request Jul 17, 2026
rusackas added a commit that referenced this pull request Jul 20, 2026
rusackas added a commit that referenced this pull request Jul 20, 2026
rusackas added a commit that referenced this pull request Jul 20, 2026
rusackas added a commit that referenced this pull request Jul 21, 2026
rusackas added a commit that referenced this pull request Jul 21, 2026
rusackas added a commit that referenced this pull request Jul 23, 2026
rusackas added a commit that referenced this pull request Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend plugins size/L viz:charts Namespace | Anything related to viz types

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants