diff --git a/config.json b/config.json index 2d10a13e..058431ef 100644 --- a/config.json +++ b/config.json @@ -152,8 +152,8 @@ }, "permissions": { "enabled": true, - "adminRoleId": null, - "moderatorRoleId": null, + "adminRoleIds": [], + "moderatorRoleIds": [], "botOwners": [], "usePermissions": true, "allowedCommands": { diff --git a/src/api/utils/configValidation.js b/src/api/utils/configValidation.js index 5c98f416..5d8f1b56 100644 --- a/src/api/utils/configValidation.js +++ b/src/api/utils/configValidation.js @@ -199,6 +199,22 @@ export const CONFIG_SCHEMA = { logChannel: { type: 'string', nullable: true }, }, }, + permissions: { + type: 'object', + properties: { + enabled: { type: 'boolean' }, + usePermissions: { type: 'boolean' }, + adminRoleIds: { type: 'array', items: { type: 'string' } }, + moderatorRoleIds: { type: 'array', items: { type: 'string' } }, + // Legacy singular fields — kept for backward compat during migration + adminRoleId: { type: 'string', nullable: true }, + moderatorRoleId: { type: 'string', nullable: true }, + modRoles: { type: 'array', items: { type: 'string' } }, + botOwners: { type: 'array', items: { type: 'string' } }, + // allowedCommands is a freeform map of command → permission level — no fixed property list + allowedCommands: { type: 'object', openProperties: true }, + }, + }, }; /** @@ -274,9 +290,10 @@ export function validateValue(value, schema, path) { for (const [key, val] of Object.entries(value)) { if (Object.hasOwn(schema.properties, key)) { errors.push(...validateValue(val, schema.properties[key], `${path}.${key}`)); - } else { + } else if (!schema.openProperties) { errors.push(`${path}.${key}: unknown config key`); } + // openProperties: true — freeform map, unknown keys are allowed } } break; diff --git a/src/modules/moderation.js b/src/modules/moderation.js index cb25f367..141dcf04 100644 --- a/src/modules/moderation.js +++ b/src/modules/moderation.js @@ -9,6 +9,7 @@ import { getPool } from '../db.js'; import { info, error as logError, warn as logWarn } from '../logger.js'; import { fetchChannelCached } from '../utils/discordCache.js'; import { parseDuration } from '../utils/duration.js'; +import { mergeRoleIds } from '../utils/permissions.js'; import { safeSend } from '../utils/safeSend.js'; import { getConfig } from './config.js'; import { fireEvent } from './webhookNotifier.js'; @@ -550,12 +551,11 @@ export function stopTempbanScheduler() { } /** - * Check if a target member is protected from moderation actions. - * Protected members include the server owner, admins, moderators, and any custom role IDs - * configured under `moderation.protectRoles`. - * @param {import('discord.js').GuildMember} target - Target member to check - * @param {import('discord.js').Guild} guild - Discord guild - * @returns {boolean} True if the target should not be moderated + * Determine whether a guild member is protected from moderation actions. + * Protection is driven by the guild's live moderation.protectRoles settings (server owner, admin/moderator roles, and explicit role IDs). + * @param {import('discord.js').GuildMember} target - Member to evaluate. + * @param {import('discord.js').Guild} guild - Guild containing the member. + * @returns {boolean} `true` if the member is protected from moderation actions, `false` otherwise. */ export function isProtectedTarget(target, guild) { // Fetch config per-invocation so live config edits take effect immediately. @@ -585,13 +585,20 @@ export function isProtectedTarget(target, guild) { return true; } + // Resolve admin/moderator role ID arrays — mergeRoleIds handles the case where + // defaults inject adminRoleIds:[] alongside a legacy adminRoleId guild override + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); + const moderatorRoleIds = mergeRoleIds( + config.permissions?.moderatorRoleIds, + config.permissions?.moderatorRoleId, + ); + const protectedRoleIds = [ - ...(protectRoles.includeAdmins && config.permissions?.adminRoleId - ? [config.permissions.adminRoleId] - : []), - ...(protectRoles.includeModerators && config.permissions?.moderatorRoleId - ? [config.permissions.moderatorRoleId] - : []), + ...(protectRoles.includeAdmins ? adminRoleIds : []), + ...(protectRoles.includeModerators ? moderatorRoleIds : []), ...(Array.isArray(protectRoles.roleIds) ? protectRoles.roleIds : []), ].filter(Boolean); diff --git a/src/utils/dbMaintenance.js b/src/utils/dbMaintenance.js index 77a5da7f..51f5d2e4 100644 --- a/src/utils/dbMaintenance.js +++ b/src/utils/dbMaintenance.js @@ -9,8 +9,8 @@ */ import { info, error as logError, warn } from '../logger.js'; -import { getConfig } from '../modules/config.js'; import { purgeOldAuditLogs } from '../modules/auditLogger.js'; +import { getConfig } from '../modules/config.js'; /** Track optional tables we've already warned about to avoid hourly log spam */ const warnedMissingOptionalTables = new Set(); diff --git a/src/utils/modExempt.js b/src/utils/modExempt.js index ddfc382c..27b562ea 100644 --- a/src/utils/modExempt.js +++ b/src/utils/modExempt.js @@ -5,6 +5,7 @@ */ import { PermissionFlagsBits } from 'discord.js'; +import { mergeRoleIds } from './permissions.js'; /** * Check whether a message author has mod/admin permissions and should be @@ -12,10 +13,13 @@ import { PermissionFlagsBits } from 'discord.js'; * * Exempt if the member: * - has the ADMINISTRATOR Discord permission, OR - * - holds the role at `config.permissions.adminRoleId` (singular ID), OR - * - holds the role at `config.permissions.moderatorRoleId` (singular ID), OR + * - holds any role in `config.permissions.adminRoleIds` (array), OR + * - holds any role in `config.permissions.moderatorRoleIds` (array), OR * - holds any role ID or name listed in `config.permissions.modRoles` (array) * + * Backward compat: also checks singular `adminRoleId` / `moderatorRoleId` fields + * so old configs continue to work without migration. + * * @param {import('discord.js').Message} message * @param {Object} config - Merged guild config * @returns {boolean} @@ -27,11 +31,20 @@ export function isExempt(message, config) { // ADMINISTRATOR permission bypasses everything if (member.permissions.has(PermissionFlagsBits.Administrator)) return true; - // Singular role IDs — the actual config schema (permissions.adminRoleId / moderatorRoleId) - const adminRoleId = config.permissions?.adminRoleId; - const moderatorRoleId = config.permissions?.moderatorRoleId; - if (adminRoleId && member.roles.cache.has(adminRoleId)) return true; - if (moderatorRoleId && member.roles.cache.has(moderatorRoleId)) return true; + // Array role IDs — new schema (permissions.adminRoleIds / moderatorRoleIds) + // Use mergeRoleIds to handle configs that have both the new empty-array default + // AND the old singular field set from a legacy guild override. + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); + if (adminRoleIds.some((id) => member.roles.cache.has(id))) return true; + + const moderatorRoleIds = mergeRoleIds( + config.permissions?.moderatorRoleIds, + config.permissions?.moderatorRoleId, + ); + if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) return true; // Legacy / test-facing array of role IDs or names (permissions.modRoles) const modRoles = config.permissions?.modRoles ?? []; diff --git a/src/utils/permissions.js b/src/utils/permissions.js index f1fe4afd..0bdea0d9 100644 --- a/src/utils/permissions.js +++ b/src/utils/permissions.js @@ -6,6 +6,35 @@ import { PermissionFlagsBits } from 'discord.js'; +/** + * Merge the new plural role IDs array with the legacy singular field. + * + * After defaults are merged, old guild configs will have BOTH `roleIds: []` + * (from defaults) AND `roleId: 'abc'` (from their stored override). Using `??` + * alone misses this case because the empty array is truthy. We always combine + * both so no configured role is ever silently dropped. + * + * @param {string[]} [roleIds=[]] - New plural field (may be empty from defaults) + * @param {string|null} [roleId=null] - Legacy singular field + * @returns {string[]} Deduplicated merged list + */ +export function mergeRoleIds(roleIds, roleId) { + // Normalize roleIds defensively — persisted config may contain a string instead of an array + let base; + if (Array.isArray(roleIds)) { + base = roleIds; + } else if (typeof roleIds === 'string' && roleIds.length > 0) { + base = [roleIds]; + } else { + base = []; + } + const merged = new Set(base); + if (typeof roleId === 'string' && roleId.length > 0) { + merged.add(roleId); + } + return [...merged]; +} + /** * Retrieve the configured bot owner user IDs. * @@ -43,11 +72,11 @@ export function isBotOwner(member, config) { } /** - * Check if a member is an admin + * Determine whether a guild member has administrative privileges. * - * @param {GuildMember} member - Discord guild member - * @param {Object} config - Bot configuration - * @returns {boolean} True if member is admin + * @param {GuildMember} member - The guild member to check. + * @param {Object} config - Bot configuration containing permission role IDs. + * @returns {boolean} `true` if the member is an admin, `false` otherwise. */ export function isAdmin(member, config) { if (!member) return false; @@ -62,9 +91,14 @@ export function isAdmin(member, config) { return true; } - // Check if member has the configured admin role - if (config.permissions?.adminRoleId) { - return member.roles.cache.has(config.permissions.adminRoleId); + // Check if member has any of the configured admin roles + // mergeRoleIds handles the case where defaults inject adminRoleIds:[] alongside a legacy adminRoleId value + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); + if (adminRoleIds.length > 0) { + return adminRoleIds.some((id) => member.roles.cache.has(id)); } return false; @@ -134,11 +168,12 @@ export function isGuildAdmin(member, config) { } /** - * Check if a member is a moderator (has MANAGE_GUILD permission or bot admin role) + * Determine whether a guild member is considered a moderator. * - * @param {GuildMember} member - Discord guild member - * @param {Object} config - Bot configuration - * @returns {boolean} True if member is a moderator + * Considers bot owners, members with the Administrator or Manage Guild permission, and members with any configured admin or moderator role IDs (supports legacy singular role ID fields). + * @param {GuildMember} member - Discord guild member to check. + * @param {Object} config - Bot configuration containing permission role settings (e.g., permissions.adminRoleIds, permissions.moderatorRoleIds or legacy adminRoleId/moderatorRoleId). + * @returns {boolean} `true` if the member is a moderator, `false` otherwise. */ export function isModerator(member, config) { if (!member) return false; @@ -158,15 +193,22 @@ export function isModerator(member, config) { return true; } - // Check bot admin role from config - if (config.permissions?.adminRoleId) { - if (member.roles.cache.has(config.permissions.adminRoleId)) { - return true; - } + // Check bot admin roles from config + const adminRoleIds = mergeRoleIds( + config.permissions?.adminRoleIds, + config.permissions?.adminRoleId, + ); + if (adminRoleIds.some((id) => member.roles.cache.has(id))) { + return true; } - if (config.permissions?.moderatorRoleId) { - return member.roles.cache.has(config.permissions.moderatorRoleId); + // Check bot moderator roles from config + const moderatorRoleIds = mergeRoleIds( + config.permissions?.moderatorRoleIds, + config.permissions?.moderatorRoleId, + ); + if (moderatorRoleIds.some((id) => member.roles.cache.has(id))) { + return true; } return false; diff --git a/tests/utils/modExempt.test.js b/tests/utils/modExempt.test.js index 4cbecff9..0494468c 100644 --- a/tests/utils/modExempt.test.js +++ b/tests/utils/modExempt.test.js @@ -48,30 +48,66 @@ describe('isExempt', () => { expect(isExempt(msg, {})).toBe(false); }); - it('should return true when member has adminRoleId', () => { + it('should return true when member has a role in adminRoleIds array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-id'] }); - const config = { permissions: { adminRoleId: 'admin-role-id' } }; + const config = { permissions: { adminRoleIds: ['admin-role-id'] } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should return true when member has any of multiple adminRoleIds', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-2'] }); + const config = { permissions: { adminRoleIds: ['admin-role-1', 'admin-role-2'] } }; expect(isExempt(msg, config)).toBe(true); }); - it('should return false when adminRoleId is set but member does not have it', () => { + it('should return false when adminRoleIds is set but member does not have any', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['other-role'] }); - const config = { permissions: { adminRoleId: 'admin-role-id' } }; + const config = { permissions: { adminRoleIds: ['admin-role-id'] } }; expect(isExempt(msg, config)).toBe(false); }); - it('should return true when member has moderatorRoleId', () => { + it('should return true when member has a role in moderatorRoleIds array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-id'] }); - const config = { permissions: { moderatorRoleId: 'mod-role-id' } }; + const config = { permissions: { moderatorRoleIds: ['mod-role-id'] } }; expect(isExempt(msg, config)).toBe(true); }); - it('should return false when moderatorRoleId is set but member does not have it', () => { + it('should return true when member has any of multiple moderatorRoleIds', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-2'] }); + const config = { permissions: { moderatorRoleIds: ['mod-role-1', 'mod-role-2'] } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should return false when moderatorRoleIds is set but member does not have any', () => { const msg = makeMessage({ isAdmin: false, roleIds: [] }); - const config = { permissions: { moderatorRoleId: 'mod-role-id' } }; + const config = { permissions: { moderatorRoleIds: ['mod-role-id'] } }; expect(isExempt(msg, config)).toBe(false); }); + it('should support backward compat: singular adminRoleId still grants exemption', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['admin-role-id'] }); + const config = { permissions: { adminRoleId: 'admin-role-id' } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should support backward compat: singular moderatorRoleId still grants exemption', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['mod-role-id'] }); + const config = { permissions: { moderatorRoleId: 'mod-role-id' } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should grant exemption via legacy adminRoleId even when adminRoleIds:[] default is present (merged config)', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['legacy-admin'] }); + const config = { permissions: { adminRoleIds: [], adminRoleId: 'legacy-admin' } }; + expect(isExempt(msg, config)).toBe(true); + }); + + it('should grant exemption via legacy moderatorRoleId even when moderatorRoleIds:[] default is present (merged config)', () => { + const msg = makeMessage({ isAdmin: false, roleIds: ['legacy-mod'] }); + const config = { permissions: { moderatorRoleIds: [], moderatorRoleId: 'legacy-mod' } }; + expect(isExempt(msg, config)).toBe(true); + }); + it('should return true when member has a role ID in modRoles array', () => { const msg = makeMessage({ isAdmin: false, roleIds: ['custom-mod'] }); const config = { permissions: { modRoles: ['custom-mod'] } }; diff --git a/tests/utils/permissions.test.js b/tests/utils/permissions.test.js index 683d0b7a..4a1898a6 100644 --- a/tests/utils/permissions.test.js +++ b/tests/utils/permissions.test.js @@ -16,6 +16,7 @@ import { isBotOwner, isGuildAdmin, isModerator, + mergeRoleIds, } from '../../src/utils/permissions.js'; const BOT_OWNER_ID = '191633014441115648'; @@ -86,32 +87,66 @@ describe('isAdmin', () => { expect(isAdmin(member, {})).toBe(true); }); - it('should return true for members with admin role', () => { + it('should return true for members with admin role (adminRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isAdmin(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); + it('should return true for members with any of multiple admin roles', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { + cache: { + has: vi.fn().mockImplementation((id) => id === '999999'), + }, + }, + }; + const config = { permissions: { adminRoleIds: ['123456', '999999'] } }; + expect(isAdmin(member, config)).toBe(true); + }); + it('should return false for regular members', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(false) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isAdmin(member, config)).toBe(false); }); - it('should return false when no adminRoleId configured and not Admin', () => { + it('should return false when no adminRoleIds configured and not Admin', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn() } }, }; expect(isAdmin(member, {})).toBe(false); }); + + it('should support backward compat: singular adminRoleId still works', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: vi.fn().mockReturnValue(true) } }, + }; + const config = { permissions: { adminRoleId: '123456' } }; + expect(isAdmin(member, config)).toBe(true); + expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); + }); + + it('should find legacy adminRoleId even when adminRoleIds:[] default is present (merged config)', () => { + // This is the real breaking case: defaults merge in adminRoleIds:[] before guild overrides + // apply, so the config has BOTH fields. ?? alone would miss the legacy value. + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: (id) => id === 'legacy-role-789' } }, + }; + const config = { permissions: { adminRoleIds: [], adminRoleId: 'legacy-role-789' } }; + expect(isAdmin(member, config)).toBe(true); + }); }); describe('hasPermission', () => { @@ -320,12 +355,12 @@ describe('isGuildAdmin', () => { expect(isGuildAdmin(member, {})).toBe(true); }); - it('should return true for members with admin role', () => { + it('should return true for members with admin role (adminRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isGuildAdmin(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); @@ -382,26 +417,52 @@ describe('isModerator', () => { expect(isModerator(member, {})).toBe(true); }); - it('should return true for members with admin role', () => { + it('should return true for members with admin role (adminRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { adminRoleId: '123456' } }; + const config = { permissions: { adminRoleIds: ['123456'] } }; expect(isModerator(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); }); - it('should return true for members with moderator role', () => { + it('should return true for members with any of multiple admin roles', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { + cache: { + has: vi.fn().mockImplementation((id) => id === '999999'), + }, + }, + }; + const config = { permissions: { adminRoleIds: ['123456', '999999'] } }; + expect(isModerator(member, config)).toBe(true); + }); + + it('should return true for members with moderator role (moderatorRoleIds array)', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, roles: { cache: { has: vi.fn().mockReturnValue(true) } }, }; - const config = { permissions: { moderatorRoleId: '654321' } }; + const config = { permissions: { moderatorRoleIds: ['654321'] } }; expect(isModerator(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); }); + it('should return true for members with any of multiple moderator roles', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { + cache: { + has: vi.fn().mockImplementation((id) => id === '888888'), + }, + }, + }; + const config = { permissions: { moderatorRoleIds: ['654321', '888888'] } }; + expect(isModerator(member, config)).toBe(true); + }); + it('should return true for moderator role when admin and moderator roles are both configured', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, @@ -412,13 +473,54 @@ describe('isModerator', () => { }, }; const config = { - permissions: { adminRoleId: '123456', moderatorRoleId: '654321' }, + permissions: { adminRoleIds: ['123456'], moderatorRoleIds: ['654321'] }, }; expect(isModerator(member, config)).toBe(true); expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); }); + it('should support backward compat: singular adminRoleId still works', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: vi.fn().mockReturnValue(true) } }, + }; + const config = { permissions: { adminRoleId: '123456' } }; + expect(isModerator(member, config)).toBe(true); + expect(member.roles.cache.has).toHaveBeenCalledWith('123456'); + }); + + it('should support backward compat: singular moderatorRoleId still works', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: vi.fn().mockReturnValue(true) } }, + }; + const config = { permissions: { moderatorRoleId: '654321' } }; + expect(isModerator(member, config)).toBe(true); + expect(member.roles.cache.has).toHaveBeenCalledWith('654321'); + }); + + it('should find legacy moderatorRoleId even when moderatorRoleIds:[] default is present (merged config)', () => { + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: (id) => id === 'legacy-mod-999' } }, + }; + const config = { permissions: { moderatorRoleIds: [], moderatorRoleId: 'legacy-mod-999' } }; + expect(isModerator(member, config)).toBe(true); + }); + + it('should grant moderator via legacy adminRoleId even when adminRoleIds:[] default is present', () => { + // isModerator() checks admin roles first — legacy adminRoleId must be found + const member = { + permissions: { has: vi.fn().mockReturnValue(false) }, + roles: { cache: { has: (id) => id === 'legacy-admin-123' } }, + }; + const config = { + permissions: { adminRoleIds: [], adminRoleId: 'legacy-admin-123', moderatorRoleIds: [] }, + }; + expect(isModerator(member, config)).toBe(true); + }); + it('should return false for regular members', () => { const member = { permissions: { has: vi.fn().mockReturnValue(false) }, @@ -470,3 +572,59 @@ describe('isBotOwner', () => { expect(isBotOwner(member, config)).toBe(false); }); }); + +describe('mergeRoleIds', () => { + it('merges a non-empty array with a singular id', () => { + expect(mergeRoleIds(['a', 'b'], 'c')).toEqual(['a', 'b', 'c']); + }); + + it('deduplicates when singular id is already in array', () => { + expect(mergeRoleIds(['a', 'b'], 'a')).toEqual(['a', 'b']); + }); + + it('handles empty array + singular id', () => { + expect(mergeRoleIds([], 'abc')).toEqual(['abc']); + }); + + it('handles array only (no singular id)', () => { + expect(mergeRoleIds(['x', 'y'], null)).toEqual(['x', 'y']); + }); + + it('handles null array + singular id (legacy-only config)', () => { + expect(mergeRoleIds(null, 'legacy-id')).toEqual(['legacy-id']); + }); + + it('handles undefined array + singular id (defaults not merged yet)', () => { + expect(mergeRoleIds(undefined, 'legacy-id')).toEqual(['legacy-id']); + }); + + it('handles both null — returns empty array', () => { + expect(mergeRoleIds(null, null)).toEqual([]); + }); + + it('normalizes a string roleIds to single-element array (malformed config)', () => { + expect(mergeRoleIds('malformed-string-id', null)).toEqual(['malformed-string-id']); + }); + + it('string roleIds + singular id deduplicates if same', () => { + expect(mergeRoleIds('role-123', 'role-123')).toEqual(['role-123']); + }); + + it('string roleIds + different singular id merges both', () => { + expect(mergeRoleIds('role-abc', 'role-xyz')).toEqual(['role-abc', 'role-xyz']); + }); + + it('empty string roleId is ignored', () => { + expect(mergeRoleIds(['a'], '')).toEqual(['a']); + }); + + it('empty string roleIds falls back to empty array', () => { + expect(mergeRoleIds('', 'abc')).toEqual(['abc']); + }); + + it('real merged-config case: defaults inject [] alongside legacy guild override', () => { + // This is the production failure scenario: defaults merge adminRoleIds:[] before + // guild overrides apply, so config has BOTH fields. + expect(mergeRoleIds([], 'legacy-guild-role')).toEqual(['legacy-guild-role']); + }); +}); diff --git a/web/src/components/dashboard/config-editor.tsx b/web/src/components/dashboard/config-editor.tsx index 51efec57..55c97a35 100644 --- a/web/src/components/dashboard/config-editor.tsx +++ b/web/src/components/dashboard/config-editor.tsx @@ -130,11 +130,12 @@ function isGuildConfig(data: unknown): data is GuildConfig { } /** - * Edit a guild's bot configuration through a multi-section UI. + * Edit a guild's bot configuration via a categorized editor with per-section controls. * - * Loads the authoritative config for the selected guild, maintains a mutable draft for user edits, - * computes and applies per-section patches to persist changes, and provides controls to save, - * discard, and validate edits (including an unsaved-changes warning and keyboard shortcut). + * Loads the authoritative config for the selected guild, keeps an editable draft, validates edits, + * and persists changes as batched, top-level section PATCHes. Provides discard and undo flows, + * an unsaved-changes warning, keyboard shortcuts (Ctrl/Cmd+S to open the diff preview, '/' to focus + * search), and a diff modal to review or revert per-section changes before saving. * * @returns The editor UI as JSX when a guild is selected and a draft config exists; `null` otherwise. */ @@ -871,7 +872,12 @@ export function ConfigEditor() { (field: string, value: unknown) => { updateDraftConfig((prev) => { if (!prev) return prev; - return { ...prev, permissions: { ...prev.permissions, [field]: value } } as GuildConfig; + const updated = { ...prev.permissions, [field]: value }; + // Auto-clear legacy singular fields whenever the plural arrays are written, + // so computePatches never includes both the old and new forms simultaneously. + if (field === 'adminRoleIds') updated.adminRoleId = null; + if (field === 'moderatorRoleIds') updated.moderatorRoleId = null; + return { ...prev, permissions: updated } as GuildConfig; }); }, [updateDraftConfig], @@ -1994,40 +2000,46 @@ export function ConfigEditor() { disabled={saving} basicContent={
-
diff --git a/web/src/components/dashboard/performance-dashboard.tsx b/web/src/components/dashboard/performance-dashboard.tsx index 15edfe1c..d405c64c 100644 --- a/web/src/components/dashboard/performance-dashboard.tsx +++ b/web/src/components/dashboard/performance-dashboard.tsx @@ -119,6 +119,15 @@ function StatCard({ title, value, subtitle, icon: Icon, alert, loading }: StatCa const AUTO_REFRESH_MS = 30_000; +/** + * Render the performance dashboard UI showing memory, CPU, and response-time metrics, + * including time-series charts, a response-time histogram, recent samples, and editable alert thresholds. + * + * The dashboard auto-refreshes in the background at a fixed interval, allows manual refresh, + * seeds the threshold editor from fetched data, and provides saving of threshold changes. + * + * @returns The dashboard UI as a JSX element. + */ export function PerformanceDashboard() { const [data, setData] = useState(null); const [loading, setLoading] = useState(false); @@ -360,9 +369,7 @@ export function PerformanceDashboard() { - (v != null ? [`${v} MB`] : [''])} - /> + (v != null ? [`${v} MB`] : [''])} /> - (v != null ? [`${v}%`] : [''])} - /> + (v != null ? [`${v}%`] : [''])} />