michael-s-molina opened a new pull request, #42088: URL: https://github.com/apache/superset/pull/42088
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY Closes the functional gap between Table (v1, `plugin-chart-table`) and Table V2 (AG Grid, `plugin-chart-ag-grid-table`), and adds an opt-in migration path from v1 to v2 via the existing `superset migrate_viz` CLI. Table V2 has been available behind the `AG_GRID_TABLE_ENABLED` flag but was missing several v1 capabilities, which made it unsafe to migrate existing `table` charts to it. This PR: 1. Closes the v2 feature gaps that were blocking migration. 2. Adds a `MigrateTableChart` processor + CLI wiring so organizations can migrate `table` → `ag-grid-table` on their own schedule. 3. Does **not** add a database (Alembic) migration — see "Why no DB migration" below. #### Feature gaps closed in Table V2 - **Drill to Detail / Drill by** — right-clicking a cell did nothing. Now works the same as in Table V1. - **Percent-metric calculation mode** — v1 lets users choose whether percent-metrics are calculated against the row-limited subset or the full dataset. V2 was missing this option entirely. - **AUTO currency formatting** — columns configured to auto-detect their currency silently rendered unformatted in V2. - **"Entire row" conditional formatting** — applying a conditional format to a whole row wasn't available, and wouldn't have rendered correctly even if set via a migrated chart. - **"Export Current View"** — lets a user export just the filtered/sorted rows they're currently looking at, instead of the entire dataset. This option was available in V1 but didn't show up at all for V2 charts. Two gaps were evaluated and intentionally left as-is: - **Matrixify** (matrix-of-charts layout) isn't available in V2 — but it's also disabled for V1 Table today, so this isn't a new gap. The migration simply drops that configuration rather than failing. - **Column ordering** can differ after migration (V2 groups metrics after dimensions) — a cosmetic difference, not a functional one. V2's sort order is intentional: it prevents a bug where dimensions added on the fly (e.g. via dashboard cross-filtering) would render out of place among the other dimension columns. Matching V1's exact order would reintroduce that bug, so we kept V2's current behavior. ### MIGRATION SCRIPT AND CLI COMMAND Migrating `table` charts to `ag-grid-table` is opt-in and runs through the existing `migrate_viz` CLI command: ```bash # migrate every table chart superset migrate_viz upgrade --viz_type table # migrate one chart, e.g. to spot-check before a wider rollout superset migrate_viz upgrade --id 123 # revert if something looks wrong superset migrate_viz downgrade --viz_type table ``` If an individual chart fails to migrate, it's skipped and logged rather than aborting the whole run — so one bad chart won't block migrating the rest. ### WHY NO DATABASE MIGRATION This PR deliberately does **not** add an Alembic revision that runs `MigrateTableChart.upgrade()` automatically on `db upgrade`, unlike prior v1→v2 migrations (Pivot Table, Treemap, Area, Sunburst) which did ship as automatic DB migrations. **Table V2 is still `IN DEVELOPMENT`** and gated behind `AG_GRID_TABLE_ENABLED`. Forcing every `table` slice in every Superset install to convert on their next `db upgrade` would be premature for a chart type still marked in-development. Instead, this ships the same underlying `MigrateTableChart.upgrade()`/`.downgrade()` logic behind the manual CLI command, so any organization that wants to migrate proactively can do so on their own schedule, spot-check a single chart via `--id`, and revert cleanly via `downgrade` if something looks wrong — without every existing Superset install being force-migrated on upgrade. The plan is to add the automatic Alembic migration later, at a breaking-change window, once: (a) Table V2 graduates out of `IN DEVELOPMENT`, and (b) v1's plugin package is being removed in that same release — so the automatic sweep lands together with v1's removal rather than as a surprise in an earlier minor version. ### TESTING INSTRUCTIONS **Automated tests** 1. `pytest tests/unit_tests/migrations/viz/table_v1_v2_test.py` — unit tests for the migration (basic migration, raw mode, page length, percent-metric calculation, conditional formatting, Matrixify handling, currency config). 2. `npx jest plugins/plugin-chart-ag-grid-table` — plugin test suite, including new drill-to-detail coverage. **Manually verify the Table V2 fixes** 1. Enable the `AG_GRID_TABLE_ENABLED` feature flag. 2. Create a Table V2 chart (or open an existing one) and check each of the following, comparing against a Table V1 chart with the same setting: - Right-click a cell → "Drill to detail" / "Drill by" should appear and work. - Set a percent metric and switch between row-limit vs. all-records calculation mode. - Configure a column to auto-detect its currency and confirm it renders formatted. - Set a conditional format to apply to the entire row and confirm it highlights the whole row. - Filter/sort the table, then use "Export Current View" from the chart menu and confirm it exports only what's currently visible. **Manually verify the migration** 1. Create or identify an existing chart with `viz_type: table`. 2. Run `superset migrate_viz upgrade --id <chart_id>` and confirm the chart now renders as Table V2 with the same data, columns, and settings. 3. Open the chart in Explore and confirm it looks/behaves as expected (sorting, totals, filters, etc. depending on what the original chart used). 4. Run `superset migrate_viz downgrade --id <chart_id>` and confirm the chart reverts to Table V1, identical to before migration. 5. If you have a `table` chart that uses Matrixify, migrate it and confirm it renders as a single flat table (this is expected — see "Feature gaps" above) rather than failing. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [x] Required feature flags: `AG_GRID_TABLE_ENABLED` (for the target viz type to render); migration CLI itself requires no flag - [x] Changes UI (Table V2: drill-to-detail context menu, Export Current View menu item, entire-row conditional formatting) - [ ] Includes DB Migration (deliberately omitted — see "Why no database migration" above) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [x] Introduces new feature or API (`superset migrate_viz upgrade/downgrade --viz_type table`) - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
