Skip to content

Commit ff24c4c

Browse files
committed
fix reviews
1 parent 1ba1a12 commit ff24c4c

File tree

3 files changed

+22
-65
lines changed

3 files changed

+22
-65
lines changed

app/containers/MediaCallHeader/MediaCallHeader.test.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,8 @@ describe('MediaCallHeader', () => {
8989
expect(queryByTestId('media-call-header-end')).toBeNull();
9090
});
9191

92-
it('should render empty placeholder when only callId is set (native accepted call, before answerCall completes)', () => {
93-
useCallStore.setState({
94-
call: null,
95-
callId: 'e3246c4d-d23a-412f-8a8b-37ec9f29ef1a',
96-
callState: 'none'
97-
});
92+
it('should render empty placeholder when native accepted but call not bound yet (before answerCall completes)', () => {
93+
useCallStore.getState().setNativeAcceptedCallId('e3246c4d-d23a-412f-8a8b-37ec9f29ef1a');
9894
const { getByTestId, queryByTestId } = render(
9995
<Wrapper>
10096
<MediaCallHeader />

app/lib/services/voip/MediaSessionInstance.test.ts

Lines changed: 9 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ const mockCallStoreReset = jest.fn();
66
const mockUseCallStoreGetState = jest.fn(() => ({
77
reset: mockCallStoreReset,
88
setCall: jest.fn(),
9-
setCallId: jest.fn(),
109
resetNativeCallId: jest.fn(),
1110
call: null as unknown,
1211
callId: null as string | null,
@@ -98,14 +97,17 @@ jest.mock('@rocket.chat/media-signaling', () => ({
9897
})
9998
}));
10099

100+
const STREAM_NOTIFY_USER = 'stream-notify-user';
101+
101102
function getStreamNotifyHandler(): (ddpMessage: IDDPMessage) => void {
102103
const calls = mockOnStreamData.mock.calls as unknown as [string, (m: IDDPMessage) => void][];
103-
const last = calls[calls.length - 1];
104-
const handler = last?.[1];
105-
if (typeof handler !== 'function') {
106-
throw new Error('stream-notify-user handler not registered');
104+
for (let i = calls.length - 1; i >= 0; i--) {
105+
const [eventName, handler] = calls[i];
106+
if (eventName === STREAM_NOTIFY_USER && typeof handler === 'function') {
107+
return handler;
108+
}
107109
}
108-
return handler;
110+
throw new Error('stream-notify-user handler not registered');
109111
}
110112

111113
describe('MediaSessionInstance', () => {
@@ -115,7 +117,6 @@ describe('MediaSessionInstance', () => {
115117
mockUseCallStoreGetState.mockReturnValue({
116118
reset: mockCallStoreReset,
117119
setCall: jest.fn(),
118-
setCallId: jest.fn(),
119120
resetNativeCallId: jest.fn(),
120121
call: null,
121122
callId: null,
@@ -197,40 +198,8 @@ describe('MediaSessionInstance', () => {
197198
});
198199

199200
describe('stream-notify-user (notification/accepted gated)', () => {
200-
it('does not call answerCall when store has no native-accepted callId', async () => {
201-
const answerSpy = jest.spyOn(mediaSessionInstance, 'answerCall').mockResolvedValue(undefined);
202-
mediaSessionInstance.init('user-1');
203-
const streamHandler = getStreamNotifyHandler();
204-
streamHandler({
205-
msg: 'changed',
206-
fields: {
207-
eventName: 'uid/media-signal',
208-
args: [
209-
{
210-
type: 'notification',
211-
notification: 'accepted',
212-
signedContractId: 'test-device-id',
213-
callId: 'from-signal'
214-
}
215-
]
216-
}
217-
});
218-
await Promise.resolve();
219-
expect(answerSpy).not.toHaveBeenCalled();
220-
answerSpy.mockRestore();
221-
});
222-
223-
it('does not call answerCall when transient callId matches signal but nativeAcceptedCallId does not', async () => {
201+
it('does not call answerCall when nativeAcceptedCallId is null', async () => {
224202
const answerSpy = jest.spyOn(mediaSessionInstance, 'answerCall').mockResolvedValue(undefined);
225-
mockUseCallStoreGetState.mockReturnValue({
226-
reset: mockCallStoreReset,
227-
setCall: jest.fn(),
228-
setCallId: jest.fn(),
229-
resetNativeCallId: jest.fn(),
230-
call: null,
231-
callId: 'from-signal',
232-
nativeAcceptedCallId: null
233-
});
234203
mediaSessionInstance.init('user-1');
235204
const streamHandler = getStreamNotifyHandler();
236205
streamHandler({
@@ -257,7 +226,6 @@ describe('MediaSessionInstance', () => {
257226
mockUseCallStoreGetState.mockReturnValue({
258227
reset: mockCallStoreReset,
259228
setCall: jest.fn(),
260-
setCallId: jest.fn(),
261229
resetNativeCallId: jest.fn(),
262230
call: null,
263231
callId: null,
@@ -289,7 +257,6 @@ describe('MediaSessionInstance', () => {
289257
mockUseCallStoreGetState.mockReturnValue({
290258
reset: mockCallStoreReset,
291259
setCall: jest.fn(),
292-
setCallId: jest.fn(),
293260
resetNativeCallId: jest.fn(),
294261
call: null,
295262
callId: null,
@@ -321,7 +288,6 @@ describe('MediaSessionInstance', () => {
321288
mockUseCallStoreGetState.mockReturnValue({
322289
reset: mockCallStoreReset,
323290
setCall: jest.fn(),
324-
setCallId: jest.fn(),
325291
resetNativeCallId: jest.fn(),
326292
call: { callId: 'from-signal' } as any,
327293
callId: 'from-signal',

app/lib/services/voip/useCallStore.ts

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const STALE_NATIVE_MS = 15_000;
1010

1111
let callListenersCleanup: (() => void) | null = null;
1212
let staleNativeTimer: ReturnType<typeof setTimeout> | null = null;
13-
/** Id this timer may clear; must match `nativeAcceptedCallId` at fire time. */
13+
/** Id the stale-native timer was armed for; must match `nativeAcceptedCallId` at fire time. Timer clears only this field, not `callId`. */
1414
let staleNativeScheduledId: string | null = null;
1515

1616
export function cleanupCallListeners(): void {
@@ -26,6 +26,14 @@ function cancelStaleNativeTimer(): void {
2626
staleNativeScheduledId = null;
2727
}
2828

29+
function clearStaleNativeIfStillUnbound(get: () => CallStore, scheduled: string): void {
30+
const st = get();
31+
if (st.call != null || st.nativeAcceptedCallId !== scheduled) {
32+
return;
33+
}
34+
useCallStore.setState({ nativeAcceptedCallId: null });
35+
}
36+
2937
function armStaleNativeTimer(get: () => CallStore): void {
3038
cancelStaleNativeTimer();
3139
const scheduledId = get().nativeAcceptedCallId;
@@ -37,17 +45,9 @@ function armStaleNativeTimer(get: () => CallStore): void {
3745
staleNativeTimer = null;
3846
const scheduled = staleNativeScheduledId;
3947
staleNativeScheduledId = null;
40-
const st = get();
41-
if (st.call != null) {
42-
return;
43-
}
44-
if (st.nativeAcceptedCallId !== scheduled) {
45-
return;
48+
if (scheduled != null) {
49+
clearStaleNativeIfStillUnbound(get, scheduled);
4650
}
47-
useCallStore.setState({
48-
nativeAcceptedCallId: null,
49-
callId: null
50-
});
5151
}, STALE_NATIVE_MS);
5252
}
5353

@@ -75,7 +75,6 @@ interface CallStoreState {
7575
}
7676

7777
interface CallStoreActions {
78-
setCallId: (callId: string | null) => void;
7978
/** Native accept paths: sets sticky id only; starts/restarts stale-native timer. */
8079
setNativeAcceptedCallId: (callId: string) => void;
8180
/** Clears sticky native id (+ transient `callId` when unbound); cancels stale timer. */
@@ -112,10 +111,6 @@ const initialState: CallStoreState = {
112111
export const useCallStore = create<CallStore>((set, get) => ({
113112
...initialState,
114113

115-
setCallId: (callId: string | null) => {
116-
set({ callId });
117-
},
118-
119114
setNativeAcceptedCallId: (callId: string) => {
120115
cancelStaleNativeTimer();
121116
set({ nativeAcceptedCallId: callId });

0 commit comments

Comments
 (0)