matrix-appservice-irc
Automatically create mapped channels if `mappings[...].createRoom` is set
#1099
Open

Automatically create mapped channels if `mappings[...].createRoom` is set #1099

Half-Shot wants to merge 15 commits into develop from hs/auto-create-rooms
Half-Shot
Half-Shot4 years ago (edited 4 years ago)

Fixes #1096

The idea behind this is that you can just specify a bunch of mappings in your config and have rooms be created on the fly for them.

There are a few problems though. The behavior of mappings is to normally only bridge things defined in the section. If you remove a mapping entry, the bridge should stop bridging it. However this is more difficult in this case as the roomId is created by the bridge and not stored in the config. We need to persist it in the db, but this means adding special behaviors. I've done this for postgres, but it's harder to do in NeDB.

Half-Shot Update config
91668c29
Half-Shot Allow choosing visibility of channels
e74dfae8
Half-Shot Don't map rooms with the createRoom flag, and check roomIds exists
45f0ead9
Half-Shot Automatically create rooms for channels
2a139226
Half-Shot Try to develop a way for rooms to be removed if removed from the config
a516514d
Half-Shot linting
4917a920
Half-Shot changelog
adcec3e6
Half-Shot only exclude channels sometimes
64552768
Half-Shot Support autocreated channels on NeDB
fc5bd836
Half-Shot Postgresql formattign
511f26ce
Half-Shot Don't track autocreated rooms when starting up
260e93ed
Half-Shot Fix config room adding and deleting
518c21d6
Half-Shot Add tests
a4d6f3aa
Half-Shot use domain
0c352176
Half-Shot lint
9582253c
Half-Shot Half-Shot requested a review from jaller94 jaller94 4 years ago
jaller94
jaller94 commented on 2020-08-13
jaller944 years ago

Still reviewing.. but here are some comments that would be helpful to have answered.

src/datastore/postgres/PgDataStore.ts
376 let exclusionQuerys = [];
377 console.log(server.getNetworkId(), server.getAutoCreateMappings());
378 for (const mapping of server.getAutoCreateMappings()) {
379
exclusionQuerys.push(mapping.channel);
jaller944 years ago

Even though this comes from a config that only an admin can configure, I wish we would escape the channel names before putting them into an SQL statement.

spec/integ/static-channels.spec.js
15 createRoom: true,
16 };
17
18
await test.beforeEach(env);
jaller944 years ago

What is this?
... Looks up env-bundle...

src/datastore/postgres/PgDataStore.ts
68 for (const [channel, data] of Object.entries(serverConfig.mappings)) {
69 if (data.createRoom) {
70 // We don't want to map this.
71
return;
jaller944 years ago

Return, not continue??

Half-Shot4 years ago

I have a habit of mixing those up 😆

jaller944 years ago

.map and functions let you use your preferred return command. 😀

jaller944 years ago
Suggested change
return;
continue;
src/datastore/NedbDataStore.ts
95
96 for (const [channel, opts] of Object.entries(serverConfig.mappings)) {
97 if (opts.createRoom) {
98
return;
jaller944 years ago

One opts.createRoom makes the entire setting of the server impossible?

jaller94
jaller94 commented on 2020-08-13
src/datastore/NedbDataStore.ts
422429 });
430 const notChannels = server.getAutoCreateMappings().map((c) => c.channel);
431 entries = (await entries).filter((e) => e.remote?.get("domain") === server.domain &&
432
!notChannels.includes(e.remote?.get("channel") as string));
jaller944 years ago (edited 4 years ago)

remote? but a cast to string.
That seems like a cast where we're possibly lying to get TypeScript happy. Can this not be string or undefined?

src/datastore/NedbDataStore.ts
428 origin: 'config',
422429 });
430 const notChannels = server.getAutoCreateMappings().map((c) => c.channel);
431
entries = (await entries).filter((e) => e.remote?.get("domain") === server.domain &&
jaller944 years ago

entries was already awaited two lines above.

jaller94 jaller94 assigned Half-Shot Half-Shot 4 years ago
Half-Shot Half-Shot requested a review 2 years ago

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
Labels
Milestone