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

Allow configuration of multiple moderator users to be invited to all rooms. #234

Merged
merged 4 commits into from
Dec 17, 2024
Merged

Allow configuration of multiple moderator users to be invited to all rooms. #234

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57ffd23
Allow configuration of multiple moderator users to be invited to all …
reivilibre Nov 28, 2024
1b41a6c
Fix SPECIAL_INTEREST_CREATION_TEMPLATE
reivilibre Dec 3, 2024
e01d120
Add basic E2E tests for what rooms are created and where the moderato…
reivilibre Dec 16, 2024
e530fd2
Throw startup error when old option supplied
reivilibre Dec 16, 2024
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
8 changes: 4 additions & 4 deletions config/default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ idServerDomain: "vector.im"
# result in emails or other communications to the user.
idServerBrand: "vector-im"

# This is the user ID of the moderator for all public-facing rooms the bot creates.
# Note that this user will be granted power level 100 (the highest) in every room
# These are the user IDs of the moderators for all public-facing rooms the bot creates.
# Note that these users will be granted power level 100 (the highest) in every room
# and be invited.
moderatorUserId: "@moderator:example.org"
moderatorUserIds: ["@moderator:example.org"]

# Settings for how the bot should represent livestreams.
livestream:
Expand Down Expand Up @@ -337,4 +337,4 @@ metrics:
address: 127.0.0.1

# if set to `true` will prevent the conf-bot from sending live invites to email/matrix_ids
dry_run_enabled: false
dry_run_enabled: false
2 changes: 1 addition & 1 deletion spec/util/e2e-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export class E2ETestEnv {
}
},
},
moderatorUserId: `@modbot:${homeserver.domain}`,
moderatorUserIds: [`@modbot:${homeserver.domain}`],
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we attempt a test for this, should be easy to whip one up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrote this now, a little fiddly but I think it's better than nothing? :-)

webserver: {
address: '0.0.0.0',
port: 0,
Expand Down
24 changes: 15 additions & 9 deletions src/Conference.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ export class Conference {
this.client,
mergeWithCreationTemplate(AUDITORIUM_BACKSTAGE_CREATION_TEMPLATE, {
room_alias_name: (new RoomAlias(alias)).localpart,
invite: [this.config.moderatorUserId],
invite: this.config.moderatorUserIds,
}),
);
await rootSpace.addChildRoom(roomId);
Expand Down Expand Up @@ -433,7 +433,7 @@ export class Conference {
subspace = await this.client.createSpace({
isPublic: true,
name: name,
invites: [this.config.moderatorUserId],
invites: this.config.moderatorUserIds,
});
this.subspaces[subspaceId] = subspace;

Expand All @@ -448,9 +448,11 @@ export class Conference {
roomId: subspace.roomId,
} as IStoredSubspace);

// Grants PL100 to the moderator in the subspace.
// Grants PL100 to the moderators in the subspace.
// We can't do this directly with `createSpace` unfortunately, as we could for plain rooms.
await this.client.setUserPowerLevel(this.config.moderatorUserId, subspace.roomId, 100);
for (let moderator of this.config.moderatorUserIds) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically we could convert createSpace to createRoom (with the necessary space state added in) and then just use the getSpace function to pull it as a space object, if we did prefer doing it that way?

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 feel more happy leaving it as it is (in a 'known working' state) than messing TBH. It is a shame that these don't compose well though.

await this.client.setUserPowerLevel(moderator, subspace.roomId, 100);
}
} else {
subspace = this.subspaces[subspaceId];
}
Expand Down Expand Up @@ -481,7 +483,7 @@ export class Conference {
);
} else {
// Create a new interest room.
roomId = await safeCreateRoom(this.client, mergeWithCreationTemplate(SPECIAL_INTEREST_CREATION_TEMPLATE(this.config.moderatorUserId), {
roomId = await safeCreateRoom(this.client, mergeWithCreationTemplate(SPECIAL_INTEREST_CREATION_TEMPLATE(this.config.moderatorUserIds), {
creation_content: {
[RSC_CONFERENCE_ID]: this.id,
[RSC_SPECIAL_INTEREST_ID]: interestRoom.id,
Expand Down Expand Up @@ -571,7 +573,7 @@ export class Conference {

await parentSpace.addChildSpace(audSpace, { order: `auditorium-${auditorium.id}` });

const roomId = await safeCreateRoom(this.client, mergeWithCreationTemplate(AUDITORIUM_CREATION_TEMPLATE(this.config.moderatorUserId), {
const roomId = await safeCreateRoom(this.client, mergeWithCreationTemplate(AUDITORIUM_CREATION_TEMPLATE(this.config.moderatorUserIds), {
creation_content: {
[RSC_CONFERENCE_ID]: this.id,
[RSC_AUDITORIUM_ID]: auditorium.id,
Expand Down Expand Up @@ -629,7 +631,7 @@ export class Conference {
}

if (!this.talks[talk.id]) {
roomId = await safeCreateRoom(this.client, mergeWithCreationTemplate(TALK_CREATION_TEMPLATE(this.config.moderatorUserId), {
roomId = await safeCreateRoom(this.client, mergeWithCreationTemplate(TALK_CREATION_TEMPLATE(this.config.moderatorUserIds), {
name: talk.title,
creation_content: {
[RSC_CONFERENCE_ID]: this.id,
Expand Down Expand Up @@ -848,7 +850,9 @@ export class Conference {
// we'll be unable to do promotions/demotions in the future.
const pls = await this.client.getRoomStateEvent(roomId, "m.room.power_levels", "");
pls['users'][await this.client.getUserId()] = 100;
pls['users'][this.config.moderatorUserId] = 100;
for (let moderator of this.config.moderatorUserIds) {
pls['users'][moderator] = 100;
}
for (const userId of mxids) {
if (pls['users'][userId]) continue;
pls['users'][userId] = 50;
Expand Down Expand Up @@ -916,7 +920,9 @@ export class Conference {
this.membersInRooms[roomId] = joinedOrLeftMembers;
const total = new Set(Object.values(this.membersInRooms).flat());
total.delete(myUserId);
total.delete(this.config.moderatorUserId);
for (let moderator of this.config.moderatorUserIds) {
total.delete(moderator);
}
attendeeTotalGauge.set(total.size);
} catch (ex) {
LogService.warn("Conference", `Failed to recalculate room membership for ${roomId}`, ex);
Expand Down
2 changes: 1 addition & 1 deletion src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface IConfig {
managementRoom: string;
idServerDomain?: string;
idServerBrand?: string;
moderatorUserId: string;
moderatorUserIds: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to still have the legacy moderatorUserId option, which we internally convert to moderatorUserIds at runtime just for anyone running an old config.

Or failing that, warn on startup that the old field is unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

refusing to start up seems like a reasonable option. I don't think we should force ourselves into too much backwards compat given we don't expect anyone else to be running this really. But refusing to start up sounds like a decent footgun-avoider.

livestream: {
auditoriumUrl: string;
talkUrl: string;
Expand Down
23 changes: 10 additions & 13 deletions src/models/room_kinds.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { RoomCreateOptions } from "matrix-bot-sdk";

export const KickPowerLevel = 50;

export const PUBLIC_ROOM_POWER_LEVELS_TEMPLATE = (moderatorUserId: string) => ({
export const PUBLIC_ROOM_POWER_LEVELS_TEMPLATE = (moderatorUserIds: string[]) => ({
ban: 50,
events_default: 0,
invite: 50,
Expand All @@ -40,10 +40,7 @@ export const PUBLIC_ROOM_POWER_LEVELS_TEMPLATE = (moderatorUserId: string) => ({
"m.space.parent": 100,
"m.space.child": 100,
},
users: {
[moderatorUserId]: 100,
// should be populated with the creator
},
users: Object.fromEntries(moderatorUserIds.map(moderator => [moderator, 100])),
});

export const PRIVATE_ROOM_POWER_LEVELS_TEMPLATE = {
Expand Down Expand Up @@ -91,7 +88,7 @@ export const CONFERENCE_ROOM_CREATION_TEMPLATE: RoomCreateOptions = {
},
};

export const AUDITORIUM_CREATION_TEMPLATE = (moderatorUserId: string) => ({
export const AUDITORIUM_CREATION_TEMPLATE = (moderatorUserIds: string[]) => ({
preset: 'public_chat',
visibility: 'public',
initial_state: [
Expand All @@ -101,8 +98,8 @@ export const AUDITORIUM_CREATION_TEMPLATE = (moderatorUserId: string) => ({
creation_content: {
[RSC_ROOM_KIND_FLAG]: RoomKind.Auditorium,
},
power_level_content_override: PUBLIC_ROOM_POWER_LEVELS_TEMPLATE(moderatorUserId),
invite: [moderatorUserId],
power_level_content_override: PUBLIC_ROOM_POWER_LEVELS_TEMPLATE(moderatorUserIds),
invite: moderatorUserIds,
} satisfies RoomCreateOptions);

export const AUDITORIUM_BACKSTAGE_CREATION_TEMPLATE: RoomCreateOptions = {
Expand All @@ -118,7 +115,7 @@ export const AUDITORIUM_BACKSTAGE_CREATION_TEMPLATE: RoomCreateOptions = {
power_level_content_override: PRIVATE_ROOM_POWER_LEVELS_TEMPLATE,
};

export const TALK_CREATION_TEMPLATE = (moderatorUserId: string) => ({ // before being opened up to the public
export const TALK_CREATION_TEMPLATE = (moderatorUserIds: string[]) => ({ // before being opened up to the public
preset: 'private_chat',
visibility: 'private',
initial_state: [
Expand All @@ -128,11 +125,11 @@ export const TALK_CREATION_TEMPLATE = (moderatorUserId: string) => ({ // before
creation_content: {
[RSC_ROOM_KIND_FLAG]: RoomKind.Talk,
},
power_level_content_override: PUBLIC_ROOM_POWER_LEVELS_TEMPLATE(moderatorUserId),
invite: [moderatorUserId],
power_level_content_override: PUBLIC_ROOM_POWER_LEVELS_TEMPLATE(moderatorUserIds),
invite: moderatorUserIds,
} satisfies RoomCreateOptions);

export const SPECIAL_INTEREST_CREATION_TEMPLATE = (moderatorUserId: string) => ({
export const SPECIAL_INTEREST_CREATION_TEMPLATE = (moderatorUserIds: string[]) => ({
preset: 'public_chat',
visibility: 'public',
initial_state: [
Expand All @@ -149,7 +146,7 @@ export const SPECIAL_INTEREST_CREATION_TEMPLATE = (moderatorUserId: string) => (
"m.room.power_levels": 50,
},
},
invite: [moderatorUserId],
invite: moderatorUserIds,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hard to tell, but does PUBLIC_ROOM_POWER_LEVELS_TEMPLATE work here? It takes a parameter but we're not giving it one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as you point out, this is probably broken. We haven't used special interest rooms for at least a couple of years now so I guess it wouldn't be a surprise. I may as well fix that obvious part up now though

} satisfies RoomCreateOptions);

export function mergeWithCreationTemplate(template: RoomCreateOptions, addlProps: any): any {
Expand Down
Loading