UI: Use react-query native error state for bulk action hooks#67284
Draft
pierrejeambrun wants to merge 1 commit into
Draft
UI: Use react-query native error state for bulk action hooks#67284pierrejeambrun wants to merge 1 commit into
pierrejeambrun wants to merge 1 commit into
Conversation
boring-cyborg
Bot
added
area:airflow-ctl
area:API
1 task
Member
Author
|
Only the last commit is relevant. |
Both ``useBulkTaskInstances`` and ``useBulkDeleteDagRuns`` mirrored
the same anti-pattern: a ``useState<unknown>`` field, an ``onError``
callback that just forwarded to ``setError``, and a tiny helper that
grabbed ``errors[0]`` from the response body and re-shaped it into
``{body:{detail:...}}`` so ``ErrorAlert`` could render the first
per-entity failure. That hid every error past the first and
duplicated mutation state into React state for no reason.
Both hooks now return ``{ bulkAction, data, error, isPending, reset }``
straight from ``useMutation``:
- ``error`` covers HTTP-level failures (4xx/5xx, network).
- ``data.delete.errors`` / ``data.update.errors`` carries the
per-entity failures the backend returns on a 200 response (partial
success). Consumers render every entry, not just the first.
- ``reset`` replaces the consumer-side ``setError(undefined)`` calls.
``onSuccess`` still invalidates queries unconditionally, fires the
toaster + clears selection when ``success.length > 0``, and only
closes the dialog when ``errors.length === 0`` — partial-success
keeps the dialog open so the user can read what failed.
The three consumers (``BulkMarkTaskInstancesAsButton``,
``BulkDeleteTaskInstancesButton``, ``BulkDeleteDagRunsButton``) now
render network errors via ``ErrorAlert`` and per-entity errors as a
``Stack`` of ``Alert`` rows below it.
pierrejeambrun
force-pushed
the
fix/bulk-action-error-handling
branch
from
0f52d79 to
c0952b0
Compare
bbovenzi
reviewed
May 22, 2026
Comment on lines
+148
to
+155
| {actionErrors.length > 0 ? ( | ||
| <Stack gap={2} mt={3}> | ||
| {actionErrors.map((actionError, index) => ( | ||
| // eslint-disable-next-line react/no-array-index-key -- per-entity errors have no stable id | ||
| <Alert key={index} status="error" title={actionError.error} /> | ||
| ))} | ||
| </Stack> | ||
| ) : undefined} |
Contributor
There was a problem hiding this comment.
Let's make an ActionErrors component to reuse all this error logic.
bbovenzi
reviewed
May 22, 2026
Contributor
bbovenzi
left a comment
bbovenzi
left a comment
There was a problem hiding this comment.
Could you add a screenshot of the modal with many errors at once? Let's make sure the modal displays correctly
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
useBulkTaskInstancesanduseBulkDeleteDagRunsboth kept the same anti-pattern @bbovenzi flagged on #67095: auseState<unknown>for the error, anonErrorcallback that just forwarded the network error tosetError, and a tiny helper that grabbederrors[0]from the response body and re-shaped it into{body:{detail:...}}soErrorAlertcould render the first per-entity failure. That hid every error past the first and duplicated mutation state into React state for no reason.Both hooks now return
{ bulkAction, data, error, isPending, reset }straight fromuseMutation:errorcovers HTTP-level failures (4xx/5xx, network).data.delete.errors/data.update.errorscarries the per-entity failures the backend returns on a 200 response (partial success). Consumers render every entry, not just the first.resetreplaces the consumer-sidesetError(undefined)calls.onSuccessstill invalidates queries unconditionally, fires the toaster + clears selection whensuccess.length > 0, and only closes the dialog whenerrors.length === 0— partial-success keeps the dialog open so the user can read what failed.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.7) following the guidelines