From 1c8930daa0b16514a67e69451f257a9145b0f398 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Wed, 9 Sep 2026 18:46:18 +0200 Subject: [PATCH] Join the call as soon as a request to join is accepted An invite that replaced a request to join was read as somebody else being mid-join. Nobody always is: a host without auto-join, a join that failed, or a fresh load already at that membership left the user waiting with nothing to press but "Cancel request". The lobby now takes such an invite up itself, showing a joining state, and offers a join of the user's own if that fails. Co-Authored-By: Claude Fable 5.1 --- locales/en/app.json | 2 + src/room/LobbyJoinState.ts | 9 +- src/room/LobbyView.test.tsx | 26 ++++++ src/room/LobbyView.tsx | 13 ++- src/room/useLoadGroupCall.test.tsx | 140 ++++++++++++++++++++++++++++- src/room/useLoadGroupCall.ts | 112 ++++++++++++++++------- 6 files changed, 265 insertions(+), 37 deletions(-) diff --git a/locales/en/app.json b/locales/en/app.json index 2116d4bdc..c293f273a 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -156,7 +156,9 @@ "invite_only_body": "You need an invite to join this call.", "join_as_guest": "Join as guest", "join_button": "Join call", + "joining": "Joining…", "leave_button": "Back to recents", + "request_accepted": "Your request to join was accepted.", "request_sent": "Request to join sent", "request_sent_body": "You will receive an invite to join the call if your request is accepted." }, diff --git a/src/room/LobbyJoinState.ts b/src/room/LobbyJoinState.ts index 802e8a6b9..7a9523b0f 100644 --- a/src/room/LobbyJoinState.ts +++ b/src/room/LobbyJoinState.ts @@ -10,8 +10,11 @@ Please see LICENSE in the repository root for full details. * the lobby offers them. Everything the lobby needs to know about membership. */ export type LobbyJoinState = - /** The user may enter the call right away. */ - | { kind: "can-join"; join: () => void } + /** + * The user may enter the call right away. `notice` is set when they got here + * by having a request accepted, but the join it entitles them to failed. + */ + | { kind: "can-join"; join: () => void; notice?: "request_accepted" } /** * The room only takes knocks. `error` is set when a previous request failed * to send. @@ -23,6 +26,8 @@ export type LobbyJoinState = } /** The request is on its way to the server. */ | { kind: "sending-request" } + /** The user's join is on its way. */ + | { kind: "joining" } /** * The request is with the room's moderators. `cancelRequest` is absent while * a withdrawal is on its way, and where withdrawing is not supported. diff --git a/src/room/LobbyView.test.tsx b/src/room/LobbyView.test.tsx index 695239936..bed8e1bfb 100644 --- a/src/room/LobbyView.test.tsx +++ b/src/room/LobbyView.test.tsx @@ -203,6 +203,22 @@ describe("LobbyView", () => { disabled: true, message: null, }, + { + joinState: { kind: "joining" }, + button: "Joining", + disabled: true, + message: null, + }, + { + joinState: { + kind: "can-join", + join: () => {}, + notice: "request_accepted", + }, + button: "Join call", + disabled: false, + message: "Your request to join was accepted.", + }, { joinState: { kind: "waiting-for-approval" }, button: "Request to join sent", @@ -270,6 +286,16 @@ describe("LobbyView", () => { ); }); + it("does nothing while the join is on its way", async () => { + const { getByTestId } = renderLobbyView({ + joinState: { kind: "joining" }, + }); + const button = getByTestId("lobby_joinCall"); + expect(button).toHaveAttribute("aria-busy", "true"); + await userEvent.click(button); + expect(button).toHaveAttribute("aria-disabled", "true"); + }); + it("joins when the join button is pressed", async () => { const join = vi.fn(); const { getByTestId } = renderLobbyView({ diff --git a/src/room/LobbyView.tsx b/src/room/LobbyView.tsx index 0c71911ca..b35ba8f5f 100644 --- a/src/room/LobbyView.tsx +++ b/src/room/LobbyView.tsx @@ -236,6 +236,7 @@ export const LobbyView: FC = ({ ); case "sending-request": + case "joining": return ( ); case "waiting-for-approval": @@ -269,6 +274,10 @@ export const LobbyView: FC = ({ const joinMessage = ((): ReactNode => { switch (joinState.kind) { + case "can-join": + return joinState.notice === undefined ? null : ( + {t("lobby.request_accepted")} + ); case "can-ask-to-join": return joinState.error === undefined ? null : ( {t("error.generic")} @@ -317,8 +326,8 @@ export const LobbyView: FC = ({ ); case "not-allowed": return {t("lobby.invite_only_body")}; - case "can-join": case "sending-request": + case "joining": return null; } })(); diff --git a/src/room/useLoadGroupCall.test.tsx b/src/room/useLoadGroupCall.test.tsx index 048e1b1b9..4ce50d32e 100644 --- a/src/room/useLoadGroupCall.test.tsx +++ b/src/room/useLoadGroupCall.test.tsx @@ -236,6 +236,53 @@ describe("useLoadGroupCall in the standalone app", () => { expect(client.waitUntilRoomReadyForGroupCalls).toHaveBeenCalledWith(roomId); }); + it("joins straight away when a request was accepted before this load", async () => { + const spec: RoomSpec = { + membership: KnownMembership.Invite, + prevMembership: KnownMembership.Knock, + }; + const room = mockRoom(spec); + const client = mockClient({ + getRoom: vi.fn().mockReturnValue(room), + joinRoom: vi.fn().mockResolvedValue(room), + }); + const { result } = renderLoad(client); + await waitForJoinState(result, "joining"); + expect(client.joinRoom).toHaveBeenCalledWith(roomId, { viaServers }); + spec.membership = KnownMembership.Join; + emitMembership(client, room, KnownMembership.Join, KnownMembership.Invite); + await waitFor(() => expect(result.current.kind).toBe("loaded")); + expect(client.getRoomSummary).not.toHaveBeenCalled(); + }); + + it("offers a join of our own when an accepted request cannot be taken up", async () => { + const error = vi.spyOn(logger, "error").mockImplementation(() => {}); + const room = mockRoom({ + membership: KnownMembership.Invite, + prevMembership: KnownMembership.Knock, + }); + const client = mockClient({ + getRoom: vi.fn().mockReturnValue(room), + joinRoom: vi.fn().mockRejectedValue(new Error("offline")), + }); + const { result } = renderLoad(client); + expect(await waitForJoinState(result, "can-join")).toMatchObject({ + notice: "request_accepted", + }); + expect(error).toHaveBeenCalled(); + }); + + it("joins an invite that answers no request without a lobby", async () => { + const room = mockRoom({ membership: KnownMembership.Invite }); + const client = mockClient({ + getRoom: vi.fn().mockReturnValue(room), + joinRoom: vi.fn().mockResolvedValue(room), + }); + const { result } = renderLoad(client); + await waitFor(() => expect(result.current.kind).toBe("loaded")); + expect(client.joinRoom).toHaveBeenCalledWith(roomId, { viaServers }); + }); + it("shows a declined request in the lobby", async () => { const spec: RoomSpec = { membership: KnownMembership.Knock }; const room = mockRoom(spec); @@ -366,7 +413,7 @@ describe("useLoadGroupCall as a widget", () => { membership: KnownMembership.Invite, prevMembership: KnownMembership.Knock, }, - "waiting-for-approval", + "joining", ], ["a plain invite", { membership: KnownMembership.Invite }, "can-join"], [ @@ -416,7 +463,12 @@ describe("useLoadGroupCall as a widget", () => { const client = mockClient({ getRoom: vi.fn().mockReturnValue(mockRoom(spec)), }); - const { result } = renderLoad(client, host()); + // A request the host never answers, so that the state a lobby opens on is + // the one under test. + const { result } = renderLoad( + client, + host(vi.fn().mockReturnValue(new Promise(() => {}))), + ); await waitForJoinState(result, expected as LobbyJoinState["kind"]); }); @@ -493,6 +545,90 @@ describe("useLoadGroupCall as a widget", () => { await waitForJoinState(result, "not-allowed"); }); + it("asks the host to join us as soon as a request is accepted", async () => { + const changeMembership = vi.fn().mockReturnValue(new Promise(() => {})); + const spec: RoomSpec = { membership: KnownMembership.Knock }; + const room = mockRoom(spec); + const client = mockClient({ getRoom: vi.fn().mockReturnValue(room) }); + const { result } = renderLoad(client, host(changeMembership)); + await waitForJoinState(result, "waiting-for-approval"); + spec.membership = KnownMembership.Invite; + emitMembership(client, room, KnownMembership.Invite, KnownMembership.Knock); + await waitForJoinState(result, "joining"); + expect(changeMembership).toHaveBeenCalledWith({ action: "join" }); + }); + + it("enters the call when the host reports us joined", async () => { + const spec: RoomSpec = { + membership: KnownMembership.Invite, + prevMembership: KnownMembership.Knock, + }; + const room = mockRoom(spec); + const client = mockClient({ getRoom: vi.fn().mockReturnValue(room) }); + // The host joins us as it answers, so no membership event of our own + // follows the reply. + const joinAsHost = vi.fn(async (): Promise => { + spec.membership = KnownMembership.Join; + return await Promise.resolve(KnownMembership.Join); + }); + const { result } = renderLoad(client, host(joinAsHost)); + await waitFor(() => expect(result.current.kind).toBe("loaded")); + }); + + it("stays in the call when the host answers our join late", async () => { + const error = vi.spyOn(logger, "error").mockImplementation(() => {}); + let refuse!: (error: Error) => void; + const spec: RoomSpec = { + membership: KnownMembership.Invite, + prevMembership: KnownMembership.Knock, + }; + const room = mockRoom(spec); + const client = mockClient({ getRoom: vi.fn().mockReturnValue(room) }); + const { result } = renderLoad( + client, + host( + vi.fn().mockReturnValue( + new Promise((_resolve, reject) => { + refuse = reject; + }), + ), + ), + ); + await waitForJoinState(result, "joining"); + spec.membership = KnownMembership.Join; + emitMembership(client, room, KnownMembership.Join, KnownMembership.Invite); + await waitFor(() => expect(result.current.kind).toBe("loaded")); + const loaded = result.current; + refuse(new Error("Request timed out")); + await waitFor(() => expect(error).toHaveBeenCalled()); + expect(result.current).toBe(loaded); + }); + + it("offers a join of our own when the host cannot answer", async () => { + const error = vi.spyOn(logger, "error").mockImplementation(() => {}); + const changeMembership = vi + .fn() + .mockRejectedValue(new Error("Request timed out")); + const client = mockClient({ + getRoom: vi.fn().mockReturnValue( + mockRoom({ + membership: KnownMembership.Invite, + prevMembership: KnownMembership.Knock, + }), + ), + }); + const { result } = renderLoad(client, host(changeMembership)); + const canJoin = await waitForJoinState(result, "can-join"); + expect(canJoin).toMatchObject({ notice: "request_accepted" }); + expect(error).toHaveBeenCalled(); + act(() => canJoin.join()); + await waitForJoinState(result, "joining"); + expect(await waitForJoinState(result, "can-join")).toMatchObject({ + notice: "request_accepted", + }); + expect(changeMembership).toHaveBeenCalledTimes(2); + }); + it("resolves when the host joined us before we could listen", async () => { const room = mockRoom({ membership: KnownMembership.Knock }); (room as { getMyMembership: () => Membership }).getMyMembership = vi diff --git a/src/room/useLoadGroupCall.ts b/src/room/useLoadGroupCall.ts index 44f7f3ee7..2b8085cab 100644 --- a/src/room/useLoadGroupCall.ts +++ b/src/room/useLoadGroupCall.ts @@ -97,10 +97,18 @@ export class CallTerminatedMessage extends Error { } } +/** The membership the local user's current one replaced, if any. */ +function previousMembership(room: Room): Membership | undefined { + return room.currentState + .getStateEvents(EventType.RoomMember, room.myUserId) + ?.getPrevContent().membership as Membership | undefined; +} + /** The join state a lobby opens on, before the user acts on it. */ type LobbyEntry = | "can-join" | "can-ask-to-join" + | "joining" | "waiting-for-approval" | "denied" | "banned" @@ -196,22 +204,31 @@ export const useLoadGroupCall = ( }; /** - * Resolve once the local user has joined the room, including when they - * already have: the host may have joined us before we could listen. + * Watch for the local user joining the room. `check` re-tests the room as + * it stands, for a join that happened without an event we saw, such as one + * the host had already applied. */ - const waitForJoin = async (roomId: string): Promise => - await new Promise((resolve) => { - const reached = (room: Room | null): boolean => { + const watchForJoin = ( + roomId: string, + ): { reached: Promise; check: () => void } => { + let check = (): void => {}; + const reached = new Promise((resolve) => { + const test = (room: Room | null): boolean => { if (room?.getMyMembership() !== KnownMembership.Join) return false; activeRoom.current = room; resolve(room); return true; }; const off = onMyMembership((room) => { - if (room.roomId === roomId && reached(room)) off(); + if (room.roomId === roomId && test(room)) off(); }); - if (reached(client.getRoom(roomId))) off(); + check = (): void => { + if (test(client.getRoom(roomId))) off(); + }; + check(); }); + return { reached, check }; + }; /** * Show the lobby and resolve once the local user has joined. Denial, ban @@ -232,9 +249,16 @@ export const useLoadGroupCall = ( changeMembershipWithClient(client, roomId, viaServers); let requestInFlight = false; let withdrawing = false; + let entered = false; + // Listening before anything is sent, so that a join landing while a + // request is in flight is not missed. + const { reached, check } = watchForJoin(roomId); + + /** Sets the lobby state, unless the user is already through the lobby. */ const setLobby = (joinState: LobbyJoinState): void => { - if (!signal.aborted) setState({ kind: "lobby", room, joinState }); + if (!entered && !signal.aborted) + setState({ kind: "lobby", room, joinState }); }; const waitForApproval = (): void => @@ -245,15 +269,24 @@ export const useLoadGroupCall = ( const canJoin = (): void => setLobby({ kind: "can-join", join }); + /** The request was accepted, but the join it entitles us to failed. */ + const acceptedButNotJoined = (): void => + setLobby({ + kind: "can-join", + join: joinOnceAccepted, + notice: "request_accepted", + }); + const onRequestResult = ( membership: Membership, operation: string, onFailure: () => void, ): void => { requestInFlight = false; - // A join resolves the promise this lobby is parked on, so there is - // nothing left to show. - if (membership === KnownMembership.Join) return; + // A membership the room already holds reaches us in the reply rather + // than in an event of its own, so the join this lobby is parked on is + // re-tested here. + if (membership === KnownMembership.Join) return check(); if (membership === KnownMembership.Knock) return waitForApproval(); logger.error( `${operation} on ${roomId} left us with membership ${membership}`, @@ -284,12 +317,34 @@ export const useLoadGroupCall = ( const join = (): void => { if (requestInFlight) return; requestInFlight = true; + setLobby({ kind: "joining" }); changeMembership({ action: "join" }).then( (membership) => onRequestResult(membership, "Joining", canJoin), (error) => onRequestError(error, "Joining", canJoin), ); }; + /** Takes up the invite an accepted request has earned us. */ + const joinOnceAccepted = (): void => { + if (requestInFlight) return; + requestInFlight = true; + setLobby({ kind: "joining" }); + changeMembership({ action: "join" }).then( + (membership) => + onRequestResult( + membership, + "Joining once accepted", + acceptedButNotJoined, + ), + (error) => + onRequestError( + error, + "Joining once accepted", + acceptedButNotJoined, + ), + ); + }; + const askToJoin = (reason?: string): void => { if (requestInFlight) return; requestInFlight = true; @@ -320,18 +375,10 @@ export const useLoadGroupCall = ( activeRoom.current = changed; switch (membership) { case KnownMembership.Invite: - // A host that changes memberships on our behalf also performs the - // join an accepted request entitles us to. - if (prevMembership !== KnownMembership.Knock) canJoin(); - else if (hostBridge.changeMembership === undefined) - changeMembership({ action: "join" }).then( - () => logger.info(`Joined ${roomId} once accepted`), - (error: unknown) => - logger.error( - `Joining ${roomId} once accepted failed`, - error, - ), - ); + // An invite that replaced a request is ours to take up: nothing + // else is going to turn it into a join. + if (prevMembership === KnownMembership.Knock) joinOnceAccepted(); + else canJoin(); break; case KnownMembership.Ban: setLobby({ kind: "banned", reason: leaveReason() }); @@ -357,6 +404,9 @@ export const useLoadGroupCall = ( case "can-ask-to-join": canAskToJoin(); break; + case "joining": + joinOnceAccepted(); + break; case "waiting-for-approval": waitForApproval(); break; @@ -371,7 +421,8 @@ export const useLoadGroupCall = ( break; } - const joined = await waitForJoin(roomId); + const joined = await reached; + entered = true; offTransitions(); return joined; }; @@ -384,16 +435,13 @@ export const useLoadGroupCall = ( room: Room, membership: Membership | undefined, ): LobbyEntry => { - const prevMembership = room.currentState - .getStateEvents(EventType.RoomMember, room.myUserId) - ?.getPrevContent().membership as Membership | undefined; + const prevMembership = previousMembership(room); if (membership === KnownMembership.Ban) return "banned"; if (membership === KnownMembership.Knock) return "waiting-for-approval"; if (membership === KnownMembership.Invite) - // An invite that replaced a request means the host is mid-join. return prevMembership === KnownMembership.Knock - ? "waiting-for-approval" + ? "joining" : "can-join"; if (prevMembership === KnownMembership.Knock) return "denied"; @@ -457,9 +505,11 @@ export const useLoadGroupCall = ( if (room && membership === KnownMembership.Ban) return await enterFromLobby(preJoinRoomInfoFromRoom(room), "banned"); - if (membership === KnownMembership.Invite) + if (room && membership === KnownMembership.Invite) return await readyForGroupCalls( - await client.joinRoom(roomId, { viaServers }), + previousMembership(room) === KnownMembership.Knock + ? await enterFromLobby(preJoinRoomInfoFromRoom(room), "joining") + : await client.joinRoom(roomId, { viaServers }), ); // If the room does not exist we first search for it with viaServers