Skip to content

fix(big-number): respect extra_form_data.time_compare in Big Number with Time Comparison#41342

Merged
EnxDev merged 1 commit into
masterfrom
enxdev/fix/big-number-respect-extra-form-data-time-compare
Jun 23, 2026
Merged

fix(big-number): respect extra_form_data.time_compare in Big Number with Time Comparison#41342
EnxDev merged 1 commit into
masterfrom
enxdev/fix/big-number-respect-extra-form-data-time-compare

Conversation

@EnxDev

@EnxDev EnxDev commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

SUMMARY

Big Number with Time Comparison (pop_kpi) ignores extra_form_data.time_compare.
When that override is set, the chart still computes its comparison from its own time_compare control, so the displayed comparison value, delta, and "Data for …" tooltip don't reflect the override.

Fix

Read extra_form_data.time_compare and let it drive the comparison, mirroring how the Table chart already handles it (#33947):

  • buildQuery.ts — when extra_form_data.time_compare is present, use it as
    time_offsets (and request offsets in that case) so the backend computes the
    comparison against the overridden range.
  • transformProps.ts — prefer extra_form_data.time_compare when selecting the
    comparison column and computing the tooltip shift.
  • Added a buildQuery test covering the override.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

With extra_form_data.time_compare set, the Big Number's comparison value, delta, and percent now match the Table chart for the same data.

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

@dosubot dosubot Bot added the viz:charts:bignumber Related to BigNumber charts label Jun 23, 2026
@bito-code-review

bito-code-review Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Code Review Agent Run #320903

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/transformProps.ts - 1
    • Missing unit test coverage · Line 115-118
      The `timeComparison` override logic at line 115-118 lacks dedicated transformProps unit tests. While `buildQuery.test.ts` covers the query-level override, there are no tests verifying the transformProps correctly passes `dashboardTimeCompare` for rendering calculations. Consider adding transformProps tests for: dashboard filter sets `time_compare`, chart control is empty; both dashboard and chart set `time_compare` with dashboard priority; fallback to chart control.
Review Details
  • Files reviewed - 3 · Commit Range: ba02e38..ba02e38
    • superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/buildQuery.ts
    • superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/BigNumber/BigNumberPeriodOverPeriod/buildQuery.test.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ 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 Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.37%. Comparing base (92109f0) to head (ba02e38).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...Number/BigNumberPeriodOverPeriod/transformProps.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #41342   +/-   ##
=======================================
  Coverage   64.36%   64.37%           
=======================================
  Files        2653     2653           
  Lines      144965   144973    +8     
  Branches    33437    33444    +7     
=======================================
+ Hits        93305    93322   +17     
+ Misses      49974    49965    -9     
  Partials     1686     1686           
Flag Coverage Δ
javascript 68.63% <75.00%> (+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.

@EnxDev EnxDev merged commit 3ff90bd into master Jun 23, 2026
72 checks passed
@EnxDev EnxDev deleted the enxdev/fix/big-number-respect-extra-form-data-time-compare branch June 23, 2026 15:05
shantanukhond pushed a commit to shantanukhond/superset that referenced this pull request Jun 24, 2026
devin-ai-integration Bot pushed a commit to cognition-demo/superset that referenced this pull request Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugins size/M viz:charts:bignumber Related to BigNumber charts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants