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

Fix synthesizeReceipt #2916

Merged
merged 10 commits into from
Nov 30, 2022
20 changes: 20 additions & 0 deletions spec/unit/notifications.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,16 @@ describe("fixNotificationCountOnDecryption", () => {
expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(1);
});

it("does not change the room count when there's no unread count", () => {
room.setUnreadNotificationCount(NotificationCountType.Total, 0);
room.setUnreadNotificationCount(NotificationCountType.Highlight, 0);

fixNotificationCountOnDecryption(mockClient, event);

expect(room.getRoomUnreadNotificationCount(NotificationCountType.Total)).toBe(0);
expect(room.getRoomUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0);
});

it("changes the thread count to highlight on decryption", () => {
expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(1);
expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0);
Expand All @@ -118,6 +128,16 @@ describe("fixNotificationCountOnDecryption", () => {
expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(1);
});

it("does not change the room count when there's no unread count", () => {
room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 0);
room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0);

fixNotificationCountOnDecryption(mockClient, event);

expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(0);
expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(0);
});

it("emits events", () => {
const cb = jest.fn();
room.on(RoomEvent.UnreadNotifications, cb);
Expand Down
20 changes: 20 additions & 0 deletions spec/unit/read-receipt.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { MAIN_ROOM_TIMELINE, ReceiptType } from '../../src/@types/read_receipts'
import { MatrixClient } from "../../src/client";
import { Feature, ServerSupport } from '../../src/feature';
import { EventType } from '../../src/matrix';
import { synthesizeReceipt } from '../../src/models/read-receipt';
import { encodeUri } from '../../src/utils';
import * as utils from "../test-utils/test-utils";

Expand Down Expand Up @@ -175,4 +176,23 @@ describe("Read receipt", () => {
await flushPromises();
});
});

describe("synthesizeReceipt", () => {
it.each([
{ event: roomEvent, destinationId: MAIN_ROOM_TIMELINE },
{ event: threadEvent, destinationId: threadEvent.threadRootId! }
])("adds the receipt to $destinationId", ({ event, destinationId }) => {
const userId = "@bob:example.org";
const receiptType = ReceiptType.Read;

const fakeReadReceipt = synthesizeReceipt(userId, event, receiptType);

const content = fakeReadReceipt.getContent()
[event.getId()!]
[receiptType]
[userId];

expect(content.thread_id).toEqual(destinationId);
});
});
});
36 changes: 26 additions & 10 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5830,8 +5830,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
).then(async (res) => {
const mapper = this.getEventMapper();
const matrixEvents = res.chunk.map(mapper);
for (const event of matrixEvents) {
await eventTimeline.getTimelineSet()?.thread?.processEvent(event);

// Process latest events first
for (const event of matrixEvents.slice().reverse()) {
await thread?.processEvent(event);
const sender = event.getSender()!;
if (!backwards || thread?.getEventReadUpTo(sender) === null) {
room.addLocalEchoReceipt(sender, event, ReceiptType.Read);
}
}

const newToken = res.next_batch;
Expand Down Expand Up @@ -9325,6 +9331,19 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
}
}

export function getCount({ type, room, event }: {
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
type: NotificationCountType; room: Room; event: MatrixEvent;
}): number {
const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;

return (isThreadEvent
? room.getThreadUnreadNotificationCount(
event.threadRootId,
type,
)
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
: room.getRoomUnreadNotificationCount(type)) ?? 0;
}

/**
* recalculates an accurate notifications count on event decryption.
* Servers do not have enough knowledge about encrypted events to calculate an
Expand All @@ -9338,19 +9357,16 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
if (!room || !cli.getUserId()) return;

const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;
const currentCount = (isThreadEvent
? room.getThreadUnreadNotificationCount(
event.threadRootId,
NotificationCountType.Highlight,
)
: room.getUnreadNotificationCount(NotificationCountType.Highlight)) ?? 0;

const totalCount = getCount({ type: NotificationCountType.Total, room, event });
const currentCount = getCount({ type: NotificationCountType.Highlight, room, event });

// Ensure the unread counts are kept up to date if the event is encrypted
// We also want to make sure that the notification count goes up if we already
// have encrypted events to avoid other code from resetting 'highlight' to zero.
const oldHighlight = !!oldActions?.tweaks?.highlight;
const newHighlight = !!actions?.tweaks?.highlight;
if (oldHighlight !== newHighlight || currentCount > 0) {
if ((oldHighlight !== newHighlight || currentCount > 0) && totalCount > 0) {
// TODO: Handle mentions received while the client is offline
// See also https://github.com/vector-im/element-web/issues/9069
const hasReadEvent = isThreadEvent
Expand All @@ -9375,7 +9391,7 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
// Fix 'Mentions Only' rooms from not having the right badge count
const totalCount = (isThreadEvent
? room.getThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Total)
: room.getUnreadNotificationCount(NotificationCountType.Total)) ?? 0;
: room.getRoomUnreadNotificationCount(NotificationCountType.Total)) ?? 0;

if (totalCount < newCount) {
if (isThreadEvent) {
Expand Down
2 changes: 1 addition & 1 deletion src/models/read-receipt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function synthesizeReceipt(userId: string, event: MatrixEvent, receiptTyp
[receiptType]: {
[userId]: {
ts: event.getTs(),
threadId: event.threadRootId ?? MAIN_ROOM_TIMELINE,
thread_id: event.threadRootId ?? MAIN_ROOM_TIMELINE,
},
},
},
Expand Down
23 changes: 15 additions & 8 deletions src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
* for this type.
*/
public getUnreadNotificationCount(type = NotificationCountType.Total): number {
let count = this.notificationCounts[type] ?? 0;
let count = this.getRoomUnreadNotificationCount(type);
if (this.client.canSupport.get(Feature.ThreadUnreadNotifications) !== ServerSupport.Unsupported) {
for (const threadNotification of this.threadNotifications.values()) {
count += threadNotification[type] ?? 0;
Expand All @@ -1192,6 +1192,16 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
return count;
}

/**
* Get one of the notification counts for this room
* @param {String} type The type of notification count to get. default: 'total'
* @return {Number} The notification count, or undefined if there is no count
* for this type.
*/
public getRoomUnreadNotificationCount(type = NotificationCountType.Total): number {
return this.notificationCounts[type] ?? 0;
}

/**
* @experimental
* Get one of the notification counts for a thread
Expand Down Expand Up @@ -1758,20 +1768,17 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
let latestMyThreadsRootEvent: MatrixEvent | undefined;
const roomState = this.getLiveTimeline().getState(EventTimeline.FORWARDS);
for (const rootEvent of threadRoots) {
this.threadsTimelineSets[0]?.addLiveEvent(rootEvent, {
const opts = {
duplicateStrategy: DuplicateStrategy.Ignore,
fromCache: false,
roomState,
});
};
this.threadsTimelineSets[0]?.addLiveEvent(rootEvent, opts);

const threadRelationship = rootEvent
.getServerAggregatedRelation<IThreadBundledRelationship>(THREAD_RELATION_TYPE.name);
if (threadRelationship?.current_user_participated) {
this.threadsTimelineSets[1]?.addLiveEvent(rootEvent, {
duplicateStrategy: DuplicateStrategy.Ignore,
fromCache: false,
roomState,
});
this.threadsTimelineSets[1]?.addLiveEvent(rootEvent, opts);
latestMyThreadsRootEvent = rootEvent;
}
}
Expand Down
14 changes: 13 additions & 1 deletion src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
this.room.on(MatrixEventEvent.BeforeRedaction, this.onBeforeRedaction);
this.room.on(RoomEvent.Redaction, this.onRedaction);
this.room.on(RoomEvent.LocalEchoUpdated, this.onEcho);
this.timelineSet.on(RoomEvent.Timeline, this.onEcho);
this.timelineSet.on(RoomEvent.Timeline, this.onTimelineEvent);

// even if this thread is thought to be originating from this client, we initialise it as we may be in a
// gappy sync and a thread around this event may already exist.
Expand Down Expand Up @@ -192,6 +192,18 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
}
};

private onTimelineEvent = (
event: MatrixEvent,
room: Room | undefined,
toStartOfTimeline: boolean | undefined,
): void => {
// Add a synthesized receipt to all live events added to the timeline
if (!toStartOfTimeline) {
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
room!.addLocalEchoReceipt(event.getSender()!, event, ReceiptType.Read);
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
}
this.onEcho(event);
};

private onEcho = async (event: MatrixEvent): Promise<void> => {
if (event.threadRootId !== this.id) return; // ignore echoes for other timelines
if (this.lastEvent === event) return;
Expand Down