Skip to content

Emit events when setting unread notifications #2748

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

Merged
merged 11 commits into from
Oct 13, 2022
26 changes: 23 additions & 3 deletions spec/unit/notifications.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { Feature, ServerSupport } from "../../src/feature";
import {
EventType,
fixNotificationCountOnDecryption,
Expand All @@ -23,6 +24,7 @@ import {
NotificationCountType,
RelationType,
Room,
RoomEvent,
} from "../../src/matrix";
import { IActionsObject } from "../../src/pushprocessor";
import { ReEmitter } from "../../src/ReEmitter";
Expand Down Expand Up @@ -56,8 +58,12 @@ describe("fixNotificationCountOnDecryption", () => {
supportsExperimentalThreads: jest.fn().mockReturnValue(true),
});
mockClient.reEmitter = mock(ReEmitter, 'ReEmitter');
mockClient.canSupport = new Map();
Object.keys(Feature).forEach(feature => {
mockClient.canSupport.set(feature as Feature, ServerSupport.Stable);
});

room = new Room(ROOM_ID, mockClient, mockClient.getUserId());
room = new Room(ROOM_ID, mockClient, mockClient.getUserId() ?? "");
room.setUnreadNotificationCount(NotificationCountType.Total, 1);
room.setUnreadNotificationCount(NotificationCountType.Highlight, 0);

Expand Down Expand Up @@ -93,12 +99,12 @@ describe("fixNotificationCountOnDecryption", () => {
});

it("changes the room count to highlight on decryption", () => {
expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(1);
expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(2);
expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(0);

fixNotificationCountOnDecryption(mockClient, event);

expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(1);
expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(2);
expect(room.getUnreadNotificationCount(NotificationCountType.Highlight)).toBe(1);
});

Expand All @@ -111,4 +117,18 @@ describe("fixNotificationCountOnDecryption", () => {
expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total)).toBe(1);
expect(room.getThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight)).toBe(1);
});

it("emits events", () => {
const cb = jest.fn();
room.on(RoomEvent.UnreadNotifications, cb);

room.setUnreadNotificationCount(NotificationCountType.Total, 1);
expect(cb).toHaveBeenLastCalledWith({ highlight: 0, total: 1 });

room.setUnreadNotificationCount(NotificationCountType.Highlight, 5);
expect(cb).toHaveBeenLastCalledWith({ highlight: 5, total: 1 });

room.setThreadUnreadNotificationCount("$123", NotificationCountType.Highlight, 5);
expect(cb).toHaveBeenLastCalledWith({ highlight: 5 }, "$123");
});
});
48 changes: 43 additions & 5 deletions spec/unit/room.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2572,15 +2572,15 @@ describe("Room", function() {
});

it("defaults to undefined", () => {
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Total)).toBeUndefined();
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Highlight)).toBeUndefined();
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Total)).toBe(0);
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Highlight)).toBe(0);
});

it("lets you set values", () => {
room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 1);

expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Total)).toBe(1);
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Highlight)).toBeUndefined();
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Highlight)).toBe(0);

room.setThreadUnreadNotificationCount("123", NotificationCountType.Highlight, 10);

Expand All @@ -2592,10 +2592,48 @@ describe("Room", function() {
room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 666);
room.setThreadUnreadNotificationCount("123", NotificationCountType.Highlight, 123);

expect(room.getThreadsAggregateNotificationType()).toBe(NotificationCountType.Highlight);

room.resetThreadUnreadNotificationCount();

expect(room.getThreadsAggregateNotificationType()).toBe(null);

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

it("sets the room threads notification type", () => {
room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 666);
expect(room.getThreadsAggregateNotificationType()).toBe(NotificationCountType.Total);
room.setThreadUnreadNotificationCount("123", NotificationCountType.Highlight, 123);
expect(room.getThreadsAggregateNotificationType()).toBe(NotificationCountType.Highlight);
room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 333);
expect(room.getThreadsAggregateNotificationType()).toBe(NotificationCountType.Highlight);
});
});

describe("hasThreadUnreadNotification", () => {
it('has no notifications by default', () => {
expect(room.hasThreadUnreadNotification()).toBe(false);
});

it('main timeline notification does not affect this', () => {
room.setUnreadNotificationCount(NotificationCountType.Highlight, 1);
expect(room.hasThreadUnreadNotification()).toBe(false);
room.setUnreadNotificationCount(NotificationCountType.Total, 1);
expect(room.hasThreadUnreadNotification()).toBe(false);

room.setThreadUnreadNotificationCount("123", NotificationCountType.Total, 1);
expect(room.hasThreadUnreadNotification()).toBe(true);
});

it('lets you reset', () => {
room.setThreadUnreadNotificationCount("123", NotificationCountType.Highlight, 1);
expect(room.hasThreadUnreadNotification()).toBe(true);

room.resetThreadUnreadNotificationCount();

expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Total)).toBeUndefined();
expect(room.getThreadUnreadNotificationCount("123", NotificationCountType.Highlight)).toBeUndefined();
expect(room.hasThreadUnreadNotification()).toBe(false);
});
});
});
83 changes: 75 additions & 8 deletions src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
ReceiptContent,
synthesizeReceipt,
} from "./read-receipt";
import { Feature, ServerSupport } from "../feature";

// These constants are used as sane defaults when the homeserver doesn't support
// the m.room_versions capability. In practice, KNOWN_SAFE_ROOM_VERSION should be
Expand Down Expand Up @@ -96,7 +97,7 @@ export interface IRecommendedVersion {
// price to pay to keep matrix-js-sdk responsive.
const MAX_NUMBER_OF_VISIBILITY_EVENTS_TO_SCAN_THROUGH = 30;

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

export enum NotificationCountType {
Highlight = "highlight",
Expand Down Expand Up @@ -127,6 +128,7 @@ export enum RoomEvent {
OldStateUpdated = "Room.OldStateUpdated",
CurrentStateUpdated = "Room.CurrentStateUpdated",
HistoryImportedWithinTimeline = "Room.historyImportedWithinTimeline",
UnreadNotifications = "Room.UnreadNotifications",
Copy link
Member

Choose a reason for hiding this comment

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

it bothers me that we mix Subject.lowerCamelCase and Subject.UpperCamelCase, ftr. Personally I prefer lowerCamelCase, but that seems to be outvoted here.

}

type EmittedEvents = RoomEvent
Expand Down Expand Up @@ -164,6 +166,10 @@ export type RoomEventHandlerMap = {
markerEvent: MatrixEvent,
room: Room,
) => void;
[RoomEvent.UnreadNotifications]: (
unreadNotifications: NotificationCount,
threadId?: string,
) => void;
[RoomEvent.TimelineRefresh]: (room: Room, eventTimelineSet: EventTimelineSet) => void;
[ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void;
} & ThreadHandlerMap
Expand All @@ -186,7 +192,8 @@ export class Room extends ReadReceipt<EmittedEvents, RoomEventHandlerMap> {
public readonly reEmitter: TypedReEmitter<EmittedEvents, RoomEventHandlerMap>;
private txnToEvent: Record<string, MatrixEvent> = {}; // Pending in-flight requests { string: MatrixEvent }
private notificationCounts: NotificationCount = {};
private threadNotifications: Map<string, NotificationCount> = new Map();
private readonly threadNotifications = new Map<string, NotificationCount>();
private roomThreadsNotificationType: NotificationCountType | null = null;
private readonly timelineSets: EventTimelineSet[];
public readonly threadsTimelineSets: EventTimelineSet[] = [];
// any filtered timeline sets we're maintaining for this room
Expand Down Expand Up @@ -1182,38 +1189,97 @@ export class Room extends ReadReceipt<EmittedEvents, RoomEventHandlerMap> {
* @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];
public getUnreadNotificationCount(type = NotificationCountType.Total): number {
let count = this.notificationCounts[type] ?? 0;
if (this.client.canSupport.get(Feature.ThreadUnreadNotifications) !== ServerSupport.Unsupported) {
for (const threadNotification of this.threadNotifications.values()) {
count += threadNotification[type] ?? 0;
}
}
return count;
}

/**
* @experimental
* Get one of the notification counts for a thread
* @param threadId the root event ID
* @param type The type of notification count to get. default: 'total'
* @returns The notification count, or undefined if there is no count
* for this type.
*/
public getThreadUnreadNotificationCount(threadId: string, type = NotificationCountType.Total): number | undefined {
return this.threadNotifications.get(threadId)?.[type];
public getThreadUnreadNotificationCount(threadId: string, type = NotificationCountType.Total): number {
Copy link
Member

Choose a reason for hiding this comment

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

this looks like a breaking change - the function is not flagged as experimental, it's public, and it has a behavioural change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should have been marked as experimental. And this is only behind a labs flag.
I believe this should not be marked as a breaking change.

Copy link
Member

Choose a reason for hiding this comment

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

The js-sdk doesn't have a concept of labs flags though, and we can't guarantee that people aren't using it. There's other breaking changes in the SDK already landed anyways, so I think we can just apply the label here and move on.

return this.threadNotifications.get(threadId)?.[type] ?? 0;
}

/**
* @experimental
* Checks if the current room has unread thread notifications
* @returns {boolean}
*/
public hasThreadUnreadNotification(): boolean {
for (const notification of this.threadNotifications.values()) {
if ((notification.highlight ?? 0) > 0 || (notification.total ?? 0) > 0) {
return true;
}
}
return false;
}

/**
* @experimental
* Swet one of the notification count for a thread
* @param threadId the root event ID
* @param type The type of notification count to get. default: 'total'
* @returns {void}
*/
public setThreadUnreadNotificationCount(threadId: string, type: NotificationCountType, count: number): void {
this.threadNotifications.set(threadId, {
const notification: NotificationCount = {
highlight: this.threadNotifications.get(threadId)?.highlight,
total: this.threadNotifications.get(threadId)?.total,
...{
[type]: count,
},
});
};

this.threadNotifications.set(threadId, notification);

// Remember what the global threads notification count type is
// for all the threads in the room
if (count > 0) {
switch (this.roomThreadsNotificationType) {
case NotificationCountType.Highlight:
break;
case NotificationCountType.Total:
if (type === NotificationCountType.Highlight) {
this.roomThreadsNotificationType = type;
}
break;
default:
this.roomThreadsNotificationType = type;
}
}

this.emit(
RoomEvent.UnreadNotifications,
notification,
threadId,
);
}

/**
* @experimental
* @returns the notification count type for all the threads in the room
*/
public getThreadsAggregateNotificationType(): NotificationCountType | null {
return this.roomThreadsNotificationType;
}

/**
* @experimental
* Resets the thread notifications for this room
*/
public resetThreadUnreadNotificationCount(): void {
this.roomThreadsNotificationType = null;
this.threadNotifications.clear();
}

Expand All @@ -1224,6 +1290,7 @@ export class Room extends ReadReceipt<EmittedEvents, RoomEventHandlerMap> {
*/
public setUnreadNotificationCount(type: NotificationCountType, count: number): void {
this.notificationCounts[type] = count;
this.emit(RoomEvent.UnreadNotifications, this.notificationCounts);
}

public setSummary(summary: IRoomSummary): void {
Expand Down