Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update threads handling for replies-to-thread-responses as per MSC update #2305

Merged
merged 2 commits into from
Apr 19, 2022
Merged

Update threads handling for replies-to-thread-responses as per MSC update #2305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8168480
Update threads handling for replies-to-thread-responses as per MSC up…
t3chguy Apr 19, 2022
9260ae5
Update tests to match new behaviour
t3chguy Apr 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions spec/integ/matrix-client-methods.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ describe("MatrixClient", function() {
]);
});

it("sends reply to thread responses to thread timeline only", () => {
it("sends reply to thread responses to main timeline only", () => {
client.clientOpts = { experimentalThreadSupport: true };

const threadRootEvent = buildEventPollStartThreadRoot();
Expand All @@ -814,12 +814,12 @@ describe("MatrixClient", function() {

expect(timeline).toEqual([
threadRootEvent,
replyToThreadResponse,
]);

expect(threaded).toEqual([
threadRootEvent,
eventMessageInThread,
replyToThreadResponse,
]);
});
});
Expand Down
34 changes: 15 additions & 19 deletions spec/unit/room.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2154,36 +2154,32 @@ describe("Room", function() {
expect(room.eventShouldLiveIn(threadReaction2Redaction, events, roots).threadId).toBe(threadRoot.getId());
});

it("reply to thread response and its relations&redactions should be only in thread timeline", () => {
it("reply to thread response and its relations&redactions should be only in main timeline", () => {
const threadRoot = mkMessage();
const threadResponse1 = mkThreadResponse(threadRoot);
const reply1 = mkReply(threadResponse1);
const threadReaction1 = mkReaction(reply1);
const threadReaction2 = mkReaction(reply1);
const threadReaction2Redaction = mkRedaction(reply1);
const reaction1 = mkReaction(reply1);
const reaction2 = mkReaction(reply1);
const reaction2Redaction = mkRedaction(reply1);

const roots = new Set([threadRoot.getId()]);
const events = [
threadRoot,
threadResponse1,
reply1,
threadReaction1,
threadReaction2,
threadReaction2Redaction,
reaction1,
reaction2,
reaction2Redaction,
];

expect(room.eventShouldLiveIn(reply1, events, roots).shouldLiveInRoom).toBeFalsy();
expect(room.eventShouldLiveIn(reply1, events, roots).shouldLiveInThread).toBeTruthy();
expect(room.eventShouldLiveIn(reply1, events, roots).threadId).toBe(threadRoot.getId());
expect(room.eventShouldLiveIn(threadReaction1, events, roots).shouldLiveInRoom).toBeFalsy();
expect(room.eventShouldLiveIn(threadReaction1, events, roots).shouldLiveInThread).toBeTruthy();
expect(room.eventShouldLiveIn(threadReaction1, events, roots).threadId).toBe(threadRoot.getId());
expect(room.eventShouldLiveIn(threadReaction2, events, roots).shouldLiveInRoom).toBeFalsy();
expect(room.eventShouldLiveIn(threadReaction2, events, roots).shouldLiveInThread).toBeTruthy();
expect(room.eventShouldLiveIn(threadReaction2, events, roots).threadId).toBe(threadRoot.getId());
expect(room.eventShouldLiveIn(threadReaction2Redaction, events, roots).shouldLiveInRoom).toBeFalsy();
expect(room.eventShouldLiveIn(threadReaction2Redaction, events, roots).shouldLiveInThread).toBeTruthy();
expect(room.eventShouldLiveIn(threadReaction2Redaction, events, roots).threadId).toBe(threadRoot.getId());
expect(room.eventShouldLiveIn(reply1, events, roots).shouldLiveInRoom).toBeTruthy();
expect(room.eventShouldLiveIn(reply1, events, roots).shouldLiveInThread).toBeFalsy();
expect(room.eventShouldLiveIn(reaction1, events, roots).shouldLiveInRoom).toBeTruthy();
expect(room.eventShouldLiveIn(reaction1, events, roots).shouldLiveInThread).toBeFalsy();
expect(room.eventShouldLiveIn(reaction2, events, roots).shouldLiveInRoom).toBeTruthy();
expect(room.eventShouldLiveIn(reaction2, events, roots).shouldLiveInThread).toBeFalsy();
expect(room.eventShouldLiveIn(reaction2Redaction, events, roots).shouldLiveInRoom).toBeTruthy();
expect(room.eventShouldLiveIn(reaction2Redaction, events, roots).shouldLiveInThread).toBeFalsy();
});

it("reply to reply to thread root should only be in the main timeline", () => {
Expand Down
10 changes: 0 additions & 10 deletions src/models/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1612,16 +1612,6 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
};
}

// A reply directly to a thread response is shown as part of the thread only, this is to provide a better
// experience when communicating with users using clients without full threads support
if (parentEvent?.isThreadRelation) {
return {
shouldLiveInRoom: false,
shouldLiveInThread: true,
threadId: parentEvent.threadRootId,
};
}

// We've exhausted all scenarios, can safely assume that this event should live in the room timeline only
return {
shouldLiveInRoom: true,
Expand Down