Skip to content

feat(time-pivot): migrate time_pivot chart to v1 chart data API - #41727

Merged
rusackas merged 2 commits into
remove-legacy-viz-pipelinefrom
legacy-viz-t2-time-pivot
Jul 3, 2026
Merged

feat(time-pivot): migrate time_pivot chart to v1 chart data API#41727
rusackas merged 2 commits into
remove-legacy-viz-pipelinefrom
legacy-viz-t2-time-pivot

Conversation

@rusackas

@rusackas rusackas commented Jul 3, 2026

Copy link
Copy Markdown
Member

SUMMARY

Tier 2 of #41714 (targets remove-legacy-viz-pipeline): migrate the nvd3 Time-series Period Pivot (time_pivot) off explore_json onto /api/v1/chart/data.

  • New TimePivot/buildQuery.ts mirroring the legacy NVD3TimePivotViz.query_obj: timeseries query with the single metric.
  • The get_data reshape is ported to transformData: timestamps are bucketed into periods with a pandas-compatible offset.rollback(normalize=True) implementation covering the frequency control's aliases (AS/A, MS/M, W/W-XXX incl. multiplier prefixes like 52W-MON, D, H, T; freeform rules fall back to day boundaries), periods are dense-ranked most-recent-first (current, -1, -2, …), points are shifted onto the latest period's time axis, and each period becomes a series carrying the legacy rank/perc metadata.
  • A TimePivot-specific transformProps reshapes v1 records and then delegates to the shared nvd3 transformProps, so legacy payloads pass through untouched.
  • useLegacyApi: true removed (and the "Legacy" tag dropped).
  • No DB migration: viz_type unchanged.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

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

TESTING INSTRUCTIONS

  • npm run test -- plugins/legacy-preset-chart-nvd3 — 10 new Jest tests (rollback per alias incl. month-end and multiplier-prefix weeks, period pivot with rank/perc, axis shifting, empty input); 47 total pass.
  • Manual: open a saved Time-series Period Pivot chart; network tab shows POST /api/v1/chart/data; period overlays identical.

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

Adds a buildQuery mirroring the legacy NVD3TimePivotViz.query_obj (a
timeseries query with the single metric) and ports the get_data reshape
into a transformData helper: timestamps are bucketed by a pandas-style
offset rollback (AS/A, MS/M, W and W-XXX, D, H, T aliases; multiplier
prefixes keep the same anchor), periods ranked most-recent-first as
current/-1/-2, points shifted onto the latest period's time axis, and
one series emitted per period with the legacy rank/perc metadata. A
TimePivot-specific transformProps reshapes v1 records before delegating
to the shared nvd3 transformProps; legacy payloads pass through.

No DB migration needed: viz_type is unchanged and legacy
granularity_sqla/time_range form data flows through extractExtras.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dosubot dosubot Bot added api:charts Related to the REST endpoints of charts change:frontend Requires changing the frontend viz:charts:timeseries Related to Timeseries 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.

Comment thread superset-frontend/plugins/legacy-preset-chart-nvd3/test/TimePivot.test.ts Outdated
@bito-code-review

Copy link
Copy Markdown
Contributor

The suggestion to replace the describe() wrapper with a direct test() case is valid for blocks containing only a single test. This simplifies the test structure by removing unnecessary nesting.

To resolve this, you can replace the describe block with a test call. For example, in superset-frontend/plugins/legacy-preset-chart-nvd3/test/TimePivot.test.ts:

test('builds a timeseries query with the single metric', () => {
  const formData: QueryFormData = {
    datasource: '5__table',
    granularity_sqla: 'ds',
    time_range: 'Last quarter',
    viz_type: 'time_pivot',
    metric: 'sum__num',
    freq: 'W-MON',
  };
  const [query] = buildQuery(formData).queries;
  expect(query.metrics).toEqual(['sum__num']);
  expect(query.is_timeseries).toBe(true);
});

Would you like me to check the rest of the comments on this PR and implement fixes for them as well?

superset-frontend/plugins/legacy-preset-chart-nvd3/test/TimePivot.test.ts

test('builds a timeseries query with the single metric', () => {
  const formData: QueryFormData = {
    datasource: '5__table',
    granularity_sqla: 'ds',
    time_range: 'Last quarter',
    viz_type: 'time_pivot',
    metric: 'sum__num',
    freq: 'W-MON',
  };
  const [query] = buildQuery(formData).queries;
  expect(query.metrics).toEqual(['sum__num']);
  expect(query.is_timeseries).toBe(true);
});

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.78947% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.09%. Comparing base (e318607) to head (ca4764a).
⚠️ Report is 8 commits behind head on remove-legacy-viz-pipeline.

Files with missing lines Patch % Lines
...y-preset-chart-nvd3/src/TimePivot/transformData.ts 81.92% 15 Missing ⚠️
...-preset-chart-nvd3/src/TimePivot/transformProps.ts 0.00% 7 Missing ⚠️
...ns/legacy-preset-chart-nvd3/src/TimePivot/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                     @@
##           remove-legacy-viz-pipeline   #41727   +/-   ##
===========================================================
  Coverage                       64.08%   64.09%           
===========================================================
  Files                            2687     2690    +3     
  Lines                          148467   148562   +95     
  Branches                        34262    34292   +30     
===========================================================
+ Hits                            95147    95220   +73     
- Misses                          51558    51580   +22     
  Partials                         1762     1762           
Flag Coverage Δ
javascript 69.44% <75.78%> (+<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.

…ribe

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rusackas
rusackas merged commit 2a73fbd into remove-legacy-viz-pipeline Jul 3, 2026
57 checks passed
@rusackas
rusackas deleted the legacy-viz-t2-time-pivot branch July 3, 2026 06:42
rusackas added a commit that referenced this pull request Jul 7, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 7, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
@dosubot dosubot Bot mentioned this pull request Jul 8, 2026
3 tasks
rusackas added a commit that referenced this pull request Jul 8, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 10, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 13, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 14, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 15, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 15, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 16, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 17, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 20, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 20, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 20, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 21, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 21, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 23, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
rusackas added a commit that referenced this pull request Jul 27, 2026
Co-authored-by: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api:charts Related to the REST endpoints of charts change:frontend Requires changing the frontend plugins size/L viz:charts:timeseries Related to Timeseries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants