feat(aci): show test notification errors in UI #104708
Conversation
| setErrors({ | ||
| ...errors, | ||
| ...error?.responseJSON?.detail, | ||
| }); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| setErrors({ | ||
| ...errors, | ||
| ...error?.responseJSON?.detail, | ||
| }); |
There was a problem hiding this comment.
Bug: Stale errors closure loses validation errors in catch block
The catch block uses the errors value from the callback closure, but setErrors is called earlier at line 141 which updates the state. If future code changes allow reaching the catch block with non-empty actionErrors, the catch block's setErrors({...errors, ...}) would use the stale errors from the closure rather than the updated state, potentially overwriting the validation errors that were just set.
| return stripActionFields(action); | ||
| }) | ||
| ); | ||
| } catch (error: any) { |
There was a problem hiding this comment.
Instead of catching, the more common usage is to send onError to useSendTestNotification(). The hook can supply the expected error type so you don't need to type any here.
A couple other things: you don't need mutateAsync here, you can just use mutate this way. Oh, and I think we should be using isPending from useSendTestNotification to disable the button while the request is in flight.
Surfacing test notification errors using the alert builder error context that we already have for form submission errors