From a62e884e8cc076b828f8e8e0f81386712776bb7c Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 12 Nov 2024 16:36:41 +0000 Subject: [PATCH 1/4] Backfill hand raised events once client has synced. --- src/useReactions.tsx | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/useReactions.tsx b/src/useReactions.tsx index c8d3c3b59..c56f08c86 100644 --- a/src/useReactions.tsx +++ b/src/useReactions.tsx @@ -11,6 +11,8 @@ import { RelationType, RoomEvent as MatrixRoomEvent, MatrixEventEvent, + ClientEvent, + SyncState, } from "matrix-js-sdk/src/matrix"; import { ReactionEventContent } from "matrix-js-sdk/src/types"; import { @@ -91,6 +93,7 @@ export const ReactionsProvider = ({ clientState?.state === "valid" && clientState.supportedFeatures.reactions; const room = rtcSession.room; const myUserId = room.client.getUserId(); + const [hasBackfilled, setHasBackfilled] = useState(false); const [reactions, setReactions] = useState>( {}, @@ -106,6 +109,7 @@ export const ReactionsProvider = ({ ); const addRaisedHand = useCallback((userId: string, info: RaisedHandInfo) => { + logger.info(`Adding raised hand for ${userId}`); setRaisedHands((prevRaisedHands) => ({ ...prevRaisedHands, [userId]: info, @@ -113,13 +117,15 @@ export const ReactionsProvider = ({ }, []); const removeRaisedHand = useCallback((userId: string) => { + logger.info(`Removing raised hand for ${userId}`); setRaisedHands( ({ [userId]: _removed, ...remainingRaisedHands }) => remainingRaisedHands, ); }, []); // This effect will check the state whenever the membership of the session changes. - useEffect(() => { + // This function is safe to run multiple times. + const backfillHandRaised = useCallback(() => { // Fetches the first reaction for a given event. const getLastReactionEvent = ( eventId: string, @@ -178,6 +184,29 @@ export const ReactionsProvider = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [room, memberships, myUserId, addRaisedHand, removeRaisedHand]); + useEffect(() => { + // Run once on startup (and will rerun when membership changes above) + backfillHandRaised(); + + function onSync(state: SyncState | null): void { + if (state === SyncState.Prepared) { + logger.debug("Backfilling hand raised after successful sync."); + backfillHandRaised(); + setHasBackfilled(true); + } + } + // The *first* time we successfully sync, fetch reactions as they may not + // be immediately available. + if (!hasBackfilled) { + room.client.on(ClientEvent.Sync, onSync); + } + return (): void => { + if (!hasBackfilled) { + room.client.off(ClientEvent.Sync, onSync); + } + }; + }, [backfillHandRaised, room, hasBackfilled]); + const latestMemberships = useLatest(memberships); const latestRaisedHands = useLatest(raisedHands); @@ -307,8 +336,8 @@ export const ReactionsProvider = ({ return (): void => { room.off(MatrixRoomEvent.Timeline, handleReactionEvent); room.off(MatrixRoomEvent.Redaction, handleReactionEvent); - room.client.off(MatrixEventEvent.Decrypted, handleReactionEvent); room.off(MatrixRoomEvent.LocalEchoUpdated, handleReactionEvent); + room.client.off(MatrixEventEvent.Decrypted, handleReactionEvent); reactionTimeouts.forEach((t) => clearTimeout(t)); // If we're clearing timeouts, we also clear all reactions. setReactions({}); @@ -319,6 +348,8 @@ export const ReactionsProvider = ({ removeRaisedHand, latestMemberships, latestRaisedHands, + backfillHandRaised, + hasBackfilled, ]); const lowerHand = useCallback(async () => { From 3b272181ae1b1a5648f6ba2ad5a9949181818e7e Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 12 Nov 2024 17:02:04 +0000 Subject: [PATCH 2/4] Use fetchRelations to backfill. --- src/useReactions.tsx | 78 ++++++++++++++++--------------------- src/utils/testReactions.tsx | 1 + 2 files changed, 35 insertions(+), 44 deletions(-) diff --git a/src/useReactions.tsx b/src/useReactions.tsx index c56f08c86..f009ae93d 100644 --- a/src/useReactions.tsx +++ b/src/useReactions.tsx @@ -11,8 +11,6 @@ import { RelationType, RoomEvent as MatrixRoomEvent, MatrixEventEvent, - ClientEvent, - SyncState, } from "matrix-js-sdk/src/matrix"; import { ReactionEventContent } from "matrix-js-sdk/src/types"; import { @@ -93,7 +91,6 @@ export const ReactionsProvider = ({ clientState?.state === "valid" && clientState.supportedFeatures.reactions; const room = rtcSession.room; const myUserId = room.client.getUserId(); - const [hasBackfilled, setHasBackfilled] = useState(false); const [reactions, setReactions] = useState>( {}, @@ -124,19 +121,29 @@ export const ReactionsProvider = ({ }, []); // This effect will check the state whenever the membership of the session changes. - // This function is safe to run multiple times. - const backfillHandRaised = useCallback(() => { + useEffect(() => { // Fetches the first reaction for a given event. - const getLastReactionEvent = ( + const getLastReactionEvent = async ( eventId: string, expectedSender: string, - ): MatrixEvent | undefined => { + ): Promise => { const relations = room.relations.getChildEventsForEvent( eventId, RelationType.Annotation, EventType.Reaction, ); - const allEvents = relations?.getRelations() ?? []; + let allEvents = relations?.getRelations() ?? []; + // We might not have synced this far. + if (allEvents.length === 0) { + const res = await room.client.fetchRelations( + room.roomId, + eventId, + RelationType.Annotation, + EventType.Reaction, + ); + allEvents = res.chunk.map((e) => new MatrixEvent(e)); + } + return allEvents.find( (reaction) => reaction.event.sender === expectedSender && @@ -166,47 +173,32 @@ export const ReactionsProvider = ({ // was raised, reset. removeRaisedHand(m.sender); } - const reaction = getLastReactionEvent(m.eventId, m.sender); - if (reaction) { - const eventId = reaction?.getId(); - if (!eventId) { - continue; - } - addRaisedHand(m.sender, { - membershipEventId: m.eventId, - reactionEventId: eventId, - time: new Date(reaction.localTimestamp), + getLastReactionEvent(m.eventId, m.sender) + .then((reaction) => { + if (reaction) { + const eventId = reaction.getId(); + if (!eventId) { + return; + } + addRaisedHand(m.sender!, { + membershipEventId: m.eventId!, + reactionEventId: eventId, + time: new Date(reaction.localTimestamp), + }); + } + }) + .catch((ex) => { + logger.warn( + `Failed to fetch reaction for member ${m.sender} (${m.eventId})`, + ex, + ); }); - } } // Ignoring raisedHands here because we don't want to trigger each time the raised // hands set is updated. // eslint-disable-next-line react-hooks/exhaustive-deps }, [room, memberships, myUserId, addRaisedHand, removeRaisedHand]); - useEffect(() => { - // Run once on startup (and will rerun when membership changes above) - backfillHandRaised(); - - function onSync(state: SyncState | null): void { - if (state === SyncState.Prepared) { - logger.debug("Backfilling hand raised after successful sync."); - backfillHandRaised(); - setHasBackfilled(true); - } - } - // The *first* time we successfully sync, fetch reactions as they may not - // be immediately available. - if (!hasBackfilled) { - room.client.on(ClientEvent.Sync, onSync); - } - return (): void => { - if (!hasBackfilled) { - room.client.off(ClientEvent.Sync, onSync); - } - }; - }, [backfillHandRaised, room, hasBackfilled]); - const latestMemberships = useLatest(memberships); const latestRaisedHands = useLatest(raisedHands); @@ -348,8 +340,6 @@ export const ReactionsProvider = ({ removeRaisedHand, latestMemberships, latestRaisedHands, - backfillHandRaised, - hasBackfilled, ]); const lowerHand = useCallback(async () => { diff --git a/src/utils/testReactions.tsx b/src/utils/testReactions.tsx index 84ff217bd..e9ced62a6 100644 --- a/src/utils/testReactions.tsx +++ b/src/utils/testReactions.tsx @@ -139,6 +139,7 @@ export class MockRoom extends EventEmitter { return Promise.resolve({ event_id: randomUUID() }); }, decryptEventIfNeeded: async () => {}, + fetchRelations: async () => Promise.resolve({ chunk: [] }), on() { return this; }, From 7a05ad9f79e0ade731caf0ddc1d85f7c466b041b Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 12 Nov 2024 17:04:28 +0000 Subject: [PATCH 3/4] update comment --- src/useReactions.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/useReactions.tsx b/src/useReactions.tsx index f009ae93d..c6412c83b 100644 --- a/src/useReactions.tsx +++ b/src/useReactions.tsx @@ -133,7 +133,7 @@ export const ReactionsProvider = ({ EventType.Reaction, ); let allEvents = relations?.getRelations() ?? []; - // We might not have synced this far. + // If we found no relations for this event, fetch via fetchRelations. if (allEvents.length === 0) { const res = await room.client.fetchRelations( room.roomId, From 1d685bc44e6995185ec8c784185c2818bae3c1aa Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 12 Nov 2024 17:10:26 +0000 Subject: [PATCH 4/4] Fix tests --- src/useReactions.test.tsx | 16 ++++++++-------- src/useReactions.tsx | 1 - 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/useReactions.test.tsx b/src/useReactions.test.tsx index 6140793fc..3fb432475 100644 --- a/src/useReactions.test.tsx +++ b/src/useReactions.test.tsx @@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { render } from "@testing-library/react"; +import { findByRole, getByRole, render } from "@testing-library/react"; import { act, FC } from "react"; import { describe, expect, test } from "vitest"; import { RoomEvent } from "matrix-js-sdk/src/matrix"; @@ -43,7 +43,7 @@ const TestComponent: FC = () => {
    {Object.entries(raisedHands).map(([userId, date]) => ( -
  • +
  • {userId}
    • @@ -106,12 +106,12 @@ describe("useReactions", () => { createHandRaisedReaction(memberEventAlice, membership), ]); const rtcSession = new MockRTCSession(room, membership); - const { queryByRole } = render( + const { findByRole } = render( , ); - expect(queryByRole("list")?.children).to.have.lengthOf(1); + expect(findByRole("listitem")).toBeTruthy(); }); // If the membership event changes for a user, we want to remove // the raised hand event. @@ -120,12 +120,12 @@ describe("useReactions", () => { createHandRaisedReaction(memberEventAlice, membership), ]); const rtcSession = new MockRTCSession(room, membership); - const { queryByRole } = render( + const { findByRole, queryByRole } = render( , ); - expect(queryByRole("list")?.children).to.have.lengthOf(1); + expect(findByRole("listitem")).toBeTruthy(); act(() => rtcSession.testRemoveMember(memberUserIdAlice)); expect(queryByRole("list")?.children).to.have.lengthOf(0); }); @@ -134,12 +134,12 @@ describe("useReactions", () => { createHandRaisedReaction(memberEventAlice, membership), ]); const rtcSession = new MockRTCSession(room, membership); - const { queryByRole } = render( + const { queryByRole, findByRole } = render( , ); - expect(queryByRole("list")?.children).to.have.lengthOf(1); + expect(findByRole("listitem")).toBeTruthy(); // Simulate leaving and rejoining act(() => { rtcSession.testRemoveMember(memberUserIdAlice); diff --git a/src/useReactions.tsx b/src/useReactions.tsx index c6412c83b..ce527eab9 100644 --- a/src/useReactions.tsx +++ b/src/useReactions.tsx @@ -143,7 +143,6 @@ export const ReactionsProvider = ({ ); allEvents = res.chunk.map((e) => new MatrixEvent(e)); } - return allEvents.find( (reaction) => reaction.event.sender === expectedSender &&