refactor channel positions in guild

src/api/routes/guilds/#guild_id/channels.ts

@@ -22,10 +22,10 @@ import {
 	ChannelModifySchema,
 	ChannelReorderSchema,
 	ChannelUpdateEvent,
+	Guild,
 	emitEvent,
 } from "@spacebar/util";
 import { Request, Response, Router } from "express";
-import { HTTPError } from "lambert-server";
 const router = Router();
 
 router.get(
@@ -96,44 +96,72 @@ router.patch(
 		const { guild_id } = req.params;
 		const body = req.body as ChannelReorderSchema;
 
-		await Promise.all([
-			body.map(async (x) => {
-				if (x.position == null && !x.parent_id)
-					throw new HTTPError(
-						`You need to at least specify position or parent_id`,
-						400,
-					);
+		const guild = await Guild.findOneOrFail({
+			where: { id: guild_id },
+			select: { channelOrdering: true },
+		});
 
-				const opts: Partial<Channel> = {};
-				if (x.position != null) opts.position = x.position;
+		// The channels not listed for this query
+		const notMentioned = guild.channelOrdering.filter(
+			(x) => !body.find((c) => c.id == x),
+		);
 
-				if (x.parent_id) {
-					opts.parent_id = x.parent_id;
-					const parent_channel = await Channel.findOneOrFail({
-						where: { id: x.parent_id, guild_id },
-						select: ["permission_overwrites"],
-					});
-					if (x.lock_permissions) {
-						opts.permission_overwrites =
-							parent_channel.permission_overwrites;
-					}
-				}
+		const withParents = body.filter((x) => x.parent_id != undefined);
+		const withPositions = body.filter((x) => x.position != undefined);
 
-				await Channel.update({ guild_id, id: x.id }, opts);
+		await Promise.all(
+			withPositions.map(async (opt) => {
 				const channel = await Channel.findOneOrFail({
-					where: { guild_id, id: x.id },
+					where: { id: opt.id },
 				});
 
+				channel.position = opt.position as number;
+				notMentioned.splice(opt.position as number, 0, channel.id);
+
 				await emitEvent({
 					event: "CHANNEL_UPDATE",
 					data: channel,
-					channel_id: x.id,
+					channel_id: channel.id,
 					guild_id,
 				} as ChannelUpdateEvent);
 			}),
-		]);
+		);
 
-		res.sendStatus(204);
+		// have to do the parents after the positions
+		await Promise.all(
+			withParents.map(async (opt) => {
+				const [channel, parent] = await Promise.all([
+					Channel.findOneOrFail({
+						where: { id: opt.id },
+					}),
+					Channel.findOneOrFail({
+						where: { id: opt.parent_id as string },
+						select: { permission_overwrites: true },
+					}),
+				]);
+
+				if (opt.lock_permissions)
+					await Channel.update(
+						{ id: channel.id },
+						{ permission_overwrites: parent.permission_overwrites },
+					);
+
+				const parentPos = notMentioned.indexOf(parent.id);
+				notMentioned.splice(parentPos + 1, 0, channel.id);
+				channel.position = (parentPos + 1) as number;
+
+				await emitEvent({
+					event: "CHANNEL_UPDATE",
+					data: channel,
+					channel_id: channel.id,
+					guild_id,
+				} as ChannelUpdateEvent);
+			}),
+		);
+
+		await Guild.update({ id: guild_id }, { channelOrdering: notMentioned });
+
+		return res.sendStatus(204);
 	},
 );
 

src/api/routes/guilds/#guild_id/index.ts

@@ -161,12 +161,6 @@ router.patch(
 		guild.assign(body);
 
 		if (body.public_updates_channel_id == "1") {
-			// move all channels up 1
-			await Channel.createQueryBuilder("channels")
-				.where({ guild: { id: guild_id } })
-				.update({ position: () => "position + 1" })
-				.execute();
-
 			// create an updates channel for them
 			const channel = await Channel.createChannel(
 				{
@@ -188,6 +182,8 @@ router.patch(
 				{ skipPermissionCheck: true },
 			);
 
+			await Guild.insertChannelInOrder(guild.id, channel.id, 0, guild);
+
 			guild.public_updates_channel_id = channel.id;
 		} else if (body.public_updates_channel_id != undefined) {
 			// ensure channel exists in this guild
@@ -198,12 +194,6 @@ router.patch(
 		}
 
 		if (body.rules_channel_id == "1") {
-			// move all channels up 1
-			await Channel.createQueryBuilder("channels")
-				.where({ guild: { id: guild_id } })
-				.update({ position: () => "position + 1" })
-				.execute();
-
 			// create a rules for them
 			const channel = await Channel.createChannel(
 				{
@@ -225,6 +215,8 @@ router.patch(
 				{ skipPermissionCheck: true },
 			);
 
+			await Guild.insertChannelInOrder(guild.id, channel.id, 0, guild);
+
 			guild.rules_channel_id = channel.id;
 		} else if (body.rules_channel_id != undefined) {
 			// ensure channel exists in this guild

src/api/routes/guilds/#guild_id/widget.json.ts

@@ -77,12 +77,7 @@ router.get(
 		// Fetch voice channels, and the @everyone permissions object
 		const channels: { id: string; name: string; position: number }[] = [];
 
-		(
-			await Channel.find({
-				where: { guild_id: guild_id, type: 2 },
-				order: { position: "ASC" },
-			})
-		).filter((doc) => {
+		(await Channel.getOrderedChannels(guild.id, guild)).filter((doc) => {
 			// Only return channels where @everyone has the CONNECT permission
 			if (
 				doc.permission_overwrites === undefined ||

src/gateway/opcodes/Identify.ts

@@ -54,6 +54,7 @@ import {
 	UserSettings,
 	checkToken,
 	emitEvent,
+	getDatabase,
 } from "@spacebar/util";
 import { check } from "./instanceOf";
 
@@ -167,7 +168,12 @@ export async function onIdentify(this: WebSocket, data: Payload) {
 					// guild channels, emoji, roles, stickers
 					// but we do want almost everything from guild.
 					// How do you do that without just enumerating the guild props?
-					guild: true,
+					guild: Object.fromEntries(
+						// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+						getDatabase()!
+							.getMetadata(Guild)
+							.columns.map((x) => [x.propertyName, true]),
+					),
 				},
 				relations: [
 					"guild",
@@ -253,18 +259,26 @@ export async function onIdentify(this: WebSocket, data: Payload) {
 	const guilds: GuildOrUnavailable[] = members.map((member) => {
 		// filter guild channels we don't have permission to view
 		// TODO: check if this causes issues when the user is granted other roles?
-		member.guild.channels = member.guild.channels.filter((channel) => {
-			const perms = Permissions.finalPermission({
-				user: {
-					id: member.id,
-					roles: member.roles.map((x) => x.id),
-				},
-				guild: member.guild,
-				channel,
-			});
+		member.guild.channels = member.guild.channels
+			.filter((channel) => {
+				const perms = Permissions.finalPermission({
+					user: {
+						id: member.id,
+						roles: member.roles.map((x) => x.id),
+					},
+					guild: member.guild,
+					channel,
+				});
 
-			return perms.has("VIEW_CHANNEL");
-		});
+				return perms.has("VIEW_CHANNEL");
+			})
+			.map((channel) => {
+				channel.position = member.guild.channelOrdering.indexOf(
+					channel.id,
+				);
+				return channel;
+			})
+			.sort((a, b) => a.position - b.position);
 
 		if (user.bot) {
 			pending_guilds.push(member.guild);

src/util/entities/Channel.ts

@@ -126,9 +126,6 @@ export class Channel extends BaseClass {
 	@Column({ nullable: true })
 	default_auto_archive_duration?: number;
 
-	@Column({ nullable: true })
-	position?: number;
-
 	@Column({ type: "simple-json", nullable: true })
 	permission_overwrites?: ChannelPermissionOverwrite[];
 
@@ -193,6 +190,9 @@ export class Channel extends BaseClass {
 	@Column()
 	default_thread_rate_limit_per_user: number = 0;
 
+	/** Must be calculated Channel.calculatePosition */
+	position: number;
+
 	// TODO: DM channel
 	static async createChannel(
 		channel: Partial<Channel>,
@@ -211,10 +211,16 @@ export class Channel extends BaseClass {
 			permissions.hasThrow("MANAGE_CHANNELS");
 		}
 
+		const guild = await Guild.findOneOrFail({
+			where: { id: channel.guild_id },
+			select: {
+				features: !opts?.skipNameChecks,
+				channelOrdering: true,
+				id: true,
+			},
+		});
+
 		if (!opts?.skipNameChecks) {
-			const guild = await Guild.findOneOrFail({
-				where: { id: channel.guild_id },
-			});
 			if (
 				!guild.features.includes("ALLOW_INVALID_CHANNEL_NAMES") &&
 				channel.name
@@ -293,14 +299,15 @@ export class Channel extends BaseClass {
 		if (!channel.permission_overwrites) channel.permission_overwrites = [];
 		// TODO: eagerly auto generate position of all guild channels
 
+		const position =
+			(channel.type === ChannelType.UNHANDLED ? 0 : channel.position) ||
+			0;
+
 		channel = {
 			...channel,
 			...(!opts?.keepId && { id: Snowflake.generate() }),
 			created_at: new Date(),
-			position:
-				(channel.type === ChannelType.UNHANDLED
-					? 0
-					: channel.position) || 0,
+			position,
 		};
 
 		const ret = Channel.create(channel);
@@ -314,6 +321,7 @@ export class Channel extends BaseClass {
 						guild_id: channel.guild_id,
 				  } as ChannelCreateEvent)
 				: Promise.resolve(),
+			Guild.insertChannelInOrder(guild.id, ret.id, position, guild),
 		]);
 
 		return ret;
@@ -456,6 +464,40 @@ export class Channel extends BaseClass {
 		await Channel.delete({ id: channel.id });
 	}
 
+	static async calculatePosition(
+		channel_id: string,
+		guild_id: string,
+		guild?: Guild,
+	) {
+		if (!guild)
+			guild = await Guild.findOneOrFail({
+				where: { id: guild_id },
+				select: { channelOrdering: true },
+			});
+
+		return guild.channelOrdering.findIndex((id) => channel_id == id);
+	}
+
+	static async getOrderedChannels(guild_id: string, guild?: Guild) {
+		if (!guild)
+			guild = await Guild.findOneOrFail({
+				where: { id: guild_id },
+				select: { channelOrdering: true },
+			});
+
+		const channels = await Promise.all(
+			guild.channelOrdering.map((id) =>
+				Channel.findOneOrFail({ where: { id } }),
+			),
+		);
+
+		return channels.reduce((r, v) => {
+			v.position = (guild as Guild).channelOrdering.indexOf(v.id);
+			r[v.position] = v;
+			return r;
+		}, [] as Array<Channel>);
+	}
+
 	isDm() {
 		return (
 			this.type === ChannelType.DM || this.type === ChannelType.GROUP_DM

src/util/entities/Guild.ts

@@ -297,6 +297,9 @@ export class Guild extends BaseClass {
 	@Column({ nullable: true })
 	premium_progress_bar_enabled: boolean = false;
 
+	@Column({ select: false, type: "simple-array" })
+	channelOrdering: string[];
+
 	static async createGuild(body: {
 		name?: string;
 		icon?: string | null;
@@ -324,6 +327,7 @@ export class Guild extends BaseClass {
 				description: "",
 				welcome_channels: [],
 			},
+			channelOrdering: [],
 
 			afk_timeout: Config.get().defaults.guild.afkTimeout,
 			default_message_notifications:
@@ -376,7 +380,7 @@ export class Guild extends BaseClass {
 
 			const parent_id = ids.get(channel.parent_id);
 
-			await Channel.createChannel(
+			const saved = await Channel.createChannel(
 				{ ...channel, guild_id, id, parent_id },
 				body.owner_id,
 				{
@@ -386,15 +390,69 @@ export class Guild extends BaseClass {
 					skipEventEmit: true,
 				},
 			);
+
+			await Guild.insertChannelInOrder(
+				guild.id,
+				saved.id,
+				parent_id ?? channel.position ?? 0,
+				guild,
+			);
 		}
 
 		return guild;
 	}
 
-	toJSON() {
+	/** Insert a channel into the guild ordering by parent channel id or position */
+	static async insertChannelInOrder(
+		guild_id: string,
+		channel_id: string,
+		position: number,
+		guild?: Guild,
+	): Promise<number>;
+	static async insertChannelInOrder(
+		guild_id: string,
+		channel_id: string,
+		parent_id: string,
+		guild?: Guild,
+	): Promise<number>;
+	static async insertChannelInOrder(
+		guild_id: string,
+		channel_id: string,
+		insertPoint: string | number,
+		guild?: Guild,
+	): Promise<number>;
+	static async insertChannelInOrder(
+		guild_id: string,
+		channel_id: string,
+		insertPoint: string | number,
+		guild?: Guild,
+	): Promise<number> {
+		if (!guild)
+			guild = await Guild.findOneOrFail({
+				where: { id: guild_id },
+				select: { channelOrdering: true },
+			});
+
+		let position;
+		if (typeof insertPoint == "string")
+			position = guild.channelOrdering.indexOf(insertPoint) + 1;
+		else position = insertPoint;
+
+		guild.channelOrdering.remove(channel_id);
+
+		guild.channelOrdering.splice(position, 0, channel_id);
+		await Guild.update(
+			{ id: guild_id },
+			{ channelOrdering: guild.channelOrdering },
+		);
+		return position;
+	}
+
+	toJSON(): Guild {
 		return {
 			...this,
 			unavailable: this.unavailable == false ? undefined : true,
+			channelOrdering: undefined,
 		};
 	}
 }

src/util/migration/mariadb/1696420827239-guildChannelOrdering.ts

@@ -0,0 +1,40 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+
+export class guildChannelOrdering1696420827239 implements MigrationInterface {
+	name = "guildChannelOrdering1696420827239";
+
+	public async up(queryRunner: QueryRunner): Promise<void> {
+		const guilds = await queryRunner.query(
+			`SELECT id FROM guilds`,
+			undefined,
+			true,
+		);
+
+		await queryRunner.query(
+			`ALTER TABLE guilds ADD channelOrdering text NOT NULL DEFAULT '[]'`,
+		);
+
+		for (const guild_id of guilds.records.map((x) => x.id)) {
+			const channels: Array<{ position: number; id: string }> = (
+				await queryRunner.query(
+					`SELECT id, position FROM channels WHERE guild_id = ?`,
+					[guild_id],
+					true,
+				)
+			).records;
+
+			channels.sort((a, b) => a.position - b.position);
+
+			await queryRunner.query(
+				`UPDATE guilds SET channelOrdering = ? WHERE id = ?`,
+				[JSON.stringify(channels.map((x) => x.id)), guild_id],
+			);
+		}
+
+		await queryRunner.query(`ALTER TABLE channels DROP COLUMN position`);
+	}
+
+	public async down(): Promise<void> {
+		// don't care actually, sorry.
+	}
+}

src/util/migration/mysql/1696420827239-guildChannelOrdering.ts

@@ -0,0 +1,40 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+
+export class guildChannelOrdering1696420827239 implements MigrationInterface {
+	name = "guildChannelOrdering1696420827239";
+
+	public async up(queryRunner: QueryRunner): Promise<void> {
+		const guilds = await queryRunner.query(
+			`SELECT id FROM guilds`,
+			undefined,
+			true,
+		);
+
+		await queryRunner.query(
+			`ALTER TABLE guilds ADD channelOrdering text NOT NULL DEFAULT '[]'`,
+		);
+
+		for (const guild_id of guilds.records.map((x) => x.id)) {
+			const channels: Array<{ position: number; id: string }> = (
+				await queryRunner.query(
+					`SELECT id, position FROM channels WHERE guild_id = ?`,
+					[guild_id],
+					true,
+				)
+			).records;
+
+			channels.sort((a, b) => a.position - b.position);
+
+			await queryRunner.query(
+				`UPDATE guilds SET channelOrdering = ? WHERE id = ?`,
+				[JSON.stringify(channels.map((x) => x.id)), guild_id],
+			);
+		}
+
+		await queryRunner.query(`ALTER TABLE channels DROP COLUMN position`);
+	}
+
+	public async down(): Promise<void> {
+		// don't care actually, sorry.
+	}
+}

src/util/migration/postgres/1696420827239-guildChannelOrdering.ts

@@ -0,0 +1,40 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+
+export class guildChannelOrdering1696420827239 implements MigrationInterface {
+	name = "guildChannelOrdering1696420827239";
+
+	public async up(queryRunner: QueryRunner): Promise<void> {
+		const guilds = await queryRunner.query(
+			`SELECT id FROM guilds`,
+			undefined,
+			true,
+		);
+
+		await queryRunner.query(
+			`ALTER TABLE guilds ADD channelOrdering text NOT NULL DEFAULT '[]'`,
+		);
+
+		for (const guild_id of guilds.records.map((x) => x.id)) {
+			const channels: Array<{ position: number; id: string }> = (
+				await queryRunner.query(
+					`SELECT id, position FROM channels WHERE guild_id = $1`,
+					[guild_id],
+					true,
+				)
+			).records;
+
+			channels.sort((a, b) => a.position - b.position);
+
+			await queryRunner.query(
+				`UPDATE guilds SET channelOrdering = $1 WHERE id = $2`,
+				[JSON.stringify(channels.map((x) => x.id)), guild_id],
+			);
+		}
+
+		await queryRunner.query(`ALTER TABLE channels DROP COLUMN position`);
+	}
+
+	public async down(): Promise<void> {
+		// don't care actually, sorry.
+	}
+}