fix: channel ordering

This commit is contained in:
Hampus Kraft 2026-02-20 22:36:48 +00:00
parent fcc8463cd8
commit e11f9bc52e
No known key found for this signature in database
GPG Key ID: 6090864C465A454D
10 changed files with 438 additions and 19 deletions

View File

@ -123,6 +123,7 @@ export async function moveChannel(guildId: string, operation: ChannelMoveOperati
{
id: operation.channelId,
parent_id: operation.newParentId,
preceding_sibling_id: operation.precedingSiblingId,
lock_permissions: false,
position: operation.position,
},

View File

@ -0,0 +1,180 @@
/*
* Copyright (C) 2026 Fluxer Contributors
*
* This file is part of Fluxer.
*
* Fluxer is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* Fluxer is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Fluxer. If not, see <https://www.gnu.org/licenses/>.
*/
import {DND_TYPES, type DragItem, type DropResult} from '@app/components/layout/types/DndTypes';
import {createChannelMoveOperation} from '@app/components/layout/utils/ChannelMoveOperation';
import {ChannelRecord} from '@app/records/ChannelRecord';
import {ChannelTypes} from '@fluxer/constants/src/ChannelConstants';
import {describe, expect, it} from 'vitest';
const TEST_GUILD_ID = 'guild';
function createChannel(params: {id: string; type: number; position: number; parentId?: string | null}): ChannelRecord {
return new ChannelRecord({
id: params.id,
guild_id: TEST_GUILD_ID,
name: params.id,
type: params.type,
position: params.position,
parent_id: params.parentId ?? null,
});
}
function createDragItem(channel: ChannelRecord): DragItem {
return {
type: DND_TYPES.CHANNEL,
id: channel.id,
channelType: channel.type,
parentId: channel.parentId,
guildId: TEST_GUILD_ID,
};
}
describe('ChannelMoveOperation', () => {
it('moves a text channel into a voice-only category at the top', () => {
const textCategory = createChannel({id: 'text-category', type: ChannelTypes.GUILD_CATEGORY, position: 0});
const voiceCategory = createChannel({id: 'voice-category', type: ChannelTypes.GUILD_CATEGORY, position: 1});
const generalText = createChannel({
id: 'general',
type: ChannelTypes.GUILD_TEXT,
position: 2,
parentId: textCategory.id,
});
const generalVoice = createChannel({
id: 'General',
type: ChannelTypes.GUILD_VOICE,
position: 3,
parentId: voiceCategory.id,
});
const operation = createChannelMoveOperation({
channels: [textCategory, voiceCategory, generalText, generalVoice],
dragItem: createDragItem(generalText),
dropResult: {
targetId: voiceCategory.id,
position: 'inside',
targetParentId: voiceCategory.id,
},
});
expect(operation).toEqual({
channelId: generalText.id,
newParentId: voiceCategory.id,
precedingSiblingId: null,
position: 0,
});
});
it('keeps text channels above voice channels even when dropped after a voice channel', () => {
const voiceCategory = createChannel({id: 'voice-category', type: ChannelTypes.GUILD_CATEGORY, position: 0});
const generalText = createChannel({id: 'general', type: ChannelTypes.GUILD_TEXT, position: 1});
const generalVoice = createChannel({
id: 'General',
type: ChannelTypes.GUILD_VOICE,
position: 2,
parentId: voiceCategory.id,
});
const dropResult: DropResult = {
targetId: generalVoice.id,
position: 'after',
targetParentId: voiceCategory.id,
};
const operation = createChannelMoveOperation({
channels: [voiceCategory, generalText, generalVoice],
dragItem: createDragItem(generalText),
dropResult,
});
expect(operation).toEqual({
channelId: generalText.id,
newParentId: voiceCategory.id,
precedingSiblingId: null,
position: 0,
});
});
it('keeps moved voice channels below all text siblings', () => {
const textCategory = createChannel({id: 'text-category', type: ChannelTypes.GUILD_CATEGORY, position: 0});
const voiceCategory = createChannel({id: 'voice-category', type: ChannelTypes.GUILD_CATEGORY, position: 1});
const textOne = createChannel({
id: 'text-one',
type: ChannelTypes.GUILD_TEXT,
position: 2,
parentId: textCategory.id,
});
const textTwo = createChannel({
id: 'text-two',
type: ChannelTypes.GUILD_TEXT,
position: 3,
parentId: textCategory.id,
});
const lounge = createChannel({
id: 'lounge',
type: ChannelTypes.GUILD_VOICE,
position: 4,
parentId: voiceCategory.id,
});
const operation = createChannelMoveOperation({
channels: [textCategory, voiceCategory, textOne, textTwo, lounge],
dragItem: createDragItem(lounge),
dropResult: {
targetId: textOne.id,
position: 'before',
targetParentId: textCategory.id,
},
});
expect(operation).toEqual({
channelId: lounge.id,
newParentId: textCategory.id,
precedingSiblingId: textTwo.id,
position: 2,
});
});
it('returns null when dropping to the same effective placement', () => {
const category = createChannel({id: 'category', type: ChannelTypes.GUILD_CATEGORY, position: 0});
const textOne = createChannel({
id: 'text-one',
type: ChannelTypes.GUILD_TEXT,
position: 1,
parentId: category.id,
});
const textTwo = createChannel({
id: 'text-two',
type: ChannelTypes.GUILD_TEXT,
position: 2,
parentId: category.id,
});
const operation = createChannelMoveOperation({
channels: [category, textOne, textTwo],
dragItem: createDragItem(textOne),
dropResult: {
targetId: textTwo.id,
position: 'before',
targetParentId: category.id,
},
});
expect(operation).toBeNull();
});
});

View File

@ -114,6 +114,10 @@ export function GuildChannelController(app: HonoApp) {
channelId: createChannelID(item.id),
position: item.position,
parentId: item.parent_id == null ? item.parent_id : createChannelID(item.parent_id),
precedingSiblingId:
item.preceding_sibling_id == null
? item.preceding_sibling_id
: createChannelID(item.preceding_sibling_id),
lockPermissions: item.lock_permissions ?? false,
})),
requestCache,

View File

@ -112,6 +112,7 @@ export class GuildChannelService {
channelId: ChannelID;
position?: number;
parentId: ChannelID | null | undefined;
precedingSiblingId: ChannelID | null | undefined;
lockPermissions: boolean;
}>;
requestCache: RequestCache;

View File

@ -803,6 +803,7 @@ export class GuildService {
channelId: ChannelID;
position?: number;
parentId: ChannelID | null | undefined;
precedingSiblingId: ChannelID | null | undefined;
lockPermissions: boolean;
}>;
requestCache: RequestCache;

View File

@ -401,6 +401,7 @@ export class ChannelOperationsService {
channelId: ChannelID;
position?: number;
parentId: ChannelID | null | undefined;
precedingSiblingId: ChannelID | null | undefined;
lockPermissions: boolean;
}>;
requestCache: RequestCache;
@ -424,6 +425,11 @@ export class ChannelOperationsService {
if (update.parentId && !channelMap.has(update.parentId)) {
throw InputValidationError.fromCode('parent_id', ValidationErrorCodes.INVALID_PARENT_CHANNEL);
}
if (update.precedingSiblingId && !channelMap.has(update.precedingSiblingId)) {
throw InputValidationError.fromCode('preceding_sibling_id', ValidationErrorCodes.INVALID_CHANNEL_ID, {
channelId: update.precedingSiblingId.toString(),
});
}
}
const viewable = new Set(await this.gatewayService.getViewableChannels({guildId, userId}));
@ -435,6 +441,9 @@ export class ChannelOperationsService {
if (update.parentId && !viewable.has(update.parentId)) {
throw new MissingPermissionsError();
}
if (update.precedingSiblingId && !viewable.has(update.precedingSiblingId)) {
throw new MissingPermissionsError();
}
}
for (const update of updates) {
@ -453,7 +462,13 @@ export class ChannelOperationsService {
private async applySinglePositionUpdate(params: {
guildId: GuildID;
userId: UserID;
update: {channelId: ChannelID; position?: number; parentId: ChannelID | null | undefined; lockPermissions: boolean};
update: {
channelId: ChannelID;
position?: number;
parentId: ChannelID | null | undefined;
precedingSiblingId: ChannelID | null | undefined;
lockPermissions: boolean;
};
requestCache: RequestCache;
}): Promise<void> {
const {guildId, update, requestCache} = params;
@ -478,26 +493,29 @@ export class ChannelOperationsService {
throw InputValidationError.fromCode('parent_id', ValidationErrorCodes.CATEGORIES_CANNOT_HAVE_PARENTS);
}
const orderedChannels = sortChannelsForOrdering(allChannels);
const siblings = orderedChannels.filter((ch) => (ch.parentId ?? null) === desiredParent);
const blockIds = computeChannelMoveBlockIds({channels: orderedChannels, targetId: target.id});
const siblingsWithoutBlock = siblings.filter((ch) => !blockIds.has(ch.id));
let precedingSibling = update.precedingSiblingId ?? null;
if (update.precedingSiblingId === undefined) {
const orderedChannels = sortChannelsForOrdering(allChannels);
const siblings = orderedChannels.filter((ch) => (ch.parentId ?? null) === desiredParent);
const blockIds = computeChannelMoveBlockIds({channels: orderedChannels, targetId: target.id});
const siblingsWithoutBlock = siblings.filter((ch) => !blockIds.has(ch.id));
let insertIndex = siblingsWithoutBlock.length;
if (update.position !== undefined) {
const adjustedPosition = Math.max(update.position, 0);
insertIndex = Math.min(adjustedPosition, siblingsWithoutBlock.length);
} else {
const isVoice = target.type === ChannelTypes.GUILD_VOICE;
if (isVoice) {
insertIndex = siblingsWithoutBlock.length;
let insertIndex = siblingsWithoutBlock.length;
if (update.position !== undefined) {
const adjustedPosition = Math.max(update.position, 0);
insertIndex = Math.min(adjustedPosition, siblingsWithoutBlock.length);
} else {
const firstVoice = siblingsWithoutBlock.findIndex((ch) => ch.type === ChannelTypes.GUILD_VOICE);
insertIndex = firstVoice === -1 ? siblingsWithoutBlock.length : firstVoice;
const isVoice = target.type === ChannelTypes.GUILD_VOICE;
if (isVoice) {
insertIndex = siblingsWithoutBlock.length;
} else {
const firstVoice = siblingsWithoutBlock.findIndex((ch) => ch.type === ChannelTypes.GUILD_VOICE);
insertIndex = firstVoice === -1 ? siblingsWithoutBlock.length : firstVoice;
}
}
}
const precedingSibling = insertIndex === 0 ? null : siblingsWithoutBlock[insertIndex - 1].id;
precedingSibling = insertIndex === 0 ? null : siblingsWithoutBlock[insertIndex - 1].id;
}
await this.executeChannelReorder({
guildId,

View File

@ -33,6 +33,7 @@ import {type ApiTestHarness, createApiTestHarness} from '@fluxer/api/src/test/Ap
import {HTTP_STATUS} from '@fluxer/api/src/test/TestConstants';
import {createBuilder} from '@fluxer/api/src/test/TestRequestBuilder';
import {ChannelTypes, Permissions} from '@fluxer/constants/src/ChannelConstants';
import {ValidationErrorCodes} from '@fluxer/constants/src/ValidationErrorCodes';
import {afterEach, beforeEach, describe, expect, test} from 'vitest';
describe('Guild Channel Positions', () => {
@ -257,4 +258,163 @@ describe('Guild Channel Positions', () => {
.sort((a, b) => (a.position ?? 0) - (b.position ?? 0));
expect(destinationSiblings.map((channel) => channel.id)).toEqual([textOne.id, textTwo.id, voiceChannel.id]);
});
test('should move default general text under the default voice category', async () => {
const account = await createTestAccount(harness);
const guild = await createGuild(harness, account.token, 'Test Guild');
const channels = await getGuildChannels(harness, account.token, guild.id);
const general = channels.find((channel) => channel.type === ChannelTypes.GUILD_TEXT && channel.name === 'general');
const voiceCategory = channels.find(
(channel) => channel.type === ChannelTypes.GUILD_CATEGORY && channel.name === 'Voice Channels',
);
const generalVoice = channels.find(
(channel) => channel.type === ChannelTypes.GUILD_VOICE && channel.name === 'General',
);
expect(general).toBeDefined();
expect(voiceCategory).toBeDefined();
expect(generalVoice).toBeDefined();
await updateChannelPositions(harness, account.token, guild.id, [
{
id: general!.id,
parent_id: voiceCategory!.id,
position: 0,
},
]);
const updatedChannels = await getGuildChannels(harness, account.token, guild.id);
const voiceCategorySiblings = updatedChannels
.filter((channel) => channel.parent_id === voiceCategory!.id)
.sort((a, b) => (a.position ?? 0) - (b.position ?? 0));
expect(voiceCategorySiblings.map((channel) => channel.id)).toEqual([general!.id, generalVoice!.id]);
});
test('should prioritise preceding_sibling_id over position when both are provided', async () => {
const account = await createTestAccount(harness);
const guild = await createGuild(harness, account.token, 'Test Guild');
const category = await createChannel(harness, account.token, guild.id, 'Category', ChannelTypes.GUILD_CATEGORY);
const textOne = await createChannel(harness, account.token, guild.id, 'one', ChannelTypes.GUILD_TEXT);
const textTwo = await createChannel(harness, account.token, guild.id, 'two', ChannelTypes.GUILD_TEXT);
const textThree = await createChannel(harness, account.token, guild.id, 'three', ChannelTypes.GUILD_TEXT);
await updateChannelPositions(harness, account.token, guild.id, [
{id: textOne.id, parent_id: category.id},
{id: textTwo.id, parent_id: category.id},
{id: textThree.id, parent_id: category.id},
]);
await updateChannelPositions(harness, account.token, guild.id, [
{
id: textThree.id,
parent_id: category.id,
position: 0,
preceding_sibling_id: textOne.id,
},
]);
const channels = await getGuildChannels(harness, account.token, guild.id);
const siblings = channels
.filter((channel) => channel.parent_id === category.id)
.sort((a, b) => (a.position ?? 0) - (b.position ?? 0));
expect(siblings.map((channel) => channel.id)).toEqual([textOne.id, textThree.id, textTwo.id]);
});
test('should reject preceding sibling from a different parent', async () => {
const account = await createTestAccount(harness);
const guild = await createGuild(harness, account.token, 'Test Guild');
const categoryOne = await createChannel(harness, account.token, guild.id, 'Cat 1', ChannelTypes.GUILD_CATEGORY);
const categoryTwo = await createChannel(harness, account.token, guild.id, 'Cat 2', ChannelTypes.GUILD_CATEGORY);
const textOne = await createChannel(harness, account.token, guild.id, 'one', ChannelTypes.GUILD_TEXT);
const textTwo = await createChannel(harness, account.token, guild.id, 'two', ChannelTypes.GUILD_TEXT);
await updateChannelPositions(harness, account.token, guild.id, [
{id: textOne.id, parent_id: categoryOne.id},
{id: textTwo.id, parent_id: categoryTwo.id},
]);
const response = await createBuilder<{
code: string;
errors: Array<{path: string; code: string; message: string}>;
}>(harness, account.token)
.patch(`/guilds/${guild.id}/channels`)
.body([
{
id: textOne.id,
parent_id: categoryOne.id,
preceding_sibling_id: textTwo.id,
},
])
.expect(HTTP_STATUS.BAD_REQUEST)
.execute();
expect(response.code).toBe('INVALID_FORM_BODY');
expect(response.errors[0]?.path).toBe('preceding_sibling_id');
expect(response.errors[0]?.code).toBe(ValidationErrorCodes.PRECEDING_CHANNEL_MUST_SHARE_PARENT);
});
test('should reject positioning a category relative to its own child', async () => {
const account = await createTestAccount(harness);
const guild = await createGuild(harness, account.token, 'Test Guild');
const category = await createChannel(harness, account.token, guild.id, 'Cat', ChannelTypes.GUILD_CATEGORY);
const child = await createChannel(harness, account.token, guild.id, 'child', ChannelTypes.GUILD_TEXT);
await updateChannelPositions(harness, account.token, guild.id, [{id: child.id, parent_id: category.id}]);
const response = await createBuilder<{
code: string;
errors: Array<{path: string; code: string; message: string}>;
}>(harness, account.token)
.patch(`/guilds/${guild.id}/channels`)
.body([
{
id: category.id,
parent_id: null,
preceding_sibling_id: child.id,
},
])
.expect(HTTP_STATUS.BAD_REQUEST)
.execute();
expect(response.code).toBe('INVALID_FORM_BODY');
expect(response.errors[0]?.path).toBe('preceding_sibling_id');
expect(response.errors[0]?.code).toBe(ValidationErrorCodes.CANNOT_POSITION_CHANNEL_RELATIVE_TO_ITSELF);
});
test('should reject text channels being positioned below voice channels via preceding_sibling_id', async () => {
const account = await createTestAccount(harness);
const guild = await createGuild(harness, account.token, 'Test Guild');
const category = await createChannel(harness, account.token, guild.id, 'Mixed', ChannelTypes.GUILD_CATEGORY);
const text = await createChannel(harness, account.token, guild.id, 'text', ChannelTypes.GUILD_TEXT);
const voice = await createChannel(harness, account.token, guild.id, 'voice', ChannelTypes.GUILD_VOICE);
await updateChannelPositions(harness, account.token, guild.id, [
{id: text.id, parent_id: category.id},
{id: voice.id, parent_id: category.id},
]);
const response = await createBuilder<{
code: string;
errors: Array<{path: string; code: string; message: string}>;
}>(harness, account.token)
.patch(`/guilds/${guild.id}/channels`)
.body([
{
id: text.id,
parent_id: category.id,
preceding_sibling_id: voice.id,
},
])
.expect(HTTP_STATUS.BAD_REQUEST)
.execute();
expect(response.code).toBe('INVALID_FORM_BODY');
expect(response.errors[0]?.path).toBe('preceding_sibling_id');
expect(response.errors[0]?.code).toBe(ValidationErrorCodes.VOICE_CHANNELS_CANNOT_BE_ABOVE_TEXT_CHANNELS);
});
});

View File

@ -97,7 +97,13 @@ export async function updateChannelPositions(
harness: ApiTestHarness,
token: string,
guildId: string,
positions: Array<{id: string; position?: number; lock_permissions?: boolean | null; parent_id?: string | null}>,
positions: Array<{
id: string;
position?: number;
lock_permissions?: boolean | null;
parent_id?: string | null;
preceding_sibling_id?: string | null;
}>,
): Promise<void> {
await createBuilder(harness, token).patch(`/guilds/${guildId}/channels`).body(positions).expect(204).execute();
}

View File

@ -234,6 +234,9 @@ export const ChannelPositionUpdateRequest = z.array(
id: SnowflakeType.describe('The ID of the channel to reposition'),
position: z.number().int().nonnegative().optional().describe('New position for the channel'),
parent_id: SnowflakeType.nullish().describe('New parent category ID'),
preceding_sibling_id: SnowflakeType.nullish().describe(
'ID of the sibling channel that should directly precede this channel after reordering',
),
lock_permissions: z.boolean().optional().describe('Whether to sync permissions with the new parent'),
}),
);

View File

@ -67,7 +67,52 @@ export function compareChannelOrdering<Id extends string | bigint>(
export function sortChannelsForOrdering<Id extends string | bigint, Channel extends ChannelOrderingChannel<Id>>(
channels: ReadonlyArray<Channel>,
): Array<Channel> {
return [...channels].sort(compareChannelOrdering);
const channelById = new Map<Id, Channel>(channels.map((channel) => [channel.id, channel]));
const childrenByParent = new Map<Id, Array<Channel>>();
const rootChannels: Array<Channel> = [];
for (const channel of channels) {
const parentId = channel.parentId ?? null;
if (parentId === null || !channelById.has(parentId)) {
rootChannels.push(channel);
continue;
}
const existingChildren = childrenByParent.get(parentId);
if (existingChildren) {
existingChildren.push(channel);
} else {
childrenByParent.set(parentId, [channel]);
}
}
const orderedChannels: Array<Channel> = [];
const seen = new Set<Id>();
const sortedRoots = [...rootChannels].sort(compareChannelOrdering);
for (const root of sortedRoots) {
orderedChannels.push(root);
seen.add(root.id);
if (root.type !== ChannelTypes.GUILD_CATEGORY) {
continue;
}
const children = childrenByParent.get(root.id);
if (!children) {
continue;
}
for (const child of [...children].sort(compareChannelOrdering)) {
orderedChannels.push(child);
seen.add(child.id);
}
}
const remaining = channels.filter((channel) => !seen.has(channel.id)).sort(compareChannelOrdering);
orderedChannels.push(...remaining);
return orderedChannels;
}
export function computeChannelMoveBlockIds<Id extends string | bigint, Channel extends ChannelOrderingChannel<Id>>({