Repository navigation
fix(Message): show error notification when pinning or unpinning fails - #3359
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Contributor
|
Size Change: +581 B (+0.05%) Total Size: 1.18 MB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-v14 #3359 +/- ##
===============================================
- Coverage 86.84% 86.60% -0.25%
===============================================
Files 526 541 +15
Lines 16699 17617 +918
Branches 5251 5431 +180
===============================================
+ Hits 14503 15257 +754
- Misses 2196 2360 +164 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
oliverlaz
added a commit
that referenced
this pull request
Oct 30, 2026
…3360) Ticket: https://linear.app/stream/issue/REACT-1264 ### 🎯 Goal v15 counterpart of #3359. A failed pin, unpin, mute, unmute or delete showed a success notification ("Message pinned", "{{ user }} has been muted", "Message deleted"), e.g. when a `user` role without the pin permission in the `videocall` scope tries to pin. `usePinHandler`, `useMuteHandler` and `useDeleteHandler` caught the API error and only reported it through the optional `notify`, which `Message.tsx` never passes. So the handlers always resolved, and the default `MessageActions` published their success notification instead of the `*:failed` one. On `release-v14` only `usePinHandler` swallowed the error, which #3359 covers. ### 🛠 Implementation details - `usePinHandler`, `useMuteHandler` and `useDeleteHandler` now rethrow the request's error. The optional `notify` / `getErrorNotification` path is unchanged and still fires before the rethrow. `useMuteHandler` no longer returns early from its `catch` when `notify` is missing, which is the path `Message.tsx` uses. - `usePinHandler` rolls back only the pin fields, onto the message as currently stored, so an edit or reaction that arrived while the request was in flight is kept. If the pin state changed again in the meantime, the rollback leaves it alone. The rollback also runs before `notify`, so a throwing callback cannot leave the optimistic state behind. - `MessageContext.handlePin` documents that it rejects on failure. - The vite example's custom message UI catches `handlePin` / `handleDelete` rejections instead of leaking an unhandled rejection. - Tests: `usePinHandler.test.tsx` (new) covers pin/unpin success, rollback, concurrent updates, a newer pin state, and a throwing `notify`. The mute and delete tests that asserted "notify (and not throw)" now assert the rethrow, plus a case without `notify`. Two `Message.test.tsx` tests named "should throw when (un)muting a user fails" now actually assert it. Note for custom UIs: `handlePin`, `handleMute` and `handleDelete` from `MessageContext` now reject when the request fails instead of resolving silently. Not included: the handlers (plus flag and mark unread) still return early without a request when a parameter is missing, so the action shows success. That only affects custom messages and handlers and is left for a follow-up. ### 🎨 UI Changes No visual changes. A failed pin, unpin, mute, unmute or delete now shows the existing error notification instead of the success one. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Bug Fixes** * When pinning or unpinning fails, the message returns to its previous pin state without overwriting newer updates. The request error is also passed back to calling code, and applicable error notifications are still shown. * Mute, unmute, and delete failures now pass their errors back to calling code, including when notifications are not configured. When configured, error notifications are still sent. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This branch was successfully deployed
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
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.
Ticket: https://linear.app/stream/issue/REACT-1264
🎯 Goal
When the server rejects a pin (e.g. a
userrole without the pin permission in thevideocallscope), the UI still showed a "Message pinned" success notification. Unpin had the same problem.usePinHandlercaught the API error, rolled back the optimistic update, and swallowed it, sohandlePinalways resolved. The Pin action inMessageActionsrelies onhandlePinthrowing to show its existing "Error pinning message" / "Error removing message pin" notification, so that branch never ran.On
release-v14onlyusePinHandlerswallowed the error, so this PR covers REACT-1264 for v14. The v15 counterpart, which also fixes mute and delete, is #3360.🛠 Implementation details
usePinHandlernow rethrows the request's error after rolling back, for both pin and unpin. This matches sibling handlers such asuseMuteHandler, which let API errors propagate.channel.state(findMessage, which also covers thread replies). An edit or reaction that arrived while the request was in flight is kept instead of being overwritten by the pre-click snapshot. If the pin state changed again in the meantime, the rollback leaves it alone. The optimistic update also starts from the stored message.MessageContext.handlePin's JSDoc now says it pins or unpins and rejects with the request's error after rolling back.handlePinrejections instead of leaking an unhandled rejection.usePinHandler.test.tsxuses a realChannel, withupdateMessagewriting tochannel.statelikeChanneldoes, and checks the stored message: pin/unpin success and rollback, an edit made during the request surviving the rollback, and a newer pin state being kept.Note for custom UIs:
handlePinfromMessageContextnow rejects when the request fails instead of resolving silently. Custom message UIs calling it directly should handle the rejection.React Native is not affected — its
handleTogglePinMessageawaitsclient.pinMessagedirectly and already shows the error notification.🎨 UI Changes
No visual changes. A rejected pin/unpin now shows the existing error notification instead of the success one.