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

Base support for MSC3840: Ignore invites #2483

Closed
wants to merge 15 commits into from
33 changes: 33 additions & 0 deletions spec/unit/matrix-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1297,4 +1297,37 @@ describe("MatrixClient", function() {
expect(result!.aliases).toEqual(response.aliases);
});
});

describe("ignoring invites", () => {
it('stores ignored invites', async function() {
Copy link
Member

Choose a reason for hiding this comment

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

we should be using should, and arrow functions:

Suggested change
it('stores ignored invites', async function() {
it('should store ignored invites', async () => {

// Mockup account data storage.
const dataStore = new Map();
client.setAccountData = function(eventType, content) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
client.setAccountData = function(eventType, content) {
client.setAccountData = (eventType, content) => {

dataStore.set(eventType, content);
return Promise.resolve();
};
client.getAccountData = function(eventType) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
client.getAccountData = function(eventType) {
client.getAccountData = (eventType) => {

const data = dataStore.get(eventType);
return new MatrixEvent({
content: data,
});
};

// Initially, the invite list should be empty but not `null`.
expect(await client.getIgnoredInvites()).toEqual({});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect(await client.getIgnoredInvites()).toEqual({});
expect(await client.getIgnoredInvites()).toStrictEqual({});


// Insert something, we should be able to recover it.
const SAMPLE = {
ignored_rooms: {
"12345": {
ts: Date.now(),
},
},
};
await client.setIgnoredInvites(SAMPLE);

// Check that it was (mock)stored on the client.
expect(await client.getIgnoredInvites()).toEqual(SAMPLE);
});
});
});
1 change: 1 addition & 0 deletions src/@types/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export enum EventType {
PushRules = "m.push_rules",
Direct = "m.direct",
IgnoredUserList = "m.ignored_user_list",
IgnoredInvites = "org.matrix.msc3840.ignored_invites",
Copy link
Member

Choose a reason for hiding this comment

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

we're not supposed to define unstable event types here - we should be using UnstableValue().name


// to_device events
RoomKey = "m.room_key",
Expand Down
49 changes: 49 additions & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3424,6 +3424,23 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
return this.getIgnoredUsers().includes(userId);
}

/**
* Gets the list of currently ignored invites.
*/
public getIgnoredInvites(): IgnoredInvites {
Copy link
Member

Choose a reason for hiding this comment

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

apologies, should have caught this in the first round of review: this function and the setter will need to be named unstable_getIgnoredInvites (and similar for the setter) to make it obvious that the functions can be ripped out.

Additionally, the return value for both will need to be documented in the jsdoc - see other functions for examples

turt2live marked this conversation as resolved.
Show resolved Hide resolved
const event = this.getAccountData(EventType.IgnoredInvites);
Copy link

Choose a reason for hiding this comment

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

what's the computational complexity of this function call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't been able to find exactly where this is implemented. However, as far as I understand, this.getAccountData should be fast (because everything is cached to memory).

Also note that you can subscribe to client.on(ClientEvent.AccountData, event => { ... }) to be informed whenever a piece of account data changes.

if (!event || !event.getContent()) return {};
return event.getContent();
}

/**
* Update the list of ignored invites.
* @param invites The new list of ignored invites.
*/
public setIgnoredInvites(invites: IgnoredInvites): Promise<{}> {
return this.setAccountData(EventType.IgnoredInvites, invites);
}

/**
* Join a room. If you have already joined the room, this will no-op.
* @param {string} roomIdOrAlias The room ID or room alias to join.
Expand Down Expand Up @@ -9320,3 +9337,35 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
* @event module:client~MatrixClient#"WellKnown.client"
* @param {object} data The JSON object returned by the server
*/

/**
* Metadata on ignored invites.
*
* This interface is expected to gain additional fields, all
* of them optional.
*/
export interface IgnoreInviteMetadata {
Copy link
Member

Choose a reason for hiding this comment

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

All of these interfaces will need a jsdoc line that says they can be removed at any point without notice due to being unstable.

/**
* The instant the user decided to ignore this invite,
* as a Matrix timestamp.
Comment on lines +9349 to +9350
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* The instant the user decided to ignore this invite,
* as a Matrix timestamp.
* The timestamp for when the user decided to ignore this invite.

*/
readonly ts: number;

/**
* A human-readable reason for ignoring this invite.
*/
readonly reason?: string;
}

/**
* Invites to ignore across all devices/sessions, as per MSC 3840.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Invites to ignore across all devices/sessions, as per MSC 3840.
* Invites to ignore across all devices/sessions, as per MSC3840.
*
* See https://github.com/matrix-org/matrix-spec-proposals/pull/3840

*
* This interface is expected to gain additional fields, all of
* them optional.
*/
export interface IgnoredInvites {
/**
* A map of room ids => metadata for rooms the user does not wish to be invited to.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* A map of room ids => metadata for rooms the user does not wish to be invited to.
* A map of room IDs => metadata for rooms the user does not wish to be invited to.

*/
readonly ignored_rooms?: Record<string, IgnoreInviteMetadata>;
}