Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Make clear notifications work with threads #9575

Merged
merged 4 commits into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
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
33 changes: 19 additions & 14 deletions src/components/views/settings/Notifications.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import AccessibleButton from "../elements/AccessibleButton";
import TagComposer from "../elements/TagComposer";
import { objectClone } from "../../../utils/objects";
import { arrayDiff } from "../../../utils/arrays";
import { getLocalNotificationAccountDataEventType } from "../../../utils/notifications";
import { clearAllNotifications, getLocalNotificationAccountDataEventType } from "../../../utils/notifications";

// TODO: this "view" component still has far too much application logic in it,
// which should be factored out to other files.
Expand Down Expand Up @@ -112,6 +112,8 @@ interface IState {
desktopNotifications: boolean;
desktopShowBody: boolean;
audioNotifications: boolean;

clearingNotifications: boolean;
}

export default class Notifications extends React.PureComponent<IProps, IState> {
Expand All @@ -126,6 +128,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
desktopNotifications: SettingsStore.getValue("notificationsEnabled"),
desktopShowBody: SettingsStore.getValue("notificationBodyEnabled"),
audioNotifications: SettingsStore.getValue("audioNotificationsEnabled"),
clearingNotifications: false,
};

this.settingWatchers = [
Expand Down Expand Up @@ -177,8 +180,12 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
])).reduce((p, c) => Object.assign(c, p), {});

this.setState<keyof Omit<IState,
"deviceNotificationsEnabled" | "desktopNotifications" | "desktopShowBody" | "audioNotifications">
>({
"deviceNotificationsEnabled" |
"desktopNotifications" |
"desktopShowBody" |
"audioNotifications" |
"clearingNotifications"
>>({
...newState,
phase: Phase.Ready,
});
Expand Down Expand Up @@ -433,17 +440,14 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
}
};

private onClearNotificationsClicked = () => {
const client = MatrixClientPeg.get();
client.getRooms().forEach(r => {
if (r.getUnreadNotificationCount() > 0) {
const events = r.getLiveTimeline().getEvents();
if (events.length) {
// noinspection JSIgnoredPromiseFromCall
client.sendReadReceipt(events[events.length - 1]);
}
}
});
private onClearNotificationsClicked = async (): Promise<void> => {
try {
this.setState({ clearingNotifications: true });
const client = MatrixClientPeg.get();
await clearAllNotifications(client);
} finally {
this.setState({ clearingNotifications: false });
}
};

private async setKeywords(keywords: string[], originalRules: IAnnotatedPushRule[]) {
Expand Down Expand Up @@ -605,6 +609,7 @@ export default class Notifications extends React.PureComponent<IProps, IState> {
) {
clearNotifsButton = <AccessibleButton
onClick={this.onClearNotificationsClicked}
disabled={this.state.clearingNotifications}
kind='danger'
className='mx_UserNotifSettings_clearNotifsButton'
>{ _t("Clear notifications") }</AccessibleButton>;
Expand Down
27 changes: 27 additions & 0 deletions src/utils/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
import { MatrixClient } from "matrix-js-sdk/src/client";
import { LOCAL_NOTIFICATION_SETTINGS_PREFIX } from "matrix-js-sdk/src/@types/event";
import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifications";
import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts";
import { Room } from "matrix-js-sdk/src/models/room";

import SettingsStore from "../settings/SettingsStore";

Expand Down Expand Up @@ -56,3 +58,28 @@ export function localNotificationsAreSilenced(cli: MatrixClient): boolean {
const event = cli.getAccountData(eventType);
return event?.getContent<LocalNotificationSettings>()?.is_silenced ?? false;
}

export function clearAllNotifications(client: MatrixClient): Promise<Array<{}>> {
const receiptPromises = client.getRooms().reduce((promises, room: Room) => {
if (room.getUnreadNotificationCount() > 0) {
const roomEvents = room.getLiveTimeline().getEvents();
const lastThreadEvents = room.lastThread?.events;

const lastRoomEvent = roomEvents?.[roomEvents?.length - 1];
const lastThreadLastEvent = lastThreadEvents?.[lastThreadEvents?.length - 1];

const lastEvent = (lastRoomEvent?.getTs() ?? 0) > (lastThreadLastEvent?.getTs() ?? 0)
? lastRoomEvent
: lastThreadLastEvent;

if (lastEvent) {
const promise = client.sendReadReceipt(lastEvent, ReceiptType.Read, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

What about private read receipts?

Copy link
Contributor Author

@germain-gg germain-gg Nov 15, 2022

Choose a reason for hiding this comment

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

@SimonBrandner I am unsure how private read receipts are supposed to work.

I have copied the logic that is in TimelinePanel that checks the sendReadReceipts setting per room to decide to send ReceiptType.Read if true and ReceiptType.ReadPrivate if false.

Can you confirm this is correct?
As a heads up, the clear notification feature did not support private read receipts before this pull request, and therefore leaked that data.

promises = [...promises, promise];
}
}

return promises;
}, []);

return Promise.all(receiptPromises);
}
43 changes: 43 additions & 0 deletions test/utils/notifications-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,21 @@ limitations under the License.

import { MatrixEvent } from "matrix-js-sdk/src/models/event";
import { mocked } from "jest-mock";
import { MatrixClient } from "matrix-js-sdk/src/client";
import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room";
import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts";

import {
localNotificationsAreSilenced,
getLocalNotificationAccountDataEventType,
createLocalNotificationSettingsIfNeeded,
deviceNotificationSettingsKeys,
clearAllNotifications,
} from "../../src/utils/notifications";
import SettingsStore from "../../src/settings/SettingsStore";
import { getMockClientWithEventEmitter } from "../test-utils/client";
import { mkMessage, stubClient } from "../test-utils/test-utils";
import { MatrixClientPeg } from "../../src/MatrixClientPeg";

jest.mock("../../src/settings/SettingsStore");

Expand Down Expand Up @@ -99,4 +105,41 @@ describe('notifications', () => {
expect(localNotificationsAreSilenced(mockClient)).toBeFalsy();
});
});

describe("clearAllNotifications", () => {
let client: MatrixClient;
let room: Room;
let sendReadReceiptSpy;

const ROOM_ID = "123";
const USER_ID = "@bob:example.org";

beforeEach(() => {
stubClient();
client = mocked(MatrixClientPeg.get());
room = new Room(ROOM_ID, client, USER_ID);
sendReadReceiptSpy = jest.spyOn(client, "sendReadReceipt").mockResolvedValue({});
jest.spyOn(client, "getRooms").mockReturnValue([room]);
});

it("does not send any requests if everything has been read", () => {
clearAllNotifications(client);
expect(sendReadReceiptSpy).not.toBeCalled();
});

it("sends unthreaded receipt requests", () => {
const message = mkMessage({
event: true,
room: ROOM_ID,
user: USER_ID,
ts: 1,
});
room.addLiveEvents([message]);
room.setUnreadNotificationCount(NotificationCountType.Total, 1);

clearAllNotifications(client);

expect(sendReadReceiptSpy).toBeCalledWith(message, ReceiptType.Read, true);
});
});
});