superset icon indicating copy to clipboard operation
superset copied to clipboard

fix(migrations): Fix the time comparison migration

Open Antonio-RiveroMartnez opened this issue 6 months ago • 1 comments

SUMMARY

After the initial version of the time comparison feature for Table and Big Number we introduced a migration to upgrade old charts to the new comparison controls, however, during that upgrade a table that never had comparison in it was wrongly showing Time Comparison Data.

This PR is fixing the migration so we can properly upgrade such old tables if needed, also we extend our tests to not only consider charts that have the enabled_time_comparison flag set to True.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE

Old version of Chart using Time Comparison once but then removing it from the controls: initial removed

The old Chart is upgraded with the previous version of the migration: upgrade

The chart start showing comparison data: wrong

AFTER

Old version of Chart using Time Comparison once but then removing it from the controls: v2_initial v2_no_comp

The old Chart is upgraded with the new version of the migration: v2_upgrade

The chart doesn't show comparison data: correct

TESTING INSTRUCTIONS

  1. Create a table chart with a version of the time comparison where Enable Time Comparison checkbox was used
  2. Run a comparison data (Mark the checkbox and select a time range)
  3. Unmark the checkbox and run the chart data again so your chart has no longer comparison info
  4. Use more updated version and run the migrations so your table is upgraded
  5. You must not see time comparison data in the table.

ADDITIONAL INFORMATION

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

Antonio-RiveroMartnez avatar Aug 27 '24 17:08 Antonio-RiveroMartnez