feat/add-unlink-space-button #63

Open
Elliu wants to merge 10 commits from Elliu/out-of-your-element:feat/add-unlink-space-button into main
5 changed files with 247 additions and 26 deletions

View file

@ -459,12 +459,12 @@ async function unbridgeDeletedChannel(channel, guildID) {
const webhook = select("webhook", ["webhook_id", "webhook_token"], {channel_id: channel.id}).get()
if (webhook) {
await discord.snow.webhook.deleteWebhook(webhook.webhook_id, webhook.webhook_token)
db.prepare("DELETE FROM webhook WHERE channel_id = ?").run(channel.id)
await db.prepare("DELETE FROM webhook WHERE channel_id = ?").run(channel.id)
}
// delete room from database
db.prepare("DELETE FROM member_cache WHERE room_id = ?").run(roomID)
db.prepare("DELETE FROM channel_room WHERE room_id = ? AND channel_id = ?").run(roomID, channel.id) // cascades to most other tables, like messages
await db.prepare("DELETE FROM member_cache WHERE room_id = ?").run(roomID)
await db.prepare("DELETE FROM channel_room WHERE room_id = ? AND channel_id = ?").run(roomID, channel.id) // cascades to most other tables, like messages
if (!botInRoom) return
@ -507,6 +507,7 @@ async function unbridgeDeletedChannel(channel, guildID) {
}
// leave room
await api.setUserPower(roomID, bot, 0)
await api.leaveRoom(roomID)
}

View file

@ -129,13 +129,13 @@ block body
h3.mt32.fs-category Linked channels
.s-card.bs-sm.p0
form.s-table-container(method="post" action=rel("/api/unlink") hx-confirm="Do you want to unlink these channels?\nIt may take a moment to clean up Matrix resources.")
form.s-table-container(method="post" action=rel("/api/unlink"))
input(type="hidden" name="guild_id" value=guild_id)
table.s-table.s-table__bx-simple
each row in linkedChannelsWithDetails
tr
td.w40: +discord(row.channel)
td.p2: button.s-btn.s-btn__muted.s-btn__xs(name="channel_id" value=row.channel.id hx-post=rel("/api/unlink") hx-trigger="click" hx-disabled-elt="this")!= icons.Icons.IconLinkSm
td.p2: button.s-btn.s-btn__muted.s-btn__xs(name="channel_id" cx-prevent-default hx-post=rel("/api/unlink") hx-confirm="Do you want to unlink these channels?\nIt may take a moment to clean up Matrix resources." value=row.channel.id hx-indicator="this" hx-disabled-elt="this")!= icons.Icons.IconLinkSm
td: +matrix(row)
else
tr
@ -176,6 +176,18 @@ block body
!= icons.Icons.IconMerge
= ` Link`
h3.mt32.fs-category Unlink server
form.s-card.d-flex.fd-row-reverse.gx24.pl24.ai-center(method="post" action=rel("/api/unlink-space"))
input(type="hidden" name="guild_id" value=guild.id)
.fl-grow1.s-prose.s-prose__sm.lh-xl
p.
Sick of this bridge, or just made a mistake? You can unlink the whole server and all its channels.#[br]
This may take a minute to process. Please be patient and wait until the page refreshes.
div
button.s-btn.s-btn__icon.s-btn__danger.s-btn__outlined(cx-prevent-default hx-post=rel("/api/unlink-space") hx-confirm="Do you want to unlink this server and all its channels?\nIt may take a minute to clean up Matrix resources." hx-indicator="this" hx-disabled-elt="this")
!= icons.Icons.IconUnsync
span.ml4= ` Unlink`
details.mt48
summary Debug room list
.d-grid.grid__2.gx24
@ -196,7 +208,7 @@ block body
ul.my8.ml24
each row in removedWrongTypeChannels
li: a(href=`https://discord.com/channels/${guild_id}/${row.id}`) (#{row.type}) #{row.name}
h3.mt24 Unavailable channels: Bridge can't access
h3.mt24 Unavailable channels: Discord bot can't access
.s-card.p0
ul.my8.ml24
each row in removedPrivateChannels

View file

@ -129,6 +129,13 @@ html(lang="en")
document.styleSheets[0].insertRule(t, document.styleSheets[0].cssRules.length)
})
})
//- Prevent default
script.
document.querySelectorAll("[cx-prevent-default]").forEach(e => {
e.addEventListener("click", event => {
event.preventDefault()
})
})
script(src=rel("/static/htmx.js"))
//- Error dialog
aside.s-modal#server-error(aria-hidden="true")

View file

@ -12,6 +12,8 @@ const auth = sync.require("../auth")
const mreq = sync.require("../../matrix/mreq")
const {reg} = require("../../matrix/read-registration")
const me = `@${reg.sender_localpart}:${reg.ooye.server_name}`
/**
* @param {H3Event} event
* @returns {import("../../matrix/api")}
@ -39,6 +41,33 @@ function getCreateSpace(event) {
return event.context.createSpace || sync.require("../../d2m/actions/create-space")
}
/**
* @param {H3Event} event
* @param {string} channel_id
* @param {string} guild_id
*/
async function doRoomUnlink(event, channel_id, guild_id) {
const createRoom = getCreateRoom(event)
// Check that the channel (if it exists) is part of this guild
/** @type {any} */
let channel = discord.channels.get(channel_id)
if (channel) {
if (!("guild_id" in channel) || channel.guild_id !== guild_id) throw createError({status: 400, message: "Bad Request", data: `Channel ID ${channel_id} is not part of guild ${guild_id}`})
} else {
// Otherwise, if the channel isn't cached, it must have been deleted.
// There's no other authentication here - it's okay for anyone to unlink a deleted channel just by knowing its ID.
channel = {id: channel_id}
}
// Check channel is currently bridged
const row = select("channel_room", "channel_id", {channel_id: channel_id}).get()
if (!row) throw createError({status: 400, message: "Bad Request", data: `Channel ID ${channel_id} is not currently bridged`})
// Do it
await createRoom.unbridgeDeletedChannel(channel, guild_id)
}
const schema = {
linkSpace: z.object({
guild_id: z.string(),
@ -52,7 +81,10 @@ const schema = {
unlink: z.object({
guild_id: z.string(),
channel_id: z.string()
})
}),
unlinkSpace: z.object({
guild_id: z.string(),
}),
}
as.router.post("/api/link-space", defineEventHandler(async event => {
@ -87,7 +119,6 @@ as.router.post("/api/link-space", defineEventHandler(async event => {
}
// Check bridge has PL 100
const me = `@${reg.sender_localpart}:${reg.ooye.server_name}`
/** @type {Ty.Event.M_Power_Levels?} */
let powerLevelsStateContent = null
try {
@ -202,7 +233,6 @@ as.router.post("/api/link", defineEventHandler(async event => {
as.router.post("/api/unlink", defineEventHandler(async event => {
const {channel_id, guild_id} = await readValidatedBody(event, schema.unlink.parse)
const managed = await auth.getManagedGuilds(event)
const createRoom = getCreateRoom(event)
// Check guild ID or nonce
if (!managed.has(guild_id)) throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
@ -211,24 +241,48 @@ as.router.post("/api/unlink", defineEventHandler(async event => {
const guild = discord.guilds.get(guild_id)
if (!guild) throw createError({status: 400, message: "Bad Request", data: "Discord guild does not exist or bot has not joined it"})
// Check that the channel (if it exists) is part of this guild
/** @type {any} */
let channel = discord.channels.get(channel_id)
if (channel) {
if (!("guild_id" in channel) || channel.guild_id !== guild_id) throw createError({status: 400, message: "Bad Request", data: `Channel ID ${channel_id} is not part of guild ${guild_id}`})
} else {
// Otherwise, if the channel isn't cached, it must have been deleted.
// There's no other authentication here - it's okay for anyone to unlink a deleted channel just by knowing its ID.
channel = {id: channel_id}
}
// Check channel is currently bridged
const row = select("channel_room", "channel_id", {channel_id: channel_id}).get()
if (!row) throw createError({status: 400, message: "Bad Request", data: `Channel ID ${channel_id} is not currently bridged`})
// Do it
await createRoom.unbridgeDeletedChannel(channel, guild_id)
await doRoomUnlink(event, channel_id, guild_id)
setResponseHeader(event, "HX-Refresh", "true")
return null // 204
}))
as.router.post("/api/unlink-space", defineEventHandler(async event => {
const {guild_id} = await readValidatedBody(event, schema.unlinkSpace.parse)
const managed = await auth.getManagedGuilds(event)
const api = getAPI(event)
// Check guild ID or nonce
if (!managed.has(guild_id)) throw createError({status: 403, message: "Forbidden", data: "Can't edit a guild you don't have Manage Server permissions in"})
// Check guild exists
const guild = discord.guilds.get(guild_id)
if (!guild) throw createError({status: 400, message: "Bad Request", data: "Discord guild does not exist or bot has not joined it"})
const spaceID = select("guild_space", "space_id", {guild_id: guild_id}).pluck().get()
if (!spaceID) {
throw createError({status: 400, message: "Bad Request", data: "Matrix space does not exist or bot has not linked it"})
}
const linkedChannels = select("channel_room", ["channel_id", "room_id", "name", "nick"], {guild_id: guild_id}).all()
for (const channel of linkedChannels) {
await doRoomUnlink(event, channel.channel_id, guild_id)
}
const remainingLinkedChannels = select("channel_room", ["channel_id", "room_id", "name", "nick"], {guild_id: guild_id}).all()
if (remainingLinkedChannels.length !== 0) {

Hi, there is this issue here, I'm not sure what would be the best thing to do with this situation, please let me know if you have any ideas of what should be best

Hi, there is this issue here, I'm not sure what would be the best thing to do with this situation, please let me know if you have any ideas of what should be best

Nice find, thanks for pointing it out. For further reading, the purpose and values of guild_active are explained here: https://gitdab.com/cadence/out-of-your-element/src/branch/main/docs/self-service-room-creation-rules.md

The message "Please add the bot to your server using the buttons on the home page" is deliberately designed to appear in this situation. However, the user experience here could be better.

There's a couple of possible paths forward here:

  • leave the Discord guild as part of this operation
  • change the view that appears in that case (guild_access_denied.pug line 31) to present the easy-mode and self-service buttons directly here, and have the user go through those if they want to do setup for the guild again
  • set guild_active autocreate = 0, forcing it into self-service mode, so it knows it's ready to bridge but nothing gets autmatically re-created (I don't like this one)

Because per the explainer doc, it's bad to leave around the a guild_active entry if you don't want the guild to be bridged. If Easy Mode was previously selected, it would cause channels to bridge themselves again just from anybody sending a message in the server. That would be pretty weird if somebody just clicked unbridge and it starts bridging again seemingly by itself! So we can't just do nothing here.

I'll let you pick which path forward you'd like to go with. I don't have any preference between the first two.

Nice find, thanks for pointing it out. For further reading, the purpose and values of guild_active are explained here: https://gitdab.com/cadence/out-of-your-element/src/branch/main/docs/self-service-room-creation-rules.md The message "Please add the bot to your server using the buttons on the home page" is deliberately designed to appear in this situation. However, the user experience here could be better. There's a couple of possible paths forward here: - leave the Discord guild as part of this operation - change the view that appears in that case (guild_access_denied.pug line 31) to present the easy-mode and self-service buttons directly here, and have the user go through those if they want to do setup for the guild again - set guild_active autocreate = 0, forcing it into self-service mode, so it knows it's ready to bridge but nothing gets autmatically re-created (I don't like this one) Because per the explainer doc, it's bad to leave around the a guild_active entry if you don't want the guild to be bridged. If Easy Mode was previously selected, it would cause channels to bridge themselves again just from anybody sending a message in the server. That would be pretty weird if somebody just clicked unbridge and it starts bridging again seemingly by itself! So we can't just do nothing here. I'll let you pick which path forward you'd like to go with. I don't have any preference between the first two.

I note that you've coded it to always leave the Matrix space after this operation. I don't think it would be out of character for it to leave the Discord server as well.

I note that you've coded it to always leave the Matrix space after this operation. I don't think it would be out of character for it to leave the Discord server as well.

Ok, I just pushed doing a full clean of guild_space, guild_active, invite, and leaving the discord server.

The clean of invite is not strictly necessary, but I you don't clean it, it might lead to buttons in the "select a space" from the self service mode later where the unbridged space is here but if you click it, you get an error because it cannot join it

Ok, I just pushed doing a full clean of `guild_space`, `guild_active`, `invite`, and leaving the discord server. The clean of `invite` is not strictly necessary, but I you don't clean it, it might lead to buttons in the "select a space" from the self service mode later where the unbridged space is here but if you click it, you get an error because it cannot join it
throw createError({status: 500, message: "Internal Server Error", data: "Some linked room still exists after trying to unlink all of them. Aborting the space unlinking..."})
}
await api.setUserPower(spaceID, me, 0)
await api.leaveRoom(spaceID)
await db.prepare("DELETE FROM guild_space WHERE guild_id=? AND space_id=?").run(guild_id, spaceID)
await db.prepare("DELETE FROM guild_active WHERE guild_id=?").run(guild_id)
await discord.snow.user.leaveGuild(guild_id)

Also, this line causes issue when running the test.
I couldn't add the snow member like createRoom or api in the test to add the function / member definition because it wasn't defined in the type, not really sure what's the best way to fix it

Also, this line causes issue when running the test. I couldn't add the `snow` member like `createRoom` or `api` in the test to add the function / member definition because it wasn't defined in the type, not really sure what's the best way to fix it
await db.prepare("DELETE FROM invite WHERE room_id=?").run(spaceID)
setResponseHeader(event, "HX-Redirect", "/")
return null
}))

View file

@ -666,7 +666,9 @@ test("web unlink room: successfully calls unbridgeDeletedChannel when the channe
})
test("web unlink room: checks that the channel is bridged", async t => {
const row = db.prepare("SELECT * FROM channel_room WHERE channel_id = '665310973967597573'").get()
db.prepare("DELETE FROM channel_room WHERE channel_id = '665310973967597573'").run()
const [error] = await tryToCatch(() => router.test("post", "/api/unlink", {
sessionData: {
managedGuilds: ["665289423482519565"]
@ -677,4 +679,149 @@ test("web unlink room: checks that the channel is bridged", async t => {
}
}))
t.equal(error.data, "Channel ID 665310973967597573 is not currently bridged")
db.prepare("INSERT INTO channel_room (channel_id, room_id, name, nick, thread_parent, custom_avatar, last_bridged_pin_timestamp, speedbump_id, speedbump_checked, speedbump_webhook_id, guild_id, custom_topic) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)").run(row.channel_id, row.room_id, row.name, row.nick, row.thread_parent, row.custom_avatar, row.last_bridged_pin_timestamp, row.speedbump_id, row.speedbump_checked, row.speedbump_webhook_id, row.guild_id, row.custom_topic)

What is the purpose of this database operation?

What is the purpose of this database operation?

The test deletes the row above, to test for checks.
But as I added more tests below related to unlinking, if this is kept deleted, then this cause unwanted errors when trying to run following tests because of inconsistent DB. So I restored it before moving to next test.

The test deletes the row above, to test for checks. But as I added more tests below related to unlinking, if this is kept deleted, then this cause unwanted errors when trying to run following tests because of inconsistent DB. So I restored it before moving to next test.
const new_row = db.prepare("SELECT * FROM channel_room WHERE channel_id = '665310973967597573'").get()
t.deepEqual(row, new_row)
})
// *****
test("web unlink space: access denied if not logged in to Discord", async t => {
const [error] = await tryToCatch(() => router.test("post", "/api/unlink-space", {
body: {
guild_id: "665289423482519565"
}
}))
t.equal(error.data, "Can't edit a guild you don't have Manage Server permissions in")
})
test("web unlink space: checks that guild exists", async t => {
const [error] = await tryToCatch(() => router.test("post", "/api/unlink-space", {
sessionData: {
managedGuilds: ["2"]
},
body: {
guild_id: "2"
}
}))
t.equal(error.data, "Discord guild does not exist or bot has not joined it")
})
test("web unlink space: checks that a space is linked to the guild before trying to unlink the space", async t => {
const row = db.prepare("SELECT * FROM guild_space WHERE guild_id = '665289423482519565'").get()
db.prepare("DELETE FROM guild_space WHERE guild_id = '665289423482519565'").run()
const [error] = await tryToCatch(() => router.test("post", "/api/unlink-space", {
sessionData: {
managedGuilds: ["665289423482519565"]
},
body: {
guild_id: "665289423482519565"
}
}))
t.equal(error.data, "Matrix space does not exist or bot has not linked it")
db.prepare("INSERT INTO guild_space (guild_id, space_id, privacy_level, presence, url_preview) VALUES (?, ?, ?, ?, ?)").run(row.guild_id, row.space_id, row.privacy_level, row.presence, row.url_preview)
const new_row = db.prepare("SELECT * FROM guild_space WHERE guild_id = '665289423482519565'").get()
t.deepEqual(row, new_row)
})
test("web unlink space: correctly abort unlinking if some linked channels remain after trying to unlink them all", async t => {
let unbridgedChannel = false
const [error] = await tryToCatch(() => router.test("post", "/api/unlink-space", {
sessionData: {
managedGuilds: ["665289423482519565"]
},
body: {
guild_id: "665289423482519565",
},
createRoom: {
async unbridgeDeletedChannel(channel, guildID) {
unbridgedChannel = true
t.equal(channel.id, "665310973967597573")
t.equal(guildID, "665289423482519565")
// Do not actually delete the link from DB, should trigger error later in check
}
},
api: {
async *generateFullHierarchy(spaceID) {
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
yield {
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
children_state: [],
guest_can_join: false,
num_joined_members: 2
}
/* c8 ignore next */
},
}
}))
t.equal(error.data, "Some linked room still exists after trying to unlink all of them. Aborting the space unlinking...")
t.equal(unbridgedChannel, true)
})
test("web unlink space: successfully calls unbridgeDeletedChannel on linked channels in space, self-downgrade power level, leave space, and delete link from DB", async t => {
const {reg} = require("../../matrix/read-registration")
const me = `@${reg.sender_localpart}:${reg.ooye.server_name}`
const getLinkRowQuery = "SELECT * FROM guild_space WHERE guild_id = '665289423482519565'"
const row = db.prepare(getLinkRowQuery).get()
t.equal(row.space_id, "!zTMspHVUBhFLLSdmnS:cadence.moe")
let unbridgedChannel = false
let downgradedPowerLevel = false
let leftRoom = false
await router.test("post", "/api/unlink-space", {
sessionData: {
managedGuilds: ["665289423482519565"]
},
body: {
guild_id: "665289423482519565",
},
createRoom: {
async unbridgeDeletedChannel(channel, guildID) {
unbridgedChannel = true
t.equal(channel.id, "665310973967597573")
t.equal(guildID, "665289423482519565")
// In order to not simulate channel deletion and not trigger the post unlink channels, pre-unlink space check
db.prepare("DELETE FROM channel_room WHERE guild_id = '665289423482519565' AND channel_id = '665310973967597573'").run()
}
},
api: {
async *generateFullHierarchy(spaceID) {
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
yield {
room_id: "!NDbIqNpJyPvfKRnNcr:cadence.moe",
children_state: [],
guest_can_join: false,
num_joined_members: 2
}
/* c8 ignore next */
},
async setUserPower(spaceID, targetUser, powerLevel) {
downgradedPowerLevel = true
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
t.equal(targetUser, me)
t.equal(powerLevel, 0)
},
async leaveRoom(spaceID) {
leftRoom = true
t.equal(spaceID, "!zTMspHVUBhFLLSdmnS:cadence.moe")
},
}
})
t.equal(unbridgedChannel, true)
t.equal(downgradedPowerLevel, true)
t.equal(leftRoom, true)
const missed_row = db.prepare(getLinkRowQuery).get()
t.equal(missed_row, undefined)
})