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.

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

Only add a local receipt if it's after an existing receipt #3399

Merged
Merged
Changes from 1 commit
Commits
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
54 changes: 51 additions & 3 deletions spec/unit/models/thread.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,11 +503,59 @@ describe("Thread", () => {
const userId = "user1";
const { thread } = await createThreadAndEvent(client, 200, 100, userId);

// Then no receipt was added to the thread (the receipt is still for the thread root).
// Then no receipt was added to the thread (the receipt is still
// for the thread root). This happens because since we have no
// recursive relations support, we know that sometimes events
// appear out of order, so we have to check their timestamps as
// a guess of the correct order.
expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(thread.rootEvent?.getId());
});
});

describe("With relations recursion support", () => {
it("Creates a local echo receipt for new events", async () => {
// Assumption: no server side support because if we have it, events
// can only be added to the timeline after the thread has been
// initialised, and we are not properly initialising it here.
expect(Thread.hasServerSideSupport).toBe(FeatureSupport.None);

// Given a client WITH relations recursion support
const client = createClientWithEventMapper(
new Map([[Feature.RelationsRecursion, ServerSupport.Stable]]),
);
Comment on lines +516 to +525
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could go to a beforeEach block because it is shared code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// And a thread with an added event (with later timestamp)
const userId = "user1";
const { thread, message } = await createThreadAndEvent(client, 1, 100, userId);

// Then a receipt was added to the thread
const receipt = thread.getReadReceiptForUserId(userId);
expect(receipt?.eventId).toEqual(message.getId());
});

it("Creates a local echo receipt even for events BEFORE an existing receipt", async () => {
// Assumption: no server side support because if we have it, events
// can only be added to the timeline after the thread has been
// initialised, and we are not properly initialising it here.
expect(Thread.hasServerSideSupport).toBe(FeatureSupport.None);

// Given a client WITH relations recursion support
const client = createClientWithEventMapper(
new Map([[Feature.RelationsRecursion, ServerSupport.Stable]]),
);

// And a thread with an added event with a lower timestamp than its other events
const userId = "user1";
const { thread, message } = await createThreadAndEvent(client, 200, 100, userId);

// Then a receipt was added to the thread, because relations
// recursion is available, so we trust the server to have
// provided us with events in the right order.
const receipt = thread.getReadReceiptForUserId(userId);
expect(receipt?.eventId).toEqual(message.getId());
});
});

async function createThreadAndEvent(
client: MatrixClient,
rootTs: number,
Expand Down Expand Up @@ -544,10 +592,10 @@ describe("Thread", () => {
return { thread, message };
}

function createClientWithEventMapper(): MatrixClient {
function createClientWithEventMapper(canSupport: Map<Feature, ServerSupport> = new Map()): MatrixClient {
const client = mock(MatrixClient, "MatrixClient");
client.reEmitter = mock(ReEmitter, "ReEmitter");
client.canSupport = new Map();
client.canSupport = canSupport;
jest.spyOn(client, "getEventMapper").mockReturnValue(eventMapperFor(client, {}));
mocked(client.supportsThreads).mockReturnValue(true);
return client;
Expand Down