mikebridge opened a new pull request, #43402:
URL: https://github.com/apache/superset/pull/43402
### SUMMARY
Archiving (or deleting) a chart that an alert or report uses is correctly
blocked server-side, but the user never learned why: the single-chart delete
handler discarded the 422 body, and the Archive modal gave no warning before
the dead-end confirm. Found in QA of the soft-delete feature (TC-081).
Three thin slices, no schema or API-surface changes:
1. **Surface the guard's message** — `handleChartDelete` now routes the
rejection through `createErrorHandler` into the failure toast (mirroring the
dashboard handler three functions down, which already did this). The shared
handler also serves the card view.
2. **Warn before confirm** — the ChartList row Archive action fetches the
chart's alerts/reports (existing report list API, `chart_id` search column — no
new endpoint) and opens a managed `DeleteModal` listing them by name and type,
capped at 10 with an overflow count: the same fetch-then-open pattern the
dataset delete modal uses. Every enumeration failure degrades to the unchanged
modal (`ALERT_REPORTS` off gates the whole report API with a 404 while the
guard still fires, and `ReportScheduleFilter` can hide reports the unfiltered
guard blocks on) — the list is advisory; the confirm-time guard stays
authoritative, so the Archive button is never disabled. With `ALERT_REPORTS`
off the pre-flight is skipped entirely and the modal opens synchronously. A
monotonic request token prevents a slower response from an earlier click
swapping the open modal to a different chart.
3. **Explain the block** — the guard message in `DeleteChartCommand` now
groups blocking reports per chart with deterministic ordering (chart name asc
with id tie-break, report names asc — the DAO query has no ORDER BY) and
carries the remedy: multi-id commands produce `Chart "Sales" is used by alerts
or reports: Weekly report, Q3 alert. Detach or delete them first.`; single-id
commands drop the chart prefix since the toast already names the chart. Three
separate gettext templates (no fragment assembly), coerced from lazy_gettext at
build time. HTTP 422, the exception class, and the `{"message"}` envelope are
unchanged, so this is message-content only — no `UPDATING.md` entry.
Bulk semantics are deliberately untouched (all-or-nothing, as today); this
PR only upgrades the feedback. The card view's own modal inconsistency (missing
`recoverable` prop, delete-flavored copy under `SOFT_DELETE`) is pre-existing
and split to a follow-up ticket rather than smuggled in here.
Docs: the Recently Archived page now notes the archive-time block.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before**: confirming Archive on a chart with an attached report showed
only a generic "There was an issue archiving: <name>" toast (the 422 body was
discarded), and the modal gave no warning.
**After**: the modal lists the blocking alerts/reports before confirm, and a
blocked attempt toasts the names plus the remedy.
_Screenshots to follow in a comment (captured from a live environment)._
### TESTING INSTRUCTIONS
1. Enable `SOFT_DELETE` and `ALERT_REPORTS`. Create a chart and attach a
Report to it (Alerts & Reports → + Report).
2. Charts list → row kebab → Archive: the modal lists the report (name +
type) under "Associated alerts and reports". Confirm anyway → danger toast:
`This chart is used by alerts or reports: <name>. Detach or delete them first.`
The chart stays live.
3. A chart with no reports shows today's modal unchanged. With
`ALERT_REPORTS` off, the modal is unchanged (no request fired) but a blocked
confirm still yields the named toast.
4. Bulk-select a blocked + a clean chart → Archive: toast groups by chart
name (`Chart "X" is used by alerts or reports: …`); nothing is archived (batch
semantics unchanged).
5. With `SOFT_DELETE` off, the row delete confirm keeps the type-DELETE
gate, and blocked deletes get the same named toast.
Automated: new unit tests for the message builder (grouping, ordering,
single-vs-multi phrasing), a toast-path test that fails without the handler
fix, ChartList modal tests (dependents render, empty/404 degrade, overflow
line, fresh fetch per open, both flag-off paths), and updated integration
assertions (`api_tests.py`, `soft_delete_tests.py`).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `SOFT_DELETE` + `ALERT_REPORTS` for the full
experience; the toast fix applies regardless
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] 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
🤖 Authored with the assistance of Claude (AI), directed and reviewed by
@mikebridge.
--
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]
