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

Threads notification proof of concept (MSC3773) #2600

Closed
wants to merge 7 commits into from
Closed

Threads notification proof of concept (MSC3773) #2600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d5116d1
MSC3773 POC
Aug 18, 2022
c1c5984
Merge branch 'gsouquet/threads-rr-poc' into gsouquet/threads-notifica…
Aug 18, 2022
2e64bce
Merge branch 'gsouquet/threads-rr-poc' into gsouquet/threads-notifica…
Aug 18, 2022
c10c81a
Setup sync filter properly
Aug 18, 2022
190fea1
Fix unread room state updating
Aug 18, 2022
66f7cdb
Merge branch 'gsouquet/threads-rr-poc' into gsouquet/threads-notifica…
Aug 18, 2022
e5edf5b
Add e2ee support for notifications highlight
Aug 23, 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
95 changes: 64 additions & 31 deletions src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,37 +1048,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
// actions for themselves, so we have to kinda help them out when they are encrypted.
// We do this so that push rules are correctly executed on events in their decrypted
// state, such as highlights when the user's name is mentioned.
this.on(MatrixEventEvent.Decrypted, (event) => {
const oldActions = event.getPushActions();
const actions = this.getPushActionsForEvent(event, true);

const room = this.getRoom(event.getRoomId());
if (!room) return;

const currentCount = room.getUnreadNotificationCount(NotificationCountType.Highlight);

// 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) {
// TODO: Handle mentions received while the client is offline
// See also https://github.com/vector-im/element-web/issues/9069
if (!room.hasUserReadEvent(this.getUserId(), event.getId())) {
let newCount = currentCount;
if (newHighlight && !oldHighlight) newCount++;
if (!newHighlight && oldHighlight) newCount--;
room.setUnreadNotificationCount(NotificationCountType.Highlight, newCount);

// Fix 'Mentions Only' rooms from not having the right badge count
const totalCount = room.getUnreadNotificationCount(NotificationCountType.Total);
if (totalCount < newCount) {
room.setUnreadNotificationCount(NotificationCountType.Total, newCount);
}
}
}
});
this.on(MatrixEventEvent.Decrypted, this.recalculateNotifications);

// Like above, we have to listen for read receipts from ourselves in order to
// correctly handle notification counts on encrypted rooms.
Expand Down Expand Up @@ -1130,6 +1100,69 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
});
}

private recalculateNotifications(event: MatrixEvent): void {
const oldActions = event.getPushActions();
const actions = this.getPushActionsForEvent(event, true);

const room = this.getRoom(event.getRoomId());
if (!room) return;

const isThreadEvent = !!event.threadRootId;
let currentCount;

if (isThreadEvent) {
currentCount = room.getThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Highlight);
} else {
currentCount = room.getUnreadNotificationCount(NotificationCountType.Highlight);
}

// 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) {
// TODO: Handle mentions received while the client is offline
// See also https://github.com/vector-im/element-web/issues/9069
const hasReadEvent = isThreadEvent
? room.getThread(event.threadRootId).hasUserReadEvent(this.getUserId(), event.getId())
: room.hasUserReadEvent(this.getUserId(), event.getId());

if (!hasReadEvent) {
let newCount = currentCount;
if (newHighlight && !oldHighlight) newCount++;
if (!newHighlight && oldHighlight) newCount--;

if (isThreadEvent) {
room.setThreadUnreadNotificationCount(
event.threadRootId,
NotificationCountType.Highlight,
newCount,
);
} else {
room.setUnreadNotificationCount(NotificationCountType.Highlight, newCount);
}

// Fix 'Mentions Only' rooms from not having the right badge count
const totalCount = isThreadEvent
? room.getThreadUnreadNotificationCount(event.threadRootId, NotificationCountType.Total)
: room.getUnreadNotificationCount(NotificationCountType.Total);

if (totalCount < newCount) {
if (isThreadEvent) {
room.setThreadUnreadNotificationCount(
event.threadRootId,
NotificationCountType.Total,
newCount,
);
} else {
room.setUnreadNotificationCount(NotificationCountType.Total, newCount);
}
}
}
}
}

/**
* High level helper method to begin syncing and poll for new events. To listen for these
* events, add a listener for {@link module:client~MatrixClient#event:"event"}
Expand Down
9 changes: 9 additions & 0 deletions src/filter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export interface IRoomEventFilter extends IFilterComponent {
types?: Array<EventType | string>;
related_by_senders?: Array<RelationType | string>;
related_by_rel_types?: string[];
unread_thread_notifications?: boolean;

// Unstable values
"io.element.relation_senders"?: Array<RelationType | string>;
Expand Down Expand Up @@ -220,6 +221,14 @@ export class Filter {
setProp(this.definition, "room.timeline.limit", limit);
}

/**
* Enable threads unread notification
* @param {boolean} enabled
*/
public setUnreadThreadNotifications(enabled: boolean) {
setProp(this.definition, "room.timeline.unread_thread_notifications", enabled);
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
}

setLazyLoadMembers(enabled: boolean) {
setProp(this.definition, "room.state.lazy_load_members", !!enabled);
}
Expand Down
182 changes: 35 additions & 147 deletions src/models/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,13 @@ export type RoomEventHandlerMap = {
[ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void;
} & ThreadHandlerMap & MatrixEventHandlerMap;

type NotificationCount = Partial<Record<NotificationCountType, number>>;

export class Room extends TimelineReceipts<EmittedEvents, RoomEventHandlerMap> {
public readonly reEmitter: TypedReEmitter<EmittedEvents, RoomEventHandlerMap>;
private txnToEvent: Record<string, MatrixEvent> = {}; // Pending in-flight requests { string: MatrixEvent }
private notificationCounts: Partial<Record<NotificationCountType, number>> = {};
private notificationCounts: NotificationCount = {};
public threadNotifications: Record<string, NotificationCount> = {};
private readonly timelineSets: EventTimelineSet[];
public readonly threadsTimelineSets: EventTimelineSet[] = [];
// any filtered timeline sets we're maintaining for this room
Expand Down Expand Up @@ -1120,14 +1123,30 @@ export class Room extends TimelineReceipts<EmittedEvents, RoomEventHandlerMap> {
return event;
}

/**
* Get sum of threads and roon notification counts
* @param {String} type The type of notification count to get. default: 'total'
* @return {Number} The notification count.
*/
public getTotalUnreadNotificationCount(type = NotificationCountType.Total): number {
return (this.getUnreadNotificationCount(type) ?? 0) +
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
this.getTotalThreadsUnreadNotificationCount(type);
}

public getTotalThreadsUnreadNotificationCount(type = NotificationCountType.Total): number {
return Object.keys(this.threadNotifications).reduce((total: number, threadId: string) => {
return total + (this.getThreadUnreadNotificationCount(threadId, type) ?? 0);
}, 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 getUnreadNotificationCount(type = NotificationCountType.Total): number | undefined {
return this.notificationCounts[type];
return this.notificationCounts[type] ?? 0;
germain-gg marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -1139,6 +1158,20 @@ export class Room extends TimelineReceipts<EmittedEvents, RoomEventHandlerMap> {
this.notificationCounts[type] = count;
}

public getThreadUnreadNotificationCount(threadId: string, type = NotificationCountType.Total): number | undefined {
return this.threadNotifications[threadId]?.[type];
}

public setThreadUnreadNotificationCount(threadId: string, type: NotificationCountType, count: number): void {
this.threadNotifications[threadId] = {
highlight: this.threadNotifications[threadId]?.highlight ?? 0,
total: this.threadNotifications[threadId]?.total ?? 0,
...{
[type]: count,
},
};
}

public setSummary(summary: IRoomSummary): void {
const heroes = summary["m.heroes"];
const joinedCount = summary["m.joined_member_count"];
Expand Down Expand Up @@ -2450,151 +2483,6 @@ export class Room extends TimelineReceipts<EmittedEvents, RoomEventHandlerMap> {
}
}

/**
* Get a list of user IDs who have <b>read up to</b> the given event.
* @param {MatrixEvent} event the event to get read receipts for.
* @return {String[]} A list of user IDs.
*/
public getUsersReadUpTo(event: MatrixEvent): string[] {
return this.getReceiptsForEvent(event).filter(function(receipt) {
return utils.isSupportedReceiptType(receipt.type);
}).map(function(receipt) {
return receipt.userId;
});
}

/**
* Gets the latest receipt for a given user in the room
* @param userId The id of the user for which we want the receipt
* @param ignoreSynthesized Whether to ignore synthesized receipts or not
* @param receiptType Optional. The type of the receipt we want to get
* @returns the latest receipts of the chosen type for the chosen user
*/
public getReadReceiptForUserId(
userId: string, ignoreSynthesized = false, receiptType = ReceiptType.Read,
): IWrappedReceipt | null {
const [realReceipt, syntheticReceipt] = this.receipts[receiptType]?.[userId] ?? [];
if (ignoreSynthesized) {
return realReceipt;
}

return syntheticReceipt ?? realReceipt;
}

/**
* Get the ID of the event that a given user has read up to, or null if we
* have received no read receipts from them.
* @param {String} userId The user ID to get read receipt event ID for
* @param {Boolean} ignoreSynthesized If true, return only receipts that have been
* sent by the server, not implicit ones generated
* by the JS SDK.
* @return {String} ID of the latest event that the given user has read, or null.
*/
public getEventReadUpTo(userId: string, ignoreSynthesized = false): string | null {
// XXX: This is very very ugly and I hope I won't have to ever add a new
// receipt type here again. IMHO this should be done by the server in
// some more intelligent manner or the client should just use timestamps

const timelineSet = this.getUnfilteredTimelineSet();
const publicReadReceipt = this.getReadReceiptForUserId(
userId,
ignoreSynthesized,
ReceiptType.Read,
);
const privateReadReceipt = this.getReadReceiptForUserId(
userId,
ignoreSynthesized,
ReceiptType.ReadPrivate,
);
const unstablePrivateReadReceipt = this.getReadReceiptForUserId(
userId,
ignoreSynthesized,
ReceiptType.UnstableReadPrivate,
);

// If we have all, compare them
if (publicReadReceipt?.eventId && privateReadReceipt?.eventId && unstablePrivateReadReceipt?.eventId) {
const comparison1 = timelineSet.compareEventOrdering(
publicReadReceipt.eventId,
privateReadReceipt.eventId,
);
const comparison2 = timelineSet.compareEventOrdering(
publicReadReceipt.eventId,
unstablePrivateReadReceipt.eventId,
);
const comparison3 = timelineSet.compareEventOrdering(
privateReadReceipt.eventId,
unstablePrivateReadReceipt.eventId,
);
if (comparison1 && comparison2 && comparison3) {
return (comparison1 > 0)
? ((comparison2 > 0) ? publicReadReceipt.eventId : unstablePrivateReadReceipt.eventId)
: ((comparison3 > 0) ? privateReadReceipt.eventId : unstablePrivateReadReceipt.eventId);
}
}

let latest = privateReadReceipt;
[unstablePrivateReadReceipt, publicReadReceipt].forEach((receipt) => {
if (receipt?.data?.ts > latest?.data?.ts) {
latest = receipt;
}
});
if (latest?.eventId) return latest?.eventId;

// The more less likely it is for a read receipt to drift out of date
// the bigger is its precedence
return (
privateReadReceipt?.eventId ??
unstablePrivateReadReceipt?.eventId ??
publicReadReceipt?.eventId ??
null
);
}

/**
* Determines if the given user has read a particular event ID with the known
* history of the room. This is not a definitive check as it relies only on
* what is available to the room at the time of execution.
* @param {String} userId The user ID to check the read state of.
* @param {String} eventId The event ID to check if the user read.
* @returns {Boolean} True if the user has read the event, false otherwise.
*/
public hasUserReadEvent(userId: string, eventId: string): boolean {
const readUpToId = this.getEventReadUpTo(userId, false);
if (readUpToId === eventId) return true;

if (this.timeline.length
&& this.timeline[this.timeline.length - 1].getSender()
&& this.timeline[this.timeline.length - 1].getSender() === userId) {
// It doesn't matter where the event is in the timeline, the user has read
// it because they've sent the latest event.
return true;
}

for (let i = this.timeline.length - 1; i >= 0; --i) {
const ev = this.timeline[i];

// If we encounter the target event first, the user hasn't read it
// however if we encounter the readUpToId first then the user has read
// it. These rules apply because we're iterating bottom-up.
if (ev.getId() === eventId) return false;
if (ev.getId() === readUpToId) return true;
}

// We don't know if the user has read it, so assume not.
return false;
}

/**
* Get a list of receipts for the given event.
* @param {MatrixEvent} event the event to get receipts for
* @return {Object[]} A list of receipts with a userId, type and data keys or
* an empty list.
*/
public getReceiptsForEvent(event: MatrixEvent): ICachedReceipt[] {
return this.receiptCacheByEventId[event.getId()] || [];
}

/**
* Add a receipt event to the room.
* @param {MatrixEvent} event The m.receipt event.
Expand Down
Loading