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
Merged
24 changes: 22 additions & 2 deletions spec/unit/notifications.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ let event: MatrixEvent;
let threadEvent: MatrixEvent;

const ROOM_ID = "!roomId:example.org";
let THREAD_ID;
let THREAD_ID: string;

function mkPushAction(notify, highlight): IActionsObject {
return {
Expand Down Expand Up @@ -76,7 +76,7 @@ describe("fixNotificationCountOnDecryption", () => {
event: true,
}, mockClient);

THREAD_ID = event.getId();
THREAD_ID = event.getId()!;
threadEvent = mkEvent({
type: EventType.RoomMessage,
content: {
Expand Down 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
17 changes: 17 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,20 @@ 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);
});
});
});
23 changes: 13 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 @@ -9338,19 +9344,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 = room.getUnreadCountForEventContext(NotificationCountType.Total, event);
const currentCount = room.getUnreadCountForEventContext(NotificationCountType.Highlight, 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 +9378,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
34 changes: 26 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,27 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
return count;
}

/**
* Get the notification for the event context (room or thread timeline)
*/
public getUnreadCountForEventContext(type = NotificationCountType.Total, event: MatrixEvent): number {
const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;

return (isThreadEvent
? this.getThreadUnreadNotificationCount(event.threadRootId, type)
: this.getRoomUnreadNotificationCount(type)) ?? 0;
}

/**
* 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 +1779,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 when paginating forward in the timeline
if (!toStartOfTimeline) {
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