Skip to content

Commit 45f99c6

Browse files
Merge pull request #1420 from sendbird/fix/clnp-6505-6889
[CLNP-6505][CLNP-6889][fix]: prevent Thread state reset when another user leaves or is banned
2 parents c7895a7 + baeb20a commit 45f99c6

5 files changed

Lines changed: 171 additions & 33 deletions

File tree

src/modules/GroupChannel/components/SuggestedMentionList/SuggestedMentionListView.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ export const SuggestedMentionListView = (props: SuggestedMentionListViewProps) =
121121
}, [
122122
currentChannel?.url,
123123
// We have to be specific like this or React would not recognize the changes in instances.
124+
currentChannel?.members?.length,
125+
currentChannel?.members.map((member: Member) => member.userId).join(),
124126
currentChannel?.members.map((member: Member) => member.nickname).join(),
125127
currentChannel?.members.map((member: Member) => member.isActive).join(),
126128
searchString,

src/modules/Thread/context/__test__/useHandleChannelEvents.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,34 @@ describe('useHandleChannelEvents', () => {
154154
);
155155
});
156156

157+
it('should pass channel and user to onUserBanned handler', () => {
158+
const mockAddHandler = jest.fn();
159+
const sdk = createMockSdk(mockAddHandler);
160+
const channel = createMockChannel();
161+
const bannedUser = { userId: 'banned-user' } as User;
162+
163+
renderChannelEventsHook({ sdk, currentChannel: channel });
164+
165+
const handler = mockAddHandler.mock.calls[0][1];
166+
handler.onUserBanned(channel, bannedUser);
167+
168+
expect(mockThreadActions.onUserBanned).toHaveBeenCalledWith(channel, bannedUser);
169+
});
170+
171+
it('should pass channel and user to onUserLeft handler', () => {
172+
const mockAddHandler = jest.fn();
173+
const sdk = createMockSdk(mockAddHandler);
174+
const channel = createMockChannel();
175+
const leavingUser = { userId: 'leaving-user' } as User;
176+
177+
renderChannelEventsHook({ sdk, currentChannel: channel });
178+
179+
const handler = mockAddHandler.mock.calls[0][1];
180+
handler.onUserLeft(channel, leavingUser);
181+
182+
expect(mockThreadActions.onUserLeft).toHaveBeenCalledWith(channel, leavingUser);
183+
});
184+
157185
it('should not add handler when sdk or currentChannel is missing', async () => {
158186
const mockAddHandler = jest.fn();
159187
const sdk = createMockSdk(mockAddHandler);

src/modules/Thread/context/__test__/useThread.spec.tsx

Lines changed: 84 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,7 @@ describe('useThread', () => {
546546
});
547547
});
548548

549-
it('handles onUserBanned action correctly', async () => {
549+
it('handles onUserBanned action correctly when current user is banned', async () => {
550550
const wrapper = ({ children }) => (
551551
<ThreadProvider channelUrl="test-channel" message={mockParentMessage}>{children}</ThreadProvider>
552552
);
@@ -562,7 +562,7 @@ describe('useThread', () => {
562562
const { onUserBanned } = result.current.actions;
563563

564564
await act(() => {
565-
onUserBanned();
565+
onUserBanned(mockChannel, { userId: 'test-user-id' });
566566
});
567567

568568
await waitFor(() => {
@@ -572,6 +572,46 @@ describe('useThread', () => {
572572
});
573573
});
574574

575+
it('handles onUserBanned action correctly when another user is banned', async () => {
576+
const wrapper = ({ children }) => (
577+
<ThreadProvider channelUrl="test-channel" message={mockParentMessage}>{children}</ThreadProvider>
578+
);
579+
580+
let result;
581+
await act(async () => {
582+
result = renderHook(() => useThread(), { wrapper }).result;
583+
584+
waitFor(() => {
585+
expect(result.current.state.currentChannel).not.toBe(undefined);
586+
});
587+
});
588+
const { onUserBanned } = result.current.actions;
589+
590+
// Channel object reflecting the ban: 'other-user-id' is no longer a member.
591+
const channelAfterBan = {
592+
url: 'test-channel',
593+
members: [{ userId: 'test-user-id', nickname: 'me' }],
594+
};
595+
596+
await act(() => {
597+
onUserBanned(channelAfterBan, { userId: 'other-user-id' });
598+
});
599+
600+
await waitFor(() => {
601+
// Thread state must not be reset when another user is banned.
602+
expect(result.current.state.channelState).not.toBe(ChannelStateTypes.NIL);
603+
expect(result.current.state.currentChannel).not.toBeNull();
604+
// currentChannel must be updated so the membership change is reflected.
605+
expect(result.current.state.currentChannel).toBe(channelAfterBan);
606+
expect(
607+
result.current.state.currentChannel.members.find((m: { userId: string }) => m.userId === 'other-user-id'),
608+
).toBeUndefined();
609+
// nicknamesMap must be regenerated so downstream consumers (e.g. mention list) see the change.
610+
expect(result.current.state.nicknamesMap.get('other-user-id')).toBeUndefined();
611+
expect(result.current.state.nicknamesMap.get('test-user-id')).toBe('me');
612+
});
613+
});
614+
575615
it('handles onUserUnbanned action correctly', async () => {
576616
const wrapper = ({ children }) => (
577617
<ThreadProvider channelUrl="test-channel" message={mockParentMessage}>{children}</ThreadProvider>
@@ -592,7 +632,7 @@ describe('useThread', () => {
592632
});
593633
});
594634

595-
it('handles onUserLeft action correctly', async () => {
635+
it('handles onUserLeft action correctly when current user has left', async () => {
596636
const wrapper = ({ children }) => (
597637
<ThreadProvider channelUrl="test-channel" message={mockParentMessage}>{children}</ThreadProvider>
598638
);
@@ -608,7 +648,7 @@ describe('useThread', () => {
608648
const { onUserLeft } = result.current.actions;
609649

610650
await act(() => {
611-
onUserLeft();
651+
onUserLeft(mockChannel, { userId: 'test-user-id' });
612652
});
613653

614654
await waitFor(() => {
@@ -618,6 +658,46 @@ describe('useThread', () => {
618658
});
619659
});
620660

661+
it('handles onUserLeft action correctly when another user has left', async () => {
662+
const wrapper = ({ children }) => (
663+
<ThreadProvider channelUrl="test-channel" message={mockParentMessage}>{children}</ThreadProvider>
664+
);
665+
666+
let result;
667+
await act(async () => {
668+
result = renderHook(() => useThread(), { wrapper }).result;
669+
670+
waitFor(() => {
671+
expect(result.current.state.currentChannel).not.toBe(undefined);
672+
});
673+
});
674+
const { onUserLeft } = result.current.actions;
675+
676+
// Channel object reflecting the leave: 'other-user-id' is no longer a member.
677+
const channelAfterLeave = {
678+
url: 'test-channel',
679+
members: [{ userId: 'test-user-id', nickname: 'me' }],
680+
};
681+
682+
await act(() => {
683+
onUserLeft(channelAfterLeave, { userId: 'other-user-id' });
684+
});
685+
686+
await waitFor(() => {
687+
// Thread state must not be reset when another user leaves.
688+
expect(result.current.state.channelState).not.toBe(ChannelStateTypes.NIL);
689+
expect(result.current.state.currentChannel).not.toBeNull();
690+
// currentChannel must be updated so the membership change is reflected.
691+
expect(result.current.state.currentChannel).toBe(channelAfterLeave);
692+
expect(
693+
result.current.state.currentChannel.members.find((m: { userId: string }) => m.userId === 'other-user-id'),
694+
).toBeUndefined();
695+
// nicknamesMap must be regenerated so downstream consumers (e.g. mention list) see the change.
696+
expect(result.current.state.nicknamesMap.get('other-user-id')).toBeUndefined();
697+
expect(result.current.state.nicknamesMap.get('test-user-id')).toBe('me');
698+
});
699+
});
700+
621701
it('handles onChannelFrozen action correctly', async () => {
622702
const wrapper = ({ children }) => (
623703
<ThreadProvider channelUrl="test-channel" message={mockParentMessage}>{children}</ThreadProvider>

src/modules/Thread/context/hooks/useHandleChannelEvents.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,15 +73,15 @@ export default function useHandleChannelEvents({
7373
},
7474
onUserBanned(channel, user) {
7575
logger.info('Thread | useHandleChannelEvents: onUserBanned', { channel, user });
76-
onUserBanned();
76+
onUserBanned(channel as GroupChannel, user);
7777
},
7878
onUserUnbanned(channel, user) {
7979
logger.info('Thread | useHandleChannelEvents: onUserUnbanned', { channel, user });
8080
onUserUnbanned();
8181
},
8282
onUserLeft(channel, user) {
8383
logger.info('Thread | useHandleChannelEvents: onUserLeft', { channel, user });
84-
onUserLeft();
84+
onUserLeft(channel as GroupChannel, user);
8585
},
8686
// channel status change
8787
onChannelFrozen(channel) {

src/modules/Thread/context/useThread.ts

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { ChannelStateTypes, FileUploadInfoParams, ParentMessageStateTypes, Threa
55
import { GroupChannel, Member } from '@sendbird/chat/groupChannel';
66
import { CoreMessageType, SendableMessageType } from '../../../utils';
77
import { EmojiContainer, User } from '@sendbird/chat';
8-
import { compareIds } from './utils';
8+
import { compareIds, getNicknamesMapFromMembers } from './utils';
99
import {
1010
BaseMessage,
1111
MultipleFilesMessage,
@@ -458,39 +458,67 @@ const useThread = () => {
458458
};
459459
}), [store]),
460460

461-
onUserBanned: useCallback(() => store.setState(state => {
462-
return {
463-
...state,
464-
channelState: ChannelStateTypes.NIL,
465-
threadListState: ThreadListStateTypes.NIL,
466-
parentMessageState: ParentMessageStateTypes.NIL,
467-
currentChannel: null,
468-
parentMessage: null,
469-
allThreadMessages: [],
470-
hasMorePrev: false,
471-
hasMoreNext: false,
472-
};
473-
}), [store]),
461+
onUserBanned: useCallback((channel: GroupChannel, user: User) => {
462+
store.setState(state => {
463+
if (state.currentChannel?.url !== channel?.url) {
464+
return state;
465+
}
466+
// Only reset state when the current user is banned
467+
if (state.currentUserId === user?.userId) {
468+
return {
469+
...state,
470+
channelState: ChannelStateTypes.NIL,
471+
threadListState: ThreadListStateTypes.NIL,
472+
parentMessageState: ParentMessageStateTypes.NIL,
473+
currentChannel: null,
474+
parentMessage: null,
475+
allThreadMessages: [],
476+
hasMorePrev: false,
477+
hasMoreNext: false,
478+
};
479+
}
480+
// Another user banned: update channel info and nicknames map
481+
return {
482+
...state,
483+
currentChannel: channel,
484+
nicknamesMap: getNicknamesMapFromMembers(channel?.members),
485+
};
486+
});
487+
}, [store]),
474488

475489
onUserUnbanned: useCallback(() => store.setState(state => {
476490
return {
477491
...state,
478492
};
479493
}), [store]),
480494

481-
onUserLeft: useCallback(() => store.setState(state => {
482-
return {
483-
...state,
484-
channelState: ChannelStateTypes.NIL,
485-
threadListState: ThreadListStateTypes.NIL,
486-
parentMessageState: ParentMessageStateTypes.NIL,
487-
currentChannel: null,
488-
parentMessage: null,
489-
allThreadMessages: [],
490-
hasMorePrev: false,
491-
hasMoreNext: false,
492-
};
493-
}), [store]),
495+
onUserLeft: useCallback((channel: GroupChannel, user: User) => {
496+
store.setState(state => {
497+
if (state.currentChannel?.url !== channel?.url) {
498+
return state;
499+
}
500+
// Only reset state when the current user has left
501+
if (state.currentUserId === user?.userId) {
502+
return {
503+
...state,
504+
channelState: ChannelStateTypes.NIL,
505+
threadListState: ThreadListStateTypes.NIL,
506+
parentMessageState: ParentMessageStateTypes.NIL,
507+
currentChannel: null,
508+
parentMessage: null,
509+
allThreadMessages: [],
510+
hasMorePrev: false,
511+
hasMoreNext: false,
512+
};
513+
}
514+
// Another user left: update channel info and nicknames map
515+
return {
516+
...state,
517+
currentChannel: channel,
518+
nicknamesMap: getNicknamesMapFromMembers(channel?.members),
519+
};
520+
});
521+
}, [store]),
494522

495523
onChannelFrozen: useCallback(() => store.setState(state => {
496524
return {

0 commit comments

Comments
 (0)