Skip to content

Commit 86bdb80

Browse files
Just-InsaneJust-Insane
committed
refactor+test(threads): extract getThreadReplyEvents and add unit tests
Extract the replyEvents IIFE into a module-level function so the fix-first- then-check-length logic can be exercised in isolated unit tests without mounting the full component. Tests cover: - Returns thread.events (minus root) when the Thread object has replies. - Excludes reactions (RelationType.Annotation) from the results. - Classic-sync regression: thread.events = [rootEvent] => falls back to the live room timeline (the bug was that length-1 thread blocked the fallback). - Falls back to live timeline when getThread() returns null. - Filters out events belonging to a different thread. - Returns [] when neither source has relevant events.
1 parent 2a63f59 commit 86bdb80

3 files changed

Lines changed: 191 additions & 28 deletions

File tree

‎.changeset/fix-thread-classic-sync.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Fix thread drawer showing no messages when using classic sync.

‎src/app/features/room/ThreadDrawer.test.ts‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/**
2+
* Unit tests for getThreadReplyEvents — the function that decides which events
3+
* to display as replies in the thread drawer.
4+
*
5+
* The bug these tests cover (classic-sync empty thread):
6+
* room.createThread(id, root, [], false) creates a Thread whose .events array
7+
* contains only the root event. After filtering out the root, the old code
8+
* checked `if (fromThread.length > 0)` on the un-filtered array — which was
9+
* truthy — and returned an empty list instead of falling back to the live
10+
* timeline. The fix: filter first, then check.
11+
*/
12+
import { describe, it, expect } from 'vitest';
13+
import { RelationType } from '$types/matrix-sdk';
14+
import { getThreadReplyEvents } from './ThreadDrawer';
15+
16+
// ── Minimal MatrixEvent factory ───────────────────────────────────────────────
17+
18+
type EventInit = {
19+
id: string;
20+
threadRootId?: string;
21+
/** When set, the event is treated as a reaction/annotation */
22+
relType?: string;
23+
};
24+
25+
function makeEvent({ id, threadRootId, relType }: EventInit) {
26+
return {
27+
getId: () => id,
28+
threadRootId,
29+
getRelation: () => (relType ? { rel_type: relType } : null),
30+
getContent: () => ({}),
31+
};
32+
}
33+
34+
// ── Minimal Room factory ──────────────────────────────────────────────────────
35+
36+
type RoomInit = {
37+
thread?: { events: ReturnType<typeof makeEvent>[] } | null;
38+
liveEvents?: ReturnType<typeof makeEvent>[];
39+
};
40+
41+
function makeRoom({ thread = null, liveEvents = [] }: RoomInit) {
42+
return {
43+
getThread: () => thread,
44+
getUnfilteredTimelineSet: () => ({
45+
getLiveTimeline: () => ({
46+
getEvents: () => liveEvents,
47+
}),
48+
}),
49+
};
50+
}
51+
52+
// ── Tests ─────────────────────────────────────────────────────────────────────
53+
54+
const ROOT_ID = '$root-event-id';
55+
const REPLY_ID = '$reply-event-id';
56+
const REACTION_ID = '$reaction-event-id';
57+
58+
describe('getThreadReplyEvents', () => {
59+
it('returns thread events minus the root when the Thread object has replies', () => {
60+
const rootEvent = makeEvent({ id: ROOT_ID, threadRootId: ROOT_ID });
61+
const replyEvent = makeEvent({ id: REPLY_ID, threadRootId: ROOT_ID });
62+
63+
const room = makeRoom({
64+
thread: { events: [rootEvent, replyEvent] as any },
65+
liveEvents: [],
66+
});
67+
68+
const result = getThreadReplyEvents(room as any, ROOT_ID);
69+
70+
expect(result).toHaveLength(1);
71+
expect(result[0].getId()).toBe(REPLY_ID);
72+
});
73+
74+
it('excludes reactions from thread events', () => {
75+
const rootEvent = makeEvent({ id: ROOT_ID, threadRootId: ROOT_ID });
76+
const replyEvent = makeEvent({ id: REPLY_ID, threadRootId: ROOT_ID });
77+
const reactionEvent = makeEvent({
78+
id: REACTION_ID,
79+
threadRootId: ROOT_ID,
80+
relType: RelationType.Annotation,
81+
});
82+
83+
const room = makeRoom({
84+
thread: { events: [rootEvent, replyEvent, reactionEvent] as any },
85+
liveEvents: [],
86+
});
87+
88+
const result = getThreadReplyEvents(room as any, ROOT_ID);
89+
90+
expect(result).toHaveLength(1);
91+
expect(result[0].getId()).toBe(REPLY_ID);
92+
});
93+
94+
// ── Classic-sync empty-thread regression ──────────────────────────────────
95+
96+
it('falls back to the live timeline when thread.events contains only the root (classic-sync case)', () => {
97+
// classic sync: thread created with no initialEvents → events = [rootEvent]
98+
const rootEvent = makeEvent({ id: ROOT_ID, threadRootId: ROOT_ID });
99+
const liveReply = makeEvent({ id: REPLY_ID, threadRootId: ROOT_ID });
100+
101+
const room = makeRoom({
102+
thread: { events: [rootEvent] as any },
103+
liveEvents: [liveReply as any],
104+
});
105+
106+
const result = getThreadReplyEvents(room as any, ROOT_ID);
107+
108+
// Without the fix: `fromThread.length > 0` was truthy → returned filtered
109+
// empty array. With the fix: filtered array is empty → falls back to live.
110+
expect(result).toHaveLength(1);
111+
expect(result[0].getId()).toBe(REPLY_ID);
112+
});
113+
114+
it('falls back to the live timeline when there is no Thread object at all', () => {
115+
const liveReply = makeEvent({ id: REPLY_ID, threadRootId: ROOT_ID });
116+
117+
const room = makeRoom({
118+
thread: null,
119+
liveEvents: [liveReply as any],
120+
});
121+
122+
const result = getThreadReplyEvents(room as any, ROOT_ID);
123+
124+
expect(result).toHaveLength(1);
125+
expect(result[0].getId()).toBe(REPLY_ID);
126+
});
127+
128+
it('excludes events from the live timeline that belong to a different thread', () => {
129+
const otherReply = makeEvent({ id: '$other-reply', threadRootId: '$other-root' });
130+
const ourReply = makeEvent({ id: REPLY_ID, threadRootId: ROOT_ID });
131+
132+
const room = makeRoom({
133+
thread: null,
134+
liveEvents: [otherReply as any, ourReply as any],
135+
});
136+
137+
const result = getThreadReplyEvents(room as any, ROOT_ID);
138+
139+
expect(result).toHaveLength(1);
140+
expect(result[0].getId()).toBe(REPLY_ID);
141+
});
142+
143+
it('returns an empty array when neither the thread nor the live timeline has replies', () => {
144+
const rootEvent = makeEvent({ id: ROOT_ID, threadRootId: ROOT_ID });
145+
146+
const room = makeRoom({
147+
thread: { events: [rootEvent] as any },
148+
liveEvents: [],
149+
});
150+
151+
const result = getThreadReplyEvents(room as any, ROOT_ID);
152+
153+
expect(result).toHaveLength(0);
154+
});
155+
});

‎src/app/features/room/ThreadDrawer.tsx‎

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,36 @@ import { RoomInput } from './RoomInput';
5454
import { RoomViewFollowing, RoomViewFollowingPlaceholder } from './RoomViewFollowing';
5555
import * as css from './ThreadDrawer.css';
5656

57+
/**
58+
* Resolve the list of reply events to show in the thread drawer.
59+
*
60+
* Prefers events from the SDK Thread object (authoritative, full history) but
61+
* falls back to scanning the main room timeline when the Thread object was
62+
* created without `initialEvents` (as happens with classic sync). In that
63+
* case `thread.events` contains only the root event, so filtering it yields an
64+
* empty array — we must fall back rather than showing nothing.
65+
*
66+
* Exported for unit testing.
67+
*/
68+
export function getThreadReplyEvents(room: Room, threadRootId: string): MatrixEvent[] {
69+
const thread = room.getThread(threadRootId);
70+
const fromThread = thread?.events ?? [];
71+
const filteredFromThread = fromThread.filter(
72+
(ev) => ev.getId() !== threadRootId && !reactionOrEditEvent(ev)
73+
);
74+
if (filteredFromThread.length > 0) {
75+
return filteredFromThread;
76+
}
77+
return room
78+
.getUnfilteredTimelineSet()
79+
.getLiveTimeline()
80+
.getEvents()
81+
.filter(
82+
(ev) =>
83+
ev.threadRootId === threadRootId && ev.getId() !== threadRootId && !reactionOrEditEvent(ev)
84+
);
85+
}
86+
5787
type ForwardedMessageProps = {
5888
isForwarded: boolean;
5989
originalTimestamp: number;
@@ -508,34 +538,7 @@ export function ThreadDrawer({ room, threadRootId, onClose, overlay }: ThreadDra
508538
markThreadAsRead();
509539
}, [mx, room, threadRootId, forceUpdate]);
510540

511-
// Use the Thread object if available (authoritative source with full history).
512-
// Fall back to scanning the live room timeline for local echoes and the
513-
// window before the Thread object is registered by the SDK.
514-
// NOTE: With classic sync the Thread object is created via room.createThread()
515-
// with empty initialEvents, so thread.events may only contain the root event.
516-
// We must filter first, then decide whether to fall back — otherwise a thread
517-
// whose events array consists solely of the root event (length === 1) prevents
518-
// the live-timeline fallback from running, and the drawer shows nothing.
519-
const replyEvents: MatrixEvent[] = (() => {
520-
const thread = room.getThread(threadRootId);
521-
const fromThread = thread?.events ?? [];
522-
const filteredFromThread = fromThread.filter(
523-
(ev) => ev.getId() !== threadRootId && !reactionOrEditEvent(ev)
524-
);
525-
if (filteredFromThread.length > 0) {
526-
return filteredFromThread;
527-
}
528-
return room
529-
.getUnfilteredTimelineSet()
530-
.getLiveTimeline()
531-
.getEvents()
532-
.filter(
533-
(ev) =>
534-
ev.threadRootId === threadRootId &&
535-
ev.getId() !== threadRootId &&
536-
!reactionOrEditEvent(ev)
537-
);
538-
})();
541+
const replyEvents = getThreadReplyEvents(room, threadRootId);
539542

540543
replyEventsRef.current = replyEvents;
541544

0 commit comments

Comments
 (0)