feat: Adds chart IDs option to migrate-viz

[michael-s-molina]

SUMMARY

Adds the optional chart_id parameter to the migrate-viz command to allow upgrading/downgrading a specific chart.

Usage: superset migrate-viz upgrade [OPTIONS]

  Upgrade a viz to the latest version.

Options:
  -t, --viz_type TEXT  The viz type to upgrade: area, bar, bubble, dist_bar,
                       dual_line, heatmap, histogram, line, pivot_table,
                       sunburst, treemap  [required]
  --id INTEGER         The chart ID to upgrade. It can set set multiple times.
  --help               Show this message and exit.

Usage: superset migrate-viz downgrade [OPTIONS]

  Downgrade a viz to the previous version.

Options:
  -t, --viz_type TEXT  The viz type to upgrade: area, bar, bubble, dist_bar,
                       dual_line, heatmap, histogram, line, pivot_table,
                       sunburst, treemap  [required]
  --id INTEGER         The chart ID to downgrade. It can set set multiple times.
  --help               Show this message and exit.

TESTING INSTRUCTIONS

Check that you can upgrade/downgrade a specific chart using the -id option.

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

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.

Project coverage is 70.34%. Comparing base (76d897e) to head (14f28e4).
Report is 369 commits behind head on master.

Files	Patch %	Lines
superset/cli/viz_migrations.py	0.00%	29 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #29361      +/-   ##
==========================================
+ Coverage   60.48%   70.34%   +9.85%     
==========================================
  Files        1931     1961      +30     
  Lines       76236    78477    +2241     
  Branches     8568     8961     +393     
==========================================
+ Hits        46114    55204    +9090     
+ Misses      28017    21076    -6941     
- Partials     2105     2197      +92     

Flag	Coverage Δ
hive	48.87% <0.00%> (-0.29%)	⬇️
javascript	58.02% <ø> (+0.31%)	⬆️
mysql	77.25% <0.00%> (?)
postgres	77.35% <0.00%> (?)
presto	53.47% <0.00%> (-0.33%)	⬇️
python	83.70% <0.00%> (+20.21%)	⬆️
sqlite	76.81% <0.00%> (?)
unit	59.16% <0.00%> (+1.54%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[giftig]

Two small functional suggestions:

1. Looks like it's currently either one chart or all charts of a type, maybe it'd make sense to accept a list of chart IDs instead of just a single one. You could either use a comma separated list or a click option to let you specify the parameter multiple times, to do that.
2. Should visualisation type be optional if you're specifying id(s)? Currently you'd have to specify it and it'd have to match the vis type of the chart ID.

[john-bodley]

Should this be something which can be specified multiple times? Also I wonder if --chart would be suffice, i.e., it's somewhat implicit that you would be specifying the chart ID.

[giftig]

Actually given the context (migrating a visualisation) maybe --id would make more sense and be more in line with -t for viz type.

[michael-s-molina]

I updated the description with:

--ids TEXT           A comma separated list of chart IDs to downgrade

[michael-s-molina]

Two small functional suggestions:

Looks like it's currently either one chart or all charts of a type, maybe it'd make sense to accept a list of chart IDs instead of just a single one. You could either use a comma separated list or a click option to let you specify the parameter multiple times, to do that.

Should visualisation type be optional if you're specifying id(s)? Currently you'd have to specify it and it'd have to match the vis type of the chart ID.

Great suggestions. I did both.

[giftig]

LGTM

[john-bodley]

Shouldn't this have two dashes rather than one? Also why not use the multiple options --id 1 --id 2 ... so that way you don't need to split a string and the types will be correct, i.e., the following will correctly fail during the validation phase:

--id 1 --id foo 

[michael-s-molina]

Change it to use double dashes 👍🏼

[michael-s-molina]

Change it to use a multi-value parameter --id