Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Loading threads with server-side assistance (#9356)
Browse files Browse the repository at this point in the history
* Fix bug with message context menu
* fix bug where ThreadSummary failed if no last reply is available
* Fix relations direction API
* Use same API for threads as for any other timeline
* Determine if event belongs to thread on jumping to event
* properly listen to thread deletion
* Add thread redaction tests
* Add fetchInitialEvent tests
* Paginate using default TimelinePanel behaviour
* Remove unused threads deleted code

Co-authored-by: Germain <[email protected]>
Co-authored-by: Germain <[email protected]>
  • Loading branch information
3 people authored Oct 28, 2022
1 parent 750ca78 commit d92fdc1
Show file tree
Hide file tree
Showing 11 changed files with 205 additions and 82 deletions.
42 changes: 1 addition & 41 deletions src/components/structures/ThreadView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ import React, { createRef, KeyboardEvent } from 'react';
import { Thread, THREAD_RELATION_TYPE, ThreadEvent } from 'matrix-js-sdk/src/models/thread';
import { Room, RoomEvent } from 'matrix-js-sdk/src/models/room';
import { IEventRelation, MatrixEvent } from 'matrix-js-sdk/src/models/event';
import { TimelineWindow } from 'matrix-js-sdk/src/timeline-window';
import { Direction } from 'matrix-js-sdk/src/models/event-timeline';
import { IRelationsRequestOpts } from 'matrix-js-sdk/src/@types/requests';
import { logger } from 'matrix-js-sdk/src/logger';
import classNames from 'classnames';

Expand Down Expand Up @@ -236,10 +233,8 @@ export default class ThreadView extends React.Component<IProps, IState> {
thread_id: thread.id,
});
thread.emit(ThreadEvent.ViewThread);
await thread.fetchInitialEvents();
this.updateThreadRelation();
this.nextBatch = thread.liveTimeline.getPaginationToken(Direction.Backward);
this.timelinePanel.current?.refreshTimeline();
this.timelinePanel.current?.refreshTimeline(this.props.initialEvent?.getId());
}

private setupThreadListeners(thread?: Thread | undefined, oldThread?: Thread | undefined): void {
Expand Down Expand Up @@ -293,40 +288,6 @@ export default class ThreadView extends React.Component<IProps, IState> {
}
};

private nextBatch: string | undefined | null = null;

private onPaginationRequest = async (
timelineWindow: TimelineWindow | null,
direction = Direction.Backward,
limit = 20,
): Promise<boolean> => {
if (!Thread.hasServerSideSupport && timelineWindow) {
timelineWindow.extend(direction, limit);
return true;
}

const opts: IRelationsRequestOpts = {
limit,
};

if (this.nextBatch) {
opts.from = this.nextBatch;
}

let nextBatch: string | null | undefined = null;
if (this.state.thread) {
const response = await this.state.thread.fetchEvents(opts);
nextBatch = response.nextBatch;
this.nextBatch = nextBatch;
}

// Advances the marker on the TimelineWindow to define the correct
// window of events to display on screen
timelineWindow?.extend(direction, limit);

return !!nextBatch;
};

private onFileDrop = (dataTransfer: DataTransfer) => {
const roomId = this.props.mxEvent.getRoomId();
if (roomId) {
Expand Down Expand Up @@ -409,7 +370,6 @@ export default class ThreadView extends React.Component<IProps, IState> {
highlightedEventId={highlightedEventId}
eventScrollIntoView={this.props.initialEventScrollIntoView}
onEventScrolledIntoView={this.resetJumpToEvent}
onPaginationRequest={this.onPaginationRequest}
/>
</>;
} else {
Expand Down
40 changes: 22 additions & 18 deletions src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1409,24 +1409,28 @@ class TimelinePanel extends React.Component<IProps, IState> {
// quite slow. So we detect that situation and shortcut straight to
// calling _reloadEvents and updating the state.

const timeline = this.props.timelineSet.getTimelineForEvent(eventId);
if (timeline) {
// This is a hot-path optimization by skipping a promise tick
// by repeating a no-op sync branch in TimelineSet.getTimelineForEvent & MatrixClient.getEventTimeline
this.timelineWindow.load(eventId, INITIAL_SIZE); // in this branch this method will happen in sync time
// This is a hot-path optimization by skipping a promise tick
// by repeating a no-op sync branch in
// TimelineSet.getTimelineForEvent & MatrixClient.getEventTimeline
if (this.props.timelineSet.getTimelineForEvent(eventId)) {
// if we've got an eventId, and the timeline exists, we can skip
// the promise tick.
this.timelineWindow.load(eventId, INITIAL_SIZE);
// in this branch this method will happen in sync time
onLoaded();
} else {
const prom = this.timelineWindow.load(eventId, INITIAL_SIZE);
this.buildLegacyCallEventGroupers();
this.setState({
events: [],
liveEvents: [],
canBackPaginate: false,
canForwardPaginate: false,
timelineLoading: true,
});
prom.then(onLoaded, onError);
return;
}

const prom = this.timelineWindow.load(eventId, INITIAL_SIZE);
this.buildLegacyCallEventGroupers();
this.setState({
events: [],
liveEvents: [],
canBackPaginate: false,
canForwardPaginate: false,
timelineLoading: true,
});
prom.then(onLoaded, onError);
}

// handle the completion of a timeline load or localEchoUpdate, by
Expand All @@ -1443,8 +1447,8 @@ class TimelinePanel extends React.Component<IProps, IState> {
}

// Force refresh the timeline before threads support pending events
public refreshTimeline(): void {
this.loadTimeline();
public refreshTimeline(eventId?: string): void {
this.loadTimeline(eventId, undefined, undefined, false);
this.reloadEvents();
}

Expand Down
10 changes: 8 additions & 2 deletions src/components/views/context_menus/MessageContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,13 @@ export default class MessageContextMenu extends React.Component<IProps, IState>
public render(): JSX.Element {
const cli = MatrixClientPeg.get();
const me = cli.getUserId();
const { mxEvent, rightClick, link, eventTileOps, reactions, collapseReplyChain } = this.props;
const {
mxEvent, rightClick, link, eventTileOps, reactions, collapseReplyChain,
...other
} = this.props;
delete other.getRelationsForEvent;
delete other.permalinkCreator;

const eventStatus = mxEvent.status;
const unsentReactionsCount = this.getUnsentReactions().length;
const contentActionable = isContentActionable(mxEvent);
Expand Down Expand Up @@ -747,7 +753,7 @@ export default class MessageContextMenu extends React.Component<IProps, IState>
return (
<React.Fragment>
<IconizedContextMenu
{...this.props}
{...other}
className="mx_MessageContextMenu"
compact={true}
data-testid="mx_MessageContextMenu"
Expand Down
14 changes: 10 additions & 4 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,11 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>

private renderThreadInfo(): React.ReactNode {
if (this.state.thread?.id === this.props.mxEvent.getId()) {
return <ThreadSummary mxEvent={this.props.mxEvent} thread={this.state.thread} />;
return <ThreadSummary
mxEvent={this.props.mxEvent}
thread={this.state.thread}
data-testid="thread-summary"
/>;
}

if (this.context.timelineRenderingType === TimelineRenderingType.Search && this.props.mxEvent.threadRootId) {
Expand Down Expand Up @@ -1528,9 +1532,11 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>

// Wrap all event tiles with the tile error boundary so that any throws even during construction are captured
const SafeEventTile = forwardRef((props: EventTileProps, ref: RefObject<UnwrappedEventTile>) => {
return <TileErrorBoundary mxEvent={props.mxEvent} layout={props.layout}>
<UnwrappedEventTile ref={ref} {...props} />
</TileErrorBoundary>;
return <>
<TileErrorBoundary mxEvent={props.mxEvent} layout={props.layout}>
<UnwrappedEventTile ref={ref} {...props} />
</TileErrorBoundary>
</>;
});
export default SafeEventTile;

Expand Down
7 changes: 5 additions & 2 deletions src/components/views/rooms/ThreadSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ interface IProps {
thread: Thread;
}

const ThreadSummary = ({ mxEvent, thread }: IProps) => {
const ThreadSummary = ({ mxEvent, thread, ...props }: IProps) => {
const roomContext = useContext(RoomContext);
const cardContext = useContext(CardContext);
const count = useTypedEventEmitterState(thread, ThreadEvent.Update, () => thread.length);
Expand All @@ -50,6 +50,7 @@ const ThreadSummary = ({ mxEvent, thread }: IProps) => {

return (
<AccessibleButton
{...props}
className="mx_ThreadSummary"
onClick={(ev: ButtonEvent) => {
defaultDispatcher.dispatch<ShowThreadPayload>({
Expand Down Expand Up @@ -94,7 +95,9 @@ export const ThreadMessagePreview = ({ thread, showDisplayname = false }: IPrevi
await cli.decryptEventIfNeeded(lastReply);
return MessagePreviewStore.instance.generatePreviewForEvent(lastReply);
}, [lastReply, content]);
if (!preview) return null;
if (!preview || !lastReply) {
return null;
}

return <>
<MemberAvatar
Expand Down
8 changes: 2 additions & 6 deletions src/stores/widgets/StopGapWidgetDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,8 @@ export class StopGapWidgetDriver extends WidgetDriver {
eventId,
relationType ?? null,
eventType ?? null,
{
from,
to,
limit,
dir,
});
{ from, to, limit, dir },
);

return {
chunk: events.map(e => e.getEffectiveEvent() as IRoomEvent),
Expand Down
5 changes: 4 additions & 1 deletion src/utils/EventUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,11 @@ export async function fetchInitialEvent(
) {
const threadId = initialEvent.threadRootId;
const room = client.getRoom(roomId);
const mapper = client.getEventMapper();
const rootEvent = room.findEventById(threadId)
?? mapper(await client.fetchRoomEvent(roomId, threadId));
try {
room.createThread(threadId, room.findEventById(threadId), [initialEvent], true);
room.createThread(threadId, rootEvent, [initialEvent], true);
} catch (e) {
logger.warn("Could not find root event: " + threadId);
}
Expand Down
53 changes: 47 additions & 6 deletions test/components/views/rooms/EventTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,19 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { act, render } from "@testing-library/react";
import React from "react";
import { act, render, screen, waitFor } from "@testing-library/react";
import { EventType } from "matrix-js-sdk/src/@types/event";
import { MatrixClient, PendingEventOrdering } from "matrix-js-sdk/src/client";
import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room";
import React from "react";

import EventTile, { EventTileProps } from "../../../../src/components/views/rooms/EventTile";
import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";
import RoomContext, { TimelineRenderingType } from "../../../../src/contexts/RoomContext";
import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
import { getRoomContext, mkMessage, stubClient } from "../../../test-utils";
import SettingsStore from "../../../../src/settings/SettingsStore";
import { getRoomContext, mkEvent, mkMessage, stubClient } from "../../../test-utils";
import { mkThread } from "../../../test-utils/threads";

describe("EventTile", () => {
Expand Down Expand Up @@ -52,9 +55,11 @@ describe("EventTile", () => {
timelineRenderingType: renderingType,
});
return render(
<RoomContext.Provider value={context}>
<TestEventTile {...overrides} />
</RoomContext.Provider>,
<MatrixClientContext.Provider value={client}>
<RoomContext.Provider value={context}>
<TestEventTile {...overrides} />
</RoomContext.Provider>,
</MatrixClientContext.Provider>,
);
}

Expand All @@ -69,6 +74,8 @@ describe("EventTile", () => {
});

jest.spyOn(client, "getRoom").mockReturnValue(room);
jest.spyOn(client, "decryptEventIfNeeded").mockResolvedValue();
jest.spyOn(SettingsStore, "getValue").mockImplementation(name => name === "feature_thread");

mxEvent = mkMessage({
room: room.roomId,
Expand All @@ -78,6 +85,40 @@ describe("EventTile", () => {
});
});

describe("EventTile thread summary", () => {
beforeEach(() => {
jest.spyOn(client, "supportsExperimentalThreads").mockReturnValue(true);
});

it("removes the thread summary when thread is deleted", async () => {
const { rootEvent, events: [, reply] } = mkThread({
room,
client,
authorId: "@alice:example.org",
participantUserIds: ["@alice:example.org"],
length: 2, // root + 1 answer
});
getComponent({
mxEvent: rootEvent,
}, TimelineRenderingType.Room);

await waitFor(() => expect(screen.queryByTestId("thread-summary")).not.toBeNull());

const redaction = mkEvent({
event: true,
type: EventType.RoomRedaction,
user: "@alice:example.org",
room: room.roomId,
redacts: reply.getId(),
content: {},
});

act(() => room.processThreadedEvents([redaction], false));

await waitFor(() => expect(screen.queryByTestId("thread-summary")).toBeNull());
});
});

describe("EventTile renderingType: ThreadsList", () => {
beforeEach(() => {
const { rootEvent } = mkThread({
Expand Down
2 changes: 2 additions & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ type MakeEventPassThruProps = {
};
type MakeEventProps = MakeEventPassThruProps & {
type: string;
redacts?: string;
content: IContent;
room?: Room["roomId"]; // to-device messages are roomless
// eslint-disable-next-line camelcase
Expand Down Expand Up @@ -245,6 +246,7 @@ export function mkEvent(opts: MakeEventProps): MatrixEvent {
event_id: "$" + Math.random() + "-" + Math.random(),
origin_server_ts: opts.ts ?? 0,
unsigned: opts.unsigned,
redacts: opts.redacts,
};
if (opts.skey !== undefined) {
event.state_key = opts.skey;
Expand Down
10 changes: 9 additions & 1 deletion test/test-utils/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { MatrixClient, MatrixEvent, RelationType, Room } from "matrix-js-sdk/src/matrix";
import { MatrixClient, MatrixEvent, MatrixEventEvent, RelationType, Room } from "matrix-js-sdk/src/matrix";
import { Thread } from "matrix-js-sdk/src/models/thread";

import { mkMessage, MessageEventProps } from "./test-utils";
Expand Down Expand Up @@ -115,10 +115,18 @@ export const mkThread = ({
ts,
currentUserId: client.getUserId(),
});
expect(rootEvent).toBeTruthy();

for (const evt of events) {
room?.reEmitter.reEmit(evt, [
MatrixEventEvent.BeforeRedaction,
]);
}

const thread = room.createThread(rootEvent.getId(), rootEvent, events, true);
// So that we do not have to mock the thread loading
thread.initialEventsFetched = true;
thread.addEvents(events, true);

return { thread, rootEvent, events };
};
Loading

0 comments on commit d92fdc1

Please sign in to comment.