Skip to content

Commit cf6c8f9

Browse files
authored
fix(Message): show error notification when pinning or unpinning fails (#3359)
Ticket: https://linear.app/stream/issue/REACT-1264 ### 🎯 Goal When the server rejects a pin (e.g. a `user` role without the pin permission in the `videocall` scope), the UI still showed a "Message pinned" success notification. Unpin had the same problem. `usePinHandler` caught the API error, rolled back the optimistic update, and swallowed it, so `handlePin` always resolved. The Pin action in `MessageActions` relies on `handlePin` throwing to show its existing "Error pinning message" / "Error removing message pin" notification, so that branch never ran. On `release-v14` only `usePinHandler` swallowed the error, so this PR covers REACT-1264 for v14. The v15 counterpart, which also fixes mute and delete, is #3360. ### 🛠 Implementation details - `usePinHandler` now rethrows the request's error after rolling back, for both pin and unpin. This matches sibling handlers such as `useMuteHandler`, which let API errors propagate. - The rollback restores only the pin fields, onto the message as currently stored in `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. - The vite example's custom message UI catches `handlePin` rejections instead of leaking an unhandled rejection. - `usePinHandler.test.tsx` uses a real `Channel`, with `updateMessage` writing to `channel.state` like `Channel` does, 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: `handlePin` from `MessageContext` now 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 `handleTogglePinMessage` awaits `client.pinMessage` directly 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.
1 parent 1ad45b9 commit cf6c8f9

4 files changed

Lines changed: 254 additions & 22 deletions

File tree

‎examples/vite/src/CustomMessageUi/variants.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,16 @@ const CustomMessageUiActions = () => {
246246
return (
247247
<div className='custom-message-ui__actions'>
248248
<div className='custom-message-ui__actions-group'>
249-
<button onClick={handlePin} title={message.pinned ? 'Unpin' : 'Pin'}>
249+
<button
250+
onClick={async (event) => {
251+
try {
252+
await handlePin(event);
253+
} catch (error) {
254+
console.error('Failed to update the message pin', error);
255+
}
256+
}}
257+
title={message.pinned ? 'Unpin' : 'Pin'}
258+
>
250259
{message.pinned ? '📍' : '📌'}
251260
</button>
252261
<button onClick={() => handleDelete()} title='Delete'>
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
import React from 'react';
2+
import { act, renderHook } from '@testing-library/react';
3+
import { fromPartial } from '@total-typescript/shoehorn';
4+
5+
import { usePinHandler } from '../usePinHandler';
6+
7+
import { ChannelActionProvider } from '../../../../context/ChannelActionContext';
8+
import { ChannelStateProvider } from '../../../../context/ChannelStateContext';
9+
import { ChatProvider } from '../../../../context/ChatContext';
10+
import {
11+
generateMessage,
12+
generateUser,
13+
getTestClientWithUser,
14+
mockChannelActionContext,
15+
mockChannelStateContext,
16+
mockChatContext,
17+
} from '../../../../mock-builders';
18+
import type { Channel, LocalMessage, StreamChat } from 'stream-chat';
19+
20+
// `usePinHandler` updates the message optimistically through `updateMessage`, rolls back only
21+
// the pin fields when the request fails, and rethrows so callers (the Pin message action) can
22+
// show an error notification instead of a success one.
23+
24+
const alice = generateUser({ name: 'alice' });
25+
let client: StreamChat;
26+
let channel: Channel;
27+
// Writes to the channel state like `Channel`'s own `updateMessage`, so tests can read it back.
28+
const updateMessage = vi.fn((message: LocalMessage) =>
29+
channel.state.addMessageSorted(message, true),
30+
);
31+
const mouseEventMock = fromPartial<React.BaseSyntheticEvent>({
32+
preventDefault: vi.fn(() => {}),
33+
});
34+
35+
function renderUsePinHandlerHook(message: LocalMessage) {
36+
const wrapper = ({ children }: { children?: React.ReactNode }) => (
37+
<ChatProvider value={mockChatContext({ client })}>
38+
<ChannelActionProvider value={mockChannelActionContext({ updateMessage })}>
39+
<ChannelStateProvider
40+
value={mockChannelStateContext({
41+
channel,
42+
channelCapabilities: { 'pin-message': true },
43+
})}
44+
>
45+
{children}
46+
</ChannelStateProvider>
47+
</ChannelActionProvider>
48+
</ChatProvider>
49+
);
50+
51+
const { result } = renderHook(() => usePinHandler(message), { wrapper });
52+
return result.current;
53+
}
54+
55+
// Adds a message to the channel state and returns it as stored, like the message a
56+
// `Message` component renders with.
57+
const seedMessage = (overrides: Parameters<typeof generateMessage>[0]) => {
58+
const message = generateMessage(overrides);
59+
channel.state.addMessageSorted(message, true);
60+
return getStoredMessage(message.id)!;
61+
};
62+
63+
const getStoredMessage = (id: string) => channel.state.findMessage(id);
64+
65+
// Keeps the pin/unpin request pending; the returned function rejects it.
66+
const mockPendingRequest = (method: 'pinMessage' | 'unpinMessage') => {
67+
let rejectRequest: (error: Error) => void = () => {};
68+
vi.spyOn(client, method).mockReturnValue(
69+
new Promise((_, reject) => {
70+
rejectRequest = reject;
71+
}),
72+
);
73+
return rejectRequest;
74+
};
75+
76+
// Starts `handlePin` without awaiting it, so assertions can run while the request is pending.
77+
const startHandlePin = (handlePin: ReturnType<typeof usePinHandler>['handlePin']) => {
78+
let pending: ReturnType<typeof handlePin> = undefined;
79+
act(() => {
80+
pending = handlePin(mouseEventMock);
81+
});
82+
return pending;
83+
};
84+
85+
describe('usePinHandler custom hook', () => {
86+
beforeAll(async () => {
87+
client = await getTestClientWithUser(alice);
88+
channel = client.channel('messaging', 'pin-handler-test');
89+
});
90+
91+
afterEach(() => {
92+
vi.restoreAllMocks();
93+
vi.clearAllMocks();
94+
});
95+
96+
it.each([
97+
{ method: 'pinMessage', pinned: false },
98+
{ method: 'unpinMessage', pinned: true },
99+
] as const)(
100+
'should update the message optimistically when $method succeeds',
101+
async ({ method, pinned }) => {
102+
const requestSpy = vi.spyOn(client, method).mockResolvedValue(fromPartial({}));
103+
const message = seedMessage({ pinned });
104+
const { handlePin } = renderUsePinHandlerHook(message);
105+
await handlePin(mouseEventMock);
106+
expect(requestSpy).toHaveBeenCalledWith(message);
107+
expect(getStoredMessage(message.id)).toEqual(
108+
expect.objectContaining({ pinned: !pinned }),
109+
);
110+
},
111+
);
112+
113+
it.each([
114+
{ method: 'pinMessage', pinned: false },
115+
{ method: 'unpinMessage', pinned: true },
116+
] as const)(
117+
'should roll back and throw when $method fails',
118+
async ({ method, pinned }) => {
119+
const rejectRequest = mockPendingRequest(method);
120+
const message = seedMessage({ pinned });
121+
const { handlePin } = renderUsePinHandlerHook(message);
122+
const pending = startHandlePin(handlePin);
123+
expect(getStoredMessage(message.id)).toEqual(
124+
expect.objectContaining({ pinned: !pinned }),
125+
);
126+
rejectRequest(new Error('request failed'));
127+
await expect(pending).rejects.toThrow('request failed');
128+
expect(getStoredMessage(message.id)).toEqual(expect.objectContaining({ pinned }));
129+
},
130+
);
131+
132+
it('should keep updates that arrived while the request was pending when rolling back', async () => {
133+
const rejectRequest = mockPendingRequest('pinMessage');
134+
const message = seedMessage({ pinned: false, text: 'original' });
135+
const { handlePin } = renderUsePinHandlerHook(message);
136+
const pending = startHandlePin(handlePin);
137+
channel.state.addMessageSorted(
138+
{ ...getStoredMessage(message.id)!, text: 'edited while pinning' },
139+
true,
140+
);
141+
rejectRequest(new Error('pin failed'));
142+
await expect(pending).rejects.toThrow('pin failed');
143+
expect(getStoredMessage(message.id)).toEqual(
144+
expect.objectContaining({ pinned: false, text: 'edited while pinning' }),
145+
);
146+
});
147+
148+
it('should keep a pin state that changed while the request was pending', async () => {
149+
const rejectRequest = mockPendingRequest('unpinMessage');
150+
const newerPinner = generateUser();
151+
const message = seedMessage({ pinned: true, pinned_by: generateUser() });
152+
const { handlePin } = renderUsePinHandlerHook(message);
153+
const pending = startHandlePin(handlePin);
154+
channel.state.addMessageSorted(
155+
{ ...getStoredMessage(message.id)!, pinned: true, pinned_by: newerPinner },
156+
true,
157+
);
158+
rejectRequest(new Error('unpin failed'));
159+
await expect(pending).rejects.toThrow('unpin failed');
160+
expect(getStoredMessage(message.id)?.pinned_by).toEqual(newerPinner);
161+
});
162+
163+
it('should keep a newer pin with the same pinned value that arrived while pinning', async () => {
164+
const rejectRequest = mockPendingRequest('pinMessage');
165+
const newerPinner = generateUser();
166+
const newerPinnedAt = new Date(Date.now() + 60_000);
167+
const message = seedMessage({ pinned: false });
168+
const { handlePin } = renderUsePinHandlerHook(message);
169+
const pending = startHandlePin(handlePin);
170+
channel.state.addMessageSorted(
171+
{
172+
...getStoredMessage(message.id)!,
173+
pinned: true,
174+
pinned_at: newerPinnedAt,
175+
pinned_by: newerPinner,
176+
},
177+
true,
178+
);
179+
rejectRequest(new Error('pin failed'));
180+
await expect(pending).rejects.toThrow('pin failed');
181+
expect(getStoredMessage(message.id)).toEqual(
182+
expect.objectContaining({ pinned: true, pinned_by: newerPinner }),
183+
);
184+
expect(getStoredMessage(message.id)?.pinned_at?.getTime()).toBe(
185+
newerPinnedAt.getTime(),
186+
);
187+
});
188+
});

‎src/components/Message/hooks/usePinHandler.ts‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,48 +5,83 @@ import { useChatContext } from '../../../context/ChatContext';
55
import type { LocalMessage } from 'stream-chat';
66
import type { ReactEventHandler } from '../types';
77

8+
// `pinned_at` is a `Date` here, and a copy of the message from an event carries its own instance,
9+
// so pins are compared by time rather than by reference.
10+
const getPinnedAtTime = (pinnedAt: LocalMessage['pinned_at']) =>
11+
pinnedAt ? new Date(pinnedAt).getTime() : undefined;
12+
813
export const usePinHandler = (message: LocalMessage) => {
914
const { updateMessage } = useChannelActionContext('usePinHandler');
10-
const { channelCapabilities = {} } = useChannelStateContext('usePinHandler');
15+
const { channel, channelCapabilities = {} } = useChannelStateContext('usePinHandler');
1116
const { client } = useChatContext('usePinHandler');
1217

1318
const canPin = !!channelCapabilities['pin-message'];
1419

20+
// The stored message, which may be newer than the `message` this hook rendered with.
21+
const getLatestMessage = () =>
22+
channel.state.findMessage(message.id, message.parent_id) ?? message;
23+
24+
// Restores only the pin fields from before the click, so updates that arrived while the request
25+
// was in flight (an edit, a reaction) are kept. Skipped unless the stored pin is still the
26+
// optimistic one: a pin or unpin that arrived meanwhile is newer than the snapshot, even when
27+
// `pinned` has the same value.
28+
const rollBackPin = (optimisticMessage: LocalMessage) => {
29+
const latestMessage = channel.state.findMessage(message.id, message.parent_id);
30+
if (!latestMessage) return;
31+
32+
const isOptimisticPinStored =
33+
latestMessage.pinned === optimisticMessage.pinned &&
34+
getPinnedAtTime(latestMessage.pinned_at) ===
35+
getPinnedAtTime(optimisticMessage.pinned_at) &&
36+
latestMessage.pinned_by?.id === optimisticMessage.pinned_by?.id;
37+
if (!isOptimisticPinStored) return;
38+
39+
updateMessage({
40+
...latestMessage,
41+
pin_expires: message.pin_expires,
42+
pinned: message.pinned,
43+
pinned_at: message.pinned_at,
44+
pinned_by: message.pinned_by,
45+
});
46+
};
47+
1548
const handlePin: ReactEventHandler = async (event) => {
1649
event.preventDefault();
1750

1851
if (!message) return;
1952

2053
if (!message.pinned) {
21-
try {
22-
const optimisticMessage: LocalMessage = {
23-
...message,
24-
pinned: true,
25-
pinned_at: new Date(),
26-
pinned_by: client.user,
27-
};
54+
const optimisticMessage: LocalMessage = {
55+
...getLatestMessage(),
56+
pinned: true,
57+
pinned_at: new Date(),
58+
pinned_by: client.user,
59+
};
2860

61+
try {
2962
updateMessage(optimisticMessage);
3063

3164
await client.pinMessage(message);
32-
} catch (e) {
33-
updateMessage(message);
65+
} catch (error) {
66+
rollBackPin(optimisticMessage);
67+
throw error;
3468
}
3569
} else {
36-
try {
37-
const optimisticMessage = {
38-
...message,
39-
pin_expires: null,
40-
pinned: false,
41-
pinned_at: null,
42-
pinned_by: null,
43-
};
70+
const optimisticMessage: LocalMessage = {
71+
...getLatestMessage(),
72+
pin_expires: null,
73+
pinned: false,
74+
pinned_at: null,
75+
pinned_by: null,
76+
};
4477

78+
try {
4579
updateMessage(optimisticMessage);
4680

4781
await client.unpinMessage(message);
48-
} catch (e) {
49-
updateMessage(message);
82+
} catch (error) {
83+
rollBackPin(optimisticMessage);
84+
throw error;
5085
}
5186
}
5287
};

‎src/context/MessageContext.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export type MessageContextValue = {
4545
handleMute: ReactEventHandler;
4646
/** Function to open a Thread on a Message */
4747
handleOpenThread: ReactEventHandler;
48-
/** Function to pin a Message in a Channel */
48+
/** Function to pin or unpin a Message in a Channel. Rejects with the request's error when it fails, after rolling back. */
4949
handlePin: ReactEventHandler;
5050
/** Function to post a reaction on a Message */
5151
handleReaction: (

0 commit comments

Comments
 (0)