From f6253296c71fe2257e67b33e2c9cac445edadc74 Mon Sep 17 00:00:00 2001 From: Matt Cavanagh <1776058+Maelstromeous@users.noreply.github.com> Date: Mon, 3 Aug 2026 01:47:24 +0100 Subject: [PATCH 1/3] feat(albion): Strip content roles and ping reactions when a member leaves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content roles were only ever added. Someone who left the guild kept both the role and their reaction on the content pings message, so the counts on those roles drifted further from reality with every leaver. The pings embed is the source of truth for which roles are content roles: rank roles share the ALB/ prefix, so the name alone can't tell the two apart, and editing the embed is already how a content role gets added. Rank roles are excluded by ID as well, so a stray mention in the text can't strip a rank. Stripping happens in two places. Deregistration covers leavers as they are found, and a sweep in the nightly scan clears historic drift plus anyone who left the Discord server outright โ€” Discord never prunes their reactions. /albion-content-roles runs the sweep on demand. Alliance members hold content roles without ever registering, so anyone with a role from ALBION_CONTENT_ROLE_EXEMPT_ROLES is left alone. The sweep aborts rather than acting if the message can't be read or no roles resolve; a failed fetch reading as an empty role list would strip the whole server. Co-Authored-By: Claude Opus 5 --- digletbot.env.example | 5 + src/albion/albion.module.ts | 4 + .../commands/content-roles.command.spec.ts | 75 ++ src/albion/commands/content-roles.command.ts | 42 ++ .../albion.content.role.service.spec.ts | 677 ++++++++++++++++++ .../services/albion.content.role.service.ts | 451 ++++++++++++ .../albion.deregistration.service.spec.ts | 43 ++ .../services/albion.deregistration.service.ts | 10 + .../services/albion.scanning.service.spec.ts | 31 +- .../services/albion.scanning.service.ts | 14 +- src/config/albion.app.config.ts | 11 + src/test.bootstrapper.ts | 88 +++ 12 files changed, 1443 insertions(+), 8 deletions(-) create mode 100644 src/albion/commands/content-roles.command.spec.ts create mode 100644 src/albion/commands/content-roles.command.ts create mode 100644 src/albion/services/albion.content.role.service.spec.ts create mode 100644 src/albion/services/albion.content.role.service.ts diff --git a/digletbot.env.example b/digletbot.env.example index 9e75bfed0..42548be47 100644 --- a/digletbot.env.example +++ b/digletbot.env.example @@ -25,6 +25,11 @@ ROLE_ALBION_US_REGISTERED=12345 ROLE_ALBION_EU_REGISTERED=12345 ROLE_ALBION_US_ANNOUNCEMENTS=12345 ROLE_ALBION_EU_ANNOUNCEMENTS=12345 +# The content pings embed, in CHANNEL_ALBION_EU_ROLES. Its ALB/ role names drive the content +# role sweep; leave it unset to disable the sweep entirely. +MESSAGE_ALBION_CONTENT_PINGS=12345 +# Comma separated role IDs that keep content roles without an Albion registration (alliance). +ALBION_CONTENT_ROLE_EXEMPT_ROLES=12345,67890 # PS2 Channels & Roles CHANNEL_PS2_VERIFY=1234567 diff --git a/src/albion/albion.module.ts b/src/albion/albion.module.ts index cd87811ba..f8c067b7c 100644 --- a/src/albion/albion.module.ts +++ b/src/albion/albion.module.ts @@ -7,6 +7,8 @@ import { AlbionRegistrationService } from './services/albion.registration.servic import { DiscordModule } from '../discord/discord.module'; import { AlbionScanningService } from './services/albion.scanning.service'; import { AlbionScanCommand } from './commands/scan.command'; +import { AlbionContentRolesCommand } from './commands/content-roles.command'; +import { AlbionContentRoleService } from './services/albion.content.role.service'; import { AlbionCronService } from './services/albion.cron.service'; import { AlbionUtilities } from './utilities/albion.utilities'; import { AlbionLogCommand } from './commands/log.command'; @@ -29,6 +31,8 @@ import { GeneralModule } from '../general/general.module'; imports: [ConfigModule, DatabaseModule, DiscordModule, GeneralModule], providers: [ AlbionApiService, + AlbionContentRoleService, + AlbionContentRolesCommand, AlbionCronService, AlbionDeregisterCommand, AlbionDeregistrationService, diff --git a/src/albion/commands/content-roles.command.spec.ts b/src/albion/commands/content-roles.command.spec.ts new file mode 100644 index 000000000..f8d4f2482 --- /dev/null +++ b/src/albion/commands/content-roles.command.spec.ts @@ -0,0 +1,75 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { Test, TestingModule } from '@nestjs/testing'; +import { ConfigService } from '@nestjs/config'; +import { AlbionContentRolesCommand } from './content-roles.command'; +import { AlbionContentRoleService } from '../services/albion.content.role.service'; +import { TestBootstrapper } from '../../test.bootstrapper'; + +const scanChannelId = TestBootstrapper.mockConfig.discord.channels.albionScans; + +describe('AlbionContentRolesCommand', () => { + let command: AlbionContentRolesCommand; + let albionContentRoleService: AlbionContentRoleService; + let mockDiscordInteraction: any; + + beforeEach(async () => { + const moduleRef: TestingModule = await Test.createTestingModule({ + providers: [ + AlbionContentRolesCommand, + { + provide: ConfigService, + useValue: { get: jest.fn() }, + }, + { + provide: AlbionContentRoleService, + useValue: { reconcile: jest.fn() }, + }, + ], + }).compile(); + TestBootstrapper.setupConfig(moduleRef); + + command = moduleRef.get(AlbionContentRolesCommand); + albionContentRoleService = moduleRef.get(AlbionContentRoleService); + + mockDiscordInteraction = TestBootstrapper.getMockDiscordInteraction( + scanChannelId, + TestBootstrapper.getMockDiscordUser(), + ); + }); + + it('should be defined', () => { + expect(command).toBeDefined(); + }); + + it('should start the sweep when the channel is correct', async () => { + await command.onAlbionContentRolesCommand({ dryRun: false }, mockDiscordInteraction); + + expect(mockDiscordInteraction[0].deferReply).toHaveBeenCalled(); + expect(mockDiscordInteraction[0].channel.send).toHaveBeenCalledWith('Starting Albion content role sweep...'); + expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(expect.anything(), false); + }); + + it('should refuse to run outside the scans channel', async () => { + mockDiscordInteraction[0].channelId = 'wrongChannelId'; + + const response = await command.onAlbionContentRolesCommand({ dryRun: false }, mockDiscordInteraction); + + expect(response).toBe(`Please use the <#${scanChannelId}> channel to perform Scans.`); + expect(mockDiscordInteraction[0].channel.send).not.toHaveBeenCalled(); + expect(albionContentRoleService.reconcile).not.toHaveBeenCalled(); + }); + + it('should flag a dry run in the response', async () => { + const response = await command.onAlbionContentRolesCommand({ dryRun: true }, mockDiscordInteraction); + + expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(expect.anything(), true); + expect(response).toBe('Albion content role sweep initiated! [DRY RUN, NO CHANGES WILL ACTUALLY BE PERFORMED]'); + }); + + it('should default to a live run when the option is omitted', async () => { + const response = await command.onAlbionContentRolesCommand({} as any, mockDiscordInteraction); + + expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(expect.anything(), false); + expect(response).toBe('Albion content role sweep initiated!'); + }); +}); diff --git a/src/albion/commands/content-roles.command.ts b/src/albion/commands/content-roles.command.ts new file mode 100644 index 000000000..aab307c3d --- /dev/null +++ b/src/albion/commands/content-roles.command.ts @@ -0,0 +1,42 @@ +import { Context, Options, SlashCommand, SlashCommandContext } from 'necord'; +import { Injectable, Logger } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { AlbionScanDto } from '../dto/albion.scan.dto'; +import { AlbionContentRoleService } from '../services/albion.content.role.service'; +import { replyTo } from '../../discord/discord.hacks'; + +@Injectable() +export class AlbionContentRolesCommand { + private readonly logger = new Logger(AlbionContentRolesCommand.name); + + constructor( + private readonly config: ConfigService, + private readonly albionContentRoleService: AlbionContentRoleService, + ) {} + + @SlashCommand({ + name: 'albion-content-roles', + description: 'Strip Albion content roles and ping reactions from members who are no longer registered', + }) + async onAlbionContentRolesCommand( + @Options() dto: AlbionScanDto, + @Context() [interaction]: SlashCommandContext, + ): Promise { + this.logger.debug('Received Albion Content Roles Command'); + // Fetching the member list and paging every reaction blows Discord's 3s reply window. + await interaction.deferReply(); + const dryRun = dto.dryRun ?? false; + + const scanChannelId = this.config.get('discord.channels.albionScans'); + + if (interaction.channelId !== scanChannelId) { + return replyTo(interaction, `Please use the <#${scanChannelId}> channel to perform Scans.`); + } + + const message = await interaction.channel.send('Starting Albion content role sweep...'); + + this.albionContentRoleService.reconcile(message, dryRun); + + return replyTo(interaction, `Albion content role sweep initiated!${dryRun ? ' [DRY RUN, NO CHANGES WILL ACTUALLY BE PERFORMED]' : ''}`); + } +} diff --git a/src/albion/services/albion.content.role.service.spec.ts b/src/albion/services/albion.content.role.service.spec.ts new file mode 100644 index 000000000..9389e4397 --- /dev/null +++ b/src/albion/services/albion.content.role.service.spec.ts @@ -0,0 +1,677 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { Test, TestingModule } from '@nestjs/testing'; +import { ConfigService } from '@nestjs/config'; +import { getRepositoryToken } from '@mikro-orm/nestjs'; +import { Collection } from 'discord.js'; +import { AlbionContentRoleService, MAX_REACTION_REMOVALS_PER_RUN } from './albion.content.role.service'; +import { AlbionRegistrationsEntity } from '../../database/entities/albion.registrations.entity'; +import { DiscordService } from '../../discord/discord.service'; +import { TestBootstrapper } from '../../test.bootstrapper'; + +const contentRoleIds = TestBootstrapper.mockContentRoleIds; +const exemptRoleId = TestBootstrapper.mockConfig.albion.contentRoleExemptRoles[0]; +const rankRoleId = TestBootstrapper.mockConfig.albion.roleMap[0].discordRoleId; +const albionGuildId = TestBootstrapper.mockConfig.albion.guildId; +const devUserId = TestBootstrapper.mockConfig.discord.devUserId; + +// Every role on the server, not just the content ones: the service has to pick the content +// roles out of a list that also holds rank roles sharing the same ALB/ prefix. +const allGuildRoles = () => TestBootstrapper.getMockContentRoleCollection() + .set(rankRoleId, { id: rankRoleId, name: 'ALB/Archmage' } as any) + .set(exemptRoleId, { id: exemptRoleId, name: 'ALB/Alliance' } as any); + +describe('AlbionContentRoleService', () => { + let service: AlbionContentRoleService; + let discordService: jest.Mocked; + let mockRepository: any; + let mockChannel: any; + let mockPingsMessage: any; + + const setupModule = async (configOverride?: any) => { + const moduleRef: TestingModule = await Test.createTestingModule({ + providers: [ + AlbionContentRoleService, + { + provide: ConfigService, + useValue: { get: jest.fn() }, + }, + { + provide: DiscordService, + useValue: { + getTextChannel: jest.fn(), + getAllRolesFromGuild: jest.fn(), + }, + }, + { + provide: getRepositoryToken(AlbionRegistrationsEntity), + useValue: mockRepository, + }, + ], + }).compile(); + + TestBootstrapper.setupConfig(moduleRef, configOverride); + + service = moduleRef.get(AlbionContentRoleService); + discordService = moduleRef.get(DiscordService) as any; + + discordService.getTextChannel.mockResolvedValue(mockChannel); + discordService.getAllRolesFromGuild.mockResolvedValue(allGuildRoles()); + }; + + beforeEach(async () => { + jest.clearAllMocks(); + + mockRepository = { + find: jest.fn().mockResolvedValue([]), + findOne: jest.fn(), + }; + + mockPingsMessage = TestBootstrapper.getMockContentPingsMessage(); + mockChannel = { + messages: { fetch: jest.fn().mockResolvedValue(mockPingsMessage) }, + }; + + await setupModule(); + }); + + it('should be defined', () => { + expect(service).toBeDefined(); + }); + + describe('getPingsMessage', () => { + it('should fetch the configured message from the configured channel', async () => { + const message = await service.getPingsMessage(); + + expect(discordService.getTextChannel).toHaveBeenCalledWith(TestBootstrapper.mockConfig.discord.channels.albionRoles); + expect(mockChannel.messages.fetch).toHaveBeenCalledWith(TestBootstrapper.mockConfig.albion.contentPingsMessageId); + expect(message).toBe(mockPingsMessage); + }); + + it('should return null when no message ID is configured', async () => { + await setupModule({ + albion: { ...TestBootstrapper.mockConfig.albion, contentPingsMessageId: undefined }, + discord: TestBootstrapper.mockConfig.discord, + }); + + expect(await service.getPingsMessage()).toBeNull(); + expect(discordService.getTextChannel).not.toHaveBeenCalled(); + }); + + it('should return null when the channel cannot be fetched', async () => { + discordService.getTextChannel.mockRejectedValueOnce(new Error('No channel')); + + expect(await service.getPingsMessage()).toBeNull(); + }); + + it('should return null when the message cannot be fetched', async () => { + mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + + expect(await service.getPingsMessage()).toBeNull(); + }); + }); + + describe('extractText', () => { + it('should read content, embed title, description and fields', () => { + const text = service.extractText({ + content: 'top level', + embeds: [ + { + title: 'A title', + description: 'A description', + fields: [{ name: 'field name', value: 'field value' }], + }, + ], + } as any); + + expect(text).toContain('top level'); + expect(text).toContain('A title'); + expect(text).toContain('A description'); + expect(text).toContain('field name'); + expect(text).toContain('field value'); + }); + + it('should cope with a message that has no embeds or content', () => { + expect(service.extractText({} as any)).toBe(''); + }); + + it('should cope with an embed with no description or fields', () => { + expect(service.extractText({ content: '', embeds: [{ title: 'Only a title' }] } as any)) + .toContain('Only a title'); + }); + + it('should cope with an embed with no title', () => { + expect(service.extractText({ embeds: [{ description: 'Only a description' }] } as any)) + .toContain('Only a description'); + }); + + it('should cope with a field with no name or value', () => { + expect(service.extractText({ embeds: [{ fields: [{}] }] } as any).trim()).toBe(''); + }); + }); + + describe('getContentRoles', () => { + it('should resolve every content role named in the embed', async () => { + const result = await service.getContentRoles({} as any); + + expect(result.roles.size).toBe(3); + expect(result.roles.map((role) => role.name).sort()).toEqual([ + 'ALB/Arena', + 'ALB/FactionWarfare', + 'ALB/Mist', + ]); + expect(result.unresolved).toEqual([]); + }); + + it('should never treat a rank role as a content role, even when the embed names one', async () => { + mockPingsMessage.embeds[0].description += '\n๐Ÿ‘‘ ALB/Archmage - the guild leader'; + + const result = await service.getContentRoles({} as any); + + expect(result.roles.has(rankRoleId)).toBe(false); + expect(result.roles.size).toBe(3); + expect(result.unresolved).toEqual([]); + }); + + it('should report role names in the embed that do not exist on the server', async () => { + mockPingsMessage.embeds[0].description += '\n๐Ÿพ ALB/Tracking - if you want to go hunting'; + + const result = await service.getContentRoles({} as any); + + expect(result.unresolved).toEqual(['ALB/Tracking']); + expect(result.roles.size).toBe(3); + }); + + it('should de-duplicate a role named twice in the embed', async () => { + mockPingsMessage.embeds[0].description += '\nโ˜๏ธ ALB/Mist - mentioned again by mistake'; + + const result = await service.getContentRoles({} as any); + + expect(result.roles.size).toBe(3); + }); + + it('should return null when the pings message cannot be read', async () => { + mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + + expect(await service.getContentRoles({} as any)).toBeNull(); + }); + + it('should return null when the embed names no ALB/ roles at all', async () => { + mockPingsMessage.embeds[0].description = 'Someone has emptied the message'; + + expect(await service.getContentRoles({} as any)).toBeNull(); + }); + + it('should return null when none of the named roles resolve', async () => { + discordService.getAllRolesFromGuild.mockResolvedValue(new Collection() as any); + + expect(await service.getContentRoles({} as any)).toBeNull(); + }); + + it('should return null when the embed only names rank roles', async () => { + mockPingsMessage.embeds[0].description = '๐Ÿ‘‘ ALB/Archmage - the guild leader'; + + expect(await service.getContentRoles({} as any)).toBeNull(); + }); + }); + + describe('fetchReactors', () => { + it('should map each user to the reactions they placed', async () => { + const message = TestBootstrapper.getMockContentPingsMessage([ + TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1', '2']), + TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['2']), + ]); + + const reactors = await service.fetchReactors(message); + + expect(reactors.size).toBe(2); + expect(reactors.get('1')).toHaveLength(1); + expect(reactors.get('2')).toHaveLength(2); + }); + + it('should page past the first 100 users', async () => { + const userIds = Array.from({ length: 250 }, (_, index) => `user-${index}`); + const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', userIds); + const message = TestBootstrapper.getMockContentPingsMessage([reaction]); + + const reactors = await service.fetchReactors(message); + + expect(reactors.size).toBe(250); + expect(reaction.users.fetch).toHaveBeenCalledTimes(3); + expect(reaction.users.fetch).toHaveBeenNthCalledWith(2, { limit: 100, after: 'user-99' }); + }); + + it('should stop paging on an exact multiple of the page size', async () => { + const userIds = Array.from({ length: 100 }, (_, index) => `user-${index}`); + const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', userIds); + + const reactors = await service.fetchReactors(TestBootstrapper.getMockContentPingsMessage([reaction])); + + expect(reactors.size).toBe(100); + // One full page, then one empty page proving there is no more + expect(reaction.users.fetch).toHaveBeenCalledTimes(2); + }); + + it('should ignore bots', async () => { + const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1'], ['bot-1']); + + const reactors = await service.fetchReactors(TestBootstrapper.getMockContentPingsMessage([reaction])); + + expect(reactors.size).toBe(1); + expect(reactors.has('bot-1')).toBe(false); + }); + + it('should return nothing for a message with no reactions', async () => { + const reactors = await service.fetchReactors(TestBootstrapper.getMockContentPingsMessage()); + + expect(reactors.size).toBe(0); + }); + }); + + describe('isExempt', () => { + it('should be false for a member who has left the server', () => { + expect(service.isExempt(null)).toBe(false); + }); + + it('should be true for a member holding an exempt role', () => { + expect(service.isExempt(TestBootstrapper.getMockGuildMemberWithRoles('1', [exemptRoleId]))).toBe(true); + }); + + it('should be false for a member holding only content roles', () => { + expect(service.isExempt(TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]))).toBe(false); + }); + + it('should be false when no exempt roles are configured', async () => { + await setupModule({ + albion: { ...TestBootstrapper.mockConfig.albion, contentRoleExemptRoles: undefined }, + discord: TestBootstrapper.mockConfig.discord, + }); + + expect(service.isExempt(TestBootstrapper.getMockGuildMemberWithRoles('1', [exemptRoleId]))).toBe(false); + }); + }); + + describe('removeContentRoles', () => { + it('should remove only the content roles the member actually holds', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + + const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), false); + + expect(result.removed).toEqual(['ALB/Mist']); + expect(member.roles.remove).toHaveBeenCalledTimes(1); + expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + }); + + it('should report but not perform removals on a dry run', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist, contentRoleIds.arena]); + + const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), true); + + expect(result.removed).toHaveLength(2); + expect(member.roles.remove).not.toHaveBeenCalled(); + }); + + it('should capture an error without abandoning the remaining roles', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.factionWarfare, contentRoleIds.mist]); + member.roles.remove.mockRejectedValueOnce(new Error('Discord says no')); + + const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), false); + + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toContain('Discord says no'); + expect(member.roles.remove).toHaveBeenCalledTimes(2); + }); + + it('should do nothing for a member with no content roles', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', []); + + const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), false); + + expect(result.removed).toEqual([]); + expect(member.roles.remove).not.toHaveBeenCalled(); + }); + }); + + describe('removeReactions', () => { + it('should remove the user from each reaction', async () => { + const first = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1']); + const second = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['1']); + + const result = await service.removeReactions([first, second], '1', false); + + expect(result.removed).toBe(2); + expect(first.users.remove).toHaveBeenCalledWith('1'); + expect(second.users.remove).toHaveBeenCalledWith('1'); + }); + + it('should report but not perform removals on a dry run', async () => { + const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1']); + + const result = await service.removeReactions([reaction], '1', true); + + expect(result.removed).toBe(1); + expect(reaction.users.remove).not.toHaveBeenCalled(); + }); + + it('should not count a failed removal as removed', async () => { + const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1']); + reaction.users.remove.mockRejectedValueOnce(new Error('Missing Permissions')); + + const result = await service.removeReactions([reaction], '1', false); + + expect(result.removed).toBe(0); + expect(result.errors[0]).toContain('Missing Permissions'); + }); + }); + + describe('stripForDeregistration', () => { + let responseChannel: any; + + beforeEach(() => { + responseChannel = { send: jest.fn() }; + mockPingsMessage.reactions.cache = new Collection([ + ['0', TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1'])], + ]); + }); + + it('should strip roles and reactions and report what it did', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + + const result = await service.stripForDeregistration('1', member, responseChannel); + + expect(result.rolesRemoved).toEqual(['ALB/Mist']); + expect(result.reactionsRemoved).toBe(1); + expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(responseChannel.send).toHaveBeenCalledWith( + expect.stringContaining('Removed 1 content role(s) and 1 content ping reaction(s) from <@1>'), + ); + }); + + it('should still clear reactions for someone who has left the server', async () => { + const result = await service.stripForDeregistration('1', null, responseChannel); + + expect(result.rolesRemoved).toEqual([]); + expect(result.reactionsRemoved).toBe(1); + expect(discordService.getAllRolesFromGuild).not.toHaveBeenCalled(); + }); + + it('should leave an exempt member entirely alone', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [exemptRoleId, contentRoleIds.mist]); + + const result = await service.stripForDeregistration('1', member, responseChannel); + + expect(result).toBeNull(); + expect(member.roles.remove).not.toHaveBeenCalled(); + expect(mockChannel.messages.fetch).not.toHaveBeenCalled(); + }); + + it('should do nothing when the pings message cannot be read', async () => { + mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + + expect(await service.stripForDeregistration('1', member, responseChannel)).toBeNull(); + expect(member.roles.remove).not.toHaveBeenCalled(); + }); + + it('should still clear reactions when the content roles cannot be resolved', async () => { + discordService.getAllRolesFromGuild.mockResolvedValue(new Collection() as any); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + + const result = await service.stripForDeregistration('1', member, responseChannel); + + expect(result.rolesRemoved).toEqual([]); + expect(result.reactionsRemoved).toBe(1); + expect(member.roles.remove).not.toHaveBeenCalled(); + }); + + it('should stay quiet when there was nothing to remove', async () => { + mockPingsMessage.reactions.cache = new Collection(); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', []); + + const result = await service.stripForDeregistration('1', member, responseChannel); + + expect(result.rolesRemoved).toEqual([]); + expect(result.reactionsRemoved).toBe(0); + expect(responseChannel.send).not.toHaveBeenCalled(); + }); + + it('should report an error and ping the dev', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + member.roles.remove.mockRejectedValueOnce(new Error('Discord says no')); + + await service.stripForDeregistration('1', member, responseChannel); + + expect(responseChannel.send).toHaveBeenCalledWith( + expect.stringContaining(`Discord says no Pinging <@${devUserId}>!`), + ); + }); + + it('should attempt every reaction rather than paging the reactor lists', async () => { + const first = TestBootstrapper.getMockContentReaction('โš”๏ธ', []); + const second = TestBootstrapper.getMockContentReaction('โ˜๏ธ', []); + mockPingsMessage.reactions.cache = new Collection([['0', first], ['1', second]]); + + await service.stripForDeregistration('1', null, responseChannel); + + expect(first.users.fetch).not.toHaveBeenCalled(); + expect(first.users.remove).toHaveBeenCalledWith('1'); + expect(second.users.remove).toHaveBeenCalledWith('1'); + }); + }); + + describe('reconcile', () => { + let scanMessage: any; + let members: Collection; + + // Per-member lines are sent as a placeholder and then edited in, so the report is only + // legible to a test that watches both calls. + let edits: string[]; + + const buildScanMessage = () => { + const guild = { + id: TestBootstrapper.mockConfig.discord.guildId, + members: { fetch: jest.fn().mockImplementation(async () => members) }, + }; + return { + id: 'scan-message', + guild, + edit: jest.fn(), + delete: jest.fn(), + channel: { + guild, + send: jest.fn().mockImplementation(async () => ({ + edit: jest.fn().mockImplementation(async (content: string) => edits.push(content)), + })), + }, + } as any; + }; + + const sentMessages = () => [ + ...(scanMessage.channel.send as jest.Mock).mock.calls.map((call) => call[0]), + ...edits, + ]; + + beforeEach(() => { + members = new Collection(); + edits = []; + scanMessage = buildScanMessage(); + }); + + it('should skip and say so when the pings message cannot be read', async () => { + mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + + expect(await service.reconcile(scanMessage)).toBe(false); + expect(sentMessages().join()).toContain('the content pings message could not be read'); + }); + + it('should skip and say so when no content roles resolve', async () => { + discordService.getAllRolesFromGuild.mockResolvedValue(new Collection() as any); + + expect(await service.reconcile(scanMessage)).toBe(false); + expect(sentMessages().join()).toContain('no content roles could be resolved'); + expect(scanMessage.guild.members.fetch).not.toHaveBeenCalled(); + }); + + it('should warn about role names in the embed that do not exist', async () => { + mockPingsMessage.embeds[0].description += '\n๐Ÿพ ALB/Tracking - if you want to go hunting'; + + await service.reconcile(scanMessage); + + expect(sentMessages().join()).toContain('**ALB/Tracking**'); + }); + + it('should strip an unregistered member of their content roles and reactions', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [contentRoleIds.mist]); + members.set('leaver', member); + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['leaver']); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(true); + expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(reaction.users.remove).toHaveBeenCalledWith('leaver'); + expect(sentMessages().join()).toContain('1 member(s) stripped of content roles'); + }); + + it('should leave a registered member alone', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('keeper', [contentRoleIds.mist]); + members.set('keeper', member); + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['keeper']); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + mockRepository.find.mockResolvedValue([{ discordId: 'keeper' } as AlbionRegistrationsEntity]); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(false); + expect(member.roles.remove).not.toHaveBeenCalled(); + expect(reaction.users.remove).not.toHaveBeenCalled(); + expect(mockRepository.find).toHaveBeenCalledWith({ guildId: albionGuildId }); + }); + + it('should leave an unregistered alliance member alone', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('ally', [exemptRoleId, contentRoleIds.mist]); + members.set('ally', member); + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['ally']); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(false); + expect(member.roles.remove).not.toHaveBeenCalled(); + expect(reaction.users.remove).not.toHaveBeenCalled(); + }); + + it('should ignore bots holding a content role', async () => { + const bot = TestBootstrapper.getMockGuildMemberWithRoles('bot', [contentRoleIds.mist], true); + members.set('bot', bot); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(false); + expect(bot.roles.remove).not.toHaveBeenCalled(); + }); + + it('should clear the reactions of someone who has left the Discord server', async () => { + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['ghost']); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(true); + expect(reaction.users.remove).toHaveBeenCalledWith('ghost'); + expect(sentMessages().join()).toContain('has left the Discord server'); + }); + + it('should never strip a rank role', async () => { + mockPingsMessage.embeds[0].description += '\n๐Ÿ‘‘ ALB/Archmage - the guild leader'; + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [rankRoleId, contentRoleIds.mist]); + members.set('leaver', member); + + await service.reconcile(scanMessage); + + expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(member.roles.remove).not.toHaveBeenCalledWith(rankRoleId); + }); + + it('should change nothing on a dry run but still report', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [contentRoleIds.mist]); + members.set('leaver', member); + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['leaver']); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + const actionRequired = await service.reconcile(scanMessage, true); + + expect(actionRequired).toBe(true); + expect(member.roles.remove).not.toHaveBeenCalled(); + expect(reaction.users.remove).not.toHaveBeenCalled(); + expect(sentMessages().join()).toContain('(DRY RUN)'); + }); + + it('should say so when nothing needs doing', async () => { + expect(await service.reconcile(scanMessage)).toBe(false); + expect(sentMessages().join()).toContain('No content role inconsistencies were detected'); + }); + + it('should report a live membership count for every content role', async () => { + members.set('a', TestBootstrapper.getMockGuildMemberWithRoles('a', [contentRoleIds.mist])); + members.set('b', TestBootstrapper.getMockGuildMemberWithRoles('b', [contentRoleIds.mist])); + mockRepository.find.mockResolvedValue([ + { discordId: 'a' } as AlbionRegistrationsEntity, + { discordId: 'b' } as AlbionRegistrationsEntity, + ]); + + await service.reconcile(scanMessage); + + const report = sentMessages().join(); + expect(report).toContain('**ALB/Mist**: 2'); + expect(report).toContain('**ALB/Arena**: 0'); + }); + + it('should count only what survives the sweep', async () => { + members.set('a', TestBootstrapper.getMockGuildMemberWithRoles('a', [contentRoleIds.mist])); + const leaver = TestBootstrapper.getMockGuildMemberWithRoles('b', [contentRoleIds.mist]); + // The sweep removes the role, so the live count must not include them any more + leaver.roles.remove.mockImplementation(async () => leaver.roles.cache.has.mockReturnValue(false)); + members.set('b', leaver); + mockRepository.find.mockResolvedValue([{ discordId: 'a' } as AlbionRegistrationsEntity]); + + await service.reconcile(scanMessage); + + expect(sentMessages().join()).toContain('**ALB/Mist**: 1'); + }); + + it('should defer reactions beyond the per-run cap to the next run', async () => { + const ghostIds = Array.from({ length: MAX_REACTION_REMOVALS_PER_RUN + 10 }, (_, index) => `ghost-${index}`); + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ghostIds); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + await service.reconcile(scanMessage); + + expect(reaction.users.remove).toHaveBeenCalledTimes(MAX_REACTION_REMOVALS_PER_RUN); + expect(sentMessages().join()).toContain('10 reaction(s) were left for the next run'); + }); + + it('should report removal errors and ping the dev', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [contentRoleIds.mist]); + member.roles.remove.mockRejectedValueOnce(new Error('Discord says no')); + members.set('leaver', member); + + await service.reconcile(scanMessage); + + expect(sentMessages().join()).toContain(`Discord says no Pinging <@${devUserId}>!`); + }); + + it('should sweep a member who reacted but holds no content role', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', []); + members.set('leaver', member); + const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['leaver']); + mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(true); + expect(reaction.users.remove).toHaveBeenCalledWith('leaver'); + expect(sentMessages().join()).toContain('is not registered'); + }); + }); +}); diff --git a/src/albion/services/albion.content.role.service.ts b/src/albion/services/albion.content.role.service.ts new file mode 100644 index 000000000..a518a62fd --- /dev/null +++ b/src/albion/services/albion.content.role.service.ts @@ -0,0 +1,451 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; +import { InjectRepository } from '@mikro-orm/nestjs'; +import { EntityRepository } from '@mikro-orm/core'; +import { + Collection, + Guild, + GuildMember, + GuildTextBasedChannel, + Message, + MessageReaction, + Role, + Snowflake, +} from 'discord.js'; +import { AlbionRegistrationsEntity } from '../../database/entities/albion.registrations.entity'; +import { AlbionRoleMapInterface } from '../../config/albion.app.config'; +import { ALBION_GUILD_EMOJI } from '../interfaces/albion.api.interfaces'; +import { DiscordService } from '../../discord/discord.service'; +import { getChannel } from '../../discord/discord.hacks'; + +// Every line of the pings embed reads " ALB/Name - description". Only the role token is +// load bearing: anyone who shouldn't be there loses every content role and every reaction they +// hold, so nothing needs to know which emoji belongs to which role. +export const CONTENT_ROLE_TOKEN = /\bALB\/[A-Za-z0-9_-]+/g; + +// reaction.users.fetch() returns 100 at most, so a popular ping needs paging or everyone past +// the first page is invisible to the sweep. The page cap is a runaway guard, nothing more. +const REACTION_PAGE_SIZE = 100; +const MAX_REACTION_PAGES = 50; + +// A safety valve for the first live run, which has years of drift to clear. Whatever is left +// over is picked up by the next run rather than hammering the API in one go. +export const MAX_REACTION_REMOVALS_PER_RUN = 500; + +export interface ContentRoleSet { + roles: Collection; + // Role names in the embed that match no Discord role. Reported rather than dropped, since + // it means the embed and the server have drifted apart. + unresolved: string[]; +} + +export interface ContentRoleStripResult { + discordId: string; + rolesRemoved: string[]; + reactionsRemoved: number; + errors: string[]; +} + +@Injectable() +export class AlbionContentRoleService { + private readonly logger = new Logger(AlbionContentRoleService.name); + + constructor( + private readonly config: ConfigService, + private readonly discordService: DiscordService, + @InjectRepository(AlbionRegistrationsEntity) private readonly albionRegistrationsRepository: EntityRepository, + ) {} + + // Null means "content roles could not be established". Callers must treat it as do-nothing: + // a failed fetch reading as an empty role list would strip the entire server. + async getContentRoles(guild: Guild): Promise { + const message = await this.getPingsMessage(); + + if (!message) { + return null; + } + + const tokens = [...new Set(this.extractText(message).match(CONTENT_ROLE_TOKEN) ?? [])]; + + if (tokens.length === 0) { + this.logger.error('Content Roles: No ALB/ role names found in the content pings message!'); + return null; + } + + const guildRoles = await this.discordService.getAllRolesFromGuild(guild); + const rankRoleIds = new Set( + (this.config.get('albion.roleMap') as AlbionRoleMapInterface[]).map((role) => role.discordRoleId), + ); + + const roles = new Collection(); + const unresolved: string[] = []; + + for (const token of tokens) { + const role = guildRoles.find((guildRole) => guildRole.name === token); + + if (!role) { + unresolved.push(token); + continue; + } + + // Rank roles share the ALB/ prefix. Excluded by ID whatever the embed happens to say, + // so a stray mention of a rank in the text can never strip someone of their rank. + if (rankRoleIds.has(role.id)) { + this.logger.warn(`Content Roles: Ignoring rank role "${role.name}" found in the pings message.`); + continue; + } + + roles.set(role.id, role); + } + + if (roles.size === 0) { + this.logger.error('Content Roles: None of the role names in the pings message resolved to a Discord role!'); + return null; + } + + return { roles, unresolved }; + } + + // The message is fetched fresh every time. It is edited by hand whenever a content role is + // added, and a cached copy would keep the sweep working off the old list. + async getPingsMessage(): Promise { + const messageId = this.config.get('albion.contentPingsMessageId'); + const channelId = this.config.get('discord.channels.albionRoles'); + + if (!messageId) { + this.logger.warn('Content Roles: No content pings message ID is configured, skipping.'); + return null; + } + + try { + const channel = await this.discordService.getTextChannel(channelId); + return await channel.messages.fetch(messageId); + } + catch (err) { + this.logger.error(`Content Roles: Failed to fetch the content pings message ${messageId}! Err: ${err.message}`); + return null; + } + } + + extractText(message: Message): string { + const parts: string[] = [message.content ?? '']; + + for (const embed of message.embeds ?? []) { + parts.push(embed.title ?? '', embed.description ?? ''); + + for (const field of embed.fields ?? []) { + parts.push(field.name ?? '', field.value ?? ''); + } + } + + return parts.join('\n'); + } + + // Who has reacted to the pings message, by Discord ID. Users who have left the server still + // appear here - Discord never prunes reactions - which is exactly the drift being cleaned up. + async fetchReactors(message: Message): Promise> { + const reactors = new Map(); + + for (const reaction of message.reactions.cache.values()) { + let after: Snowflake | undefined; + + for (let page = 0; page < MAX_REACTION_PAGES; page++) { + const users = await reaction.users.fetch({ limit: REACTION_PAGE_SIZE, after }); + + if (users.size === 0) { + break; + } + + for (const [discordId, user] of users) { + if (user.bot) { + continue; + } + reactors.set(discordId, [...(reactors.get(discordId) ?? []), reaction]); + } + + if (users.size < REACTION_PAGE_SIZE) { + break; + } + + after = users.lastKey(); + } + } + + return reactors; + } + + isExempt(member: GuildMember | null): boolean { + if (!member) { + return false; + } + + const exemptRoles: string[] = this.config.get('albion.contentRoleExemptRoles') ?? []; + + return exemptRoles.some((roleId) => member.roles.cache.has(roleId)); + } + + async removeContentRoles( + member: GuildMember, + contentRoles: Collection, + dryRun: boolean, + ): Promise<{ removed: string[], errors: string[] }> { + const removed: string[] = []; + const errors: string[] = []; + + for (const role of contentRoles.values()) { + if (!member.roles.cache.has(role.id)) { + continue; + } + + removed.push(role.name); + + if (dryRun) { + continue; + } + + try { + await member.roles.remove(role.id); + } + catch (err) { + errors.push(`Failed to remove role "${role.name}" from <@${member.id}>: ${err.message}`); + this.logger.error(`Content Roles: Failed to remove role ${role.name} from ${member.id}! Err: ${err.message}`); + } + } + + return { removed, errors }; + } + + // Reactions can only be removed, never added on someone's behalf, so clearing them is what + // lets a returning member opt back in by clicking the emoji again. + async removeReactions( + reactions: MessageReaction[], + discordId: Snowflake, + dryRun: boolean, + ): Promise<{ removed: number, errors: string[] }> { + const errors: string[] = []; + let removed = 0; + + for (const reaction of reactions) { + removed++; + + if (dryRun) { + continue; + } + + try { + await reaction.users.remove(discordId); + } + catch (err) { + removed--; + errors.push(`Failed to remove reaction ${reaction.emoji.name} from <@${discordId}>: ${err.message}`); + this.logger.error(`Content Roles: Failed to remove reaction from ${discordId}! Err: ${err.message}`); + } + } + + return { removed, errors }; + } + + // Called from deregistration, so it runs for every leaver the scan finds and for every manual + // deregistration. discordMember is null when they have already left the server, in which case + // there are no roles left to strip but their reactions outlive them. + async stripForDeregistration( + discordId: Snowflake, + discordMember: GuildMember | null, + responseChannel: GuildTextBasedChannel, + ): Promise { + // Alliance members hold content roles without ever registering. Exempt uniformly rather + // than only in the sweep, so leaving the guild but staying in the alliance keeps the pings. + if (this.isExempt(discordMember)) { + this.logger.log(`Content Roles: ${discordId} holds an exempt role, leaving their content roles alone.`); + return null; + } + + const pingsMessage = await this.getPingsMessage(); + + if (!pingsMessage) { + return null; + } + + const result: ContentRoleStripResult = { + discordId, + rolesRemoved: [], + reactionsRemoved: 0, + errors: [], + }; + + if (discordMember) { + const contentRoles = await this.getContentRoles(discordMember.guild); + + if (contentRoles) { + const roleResult = await this.removeContentRoles(discordMember, contentRoles.roles, false); + result.rolesRemoved = roleResult.removed; + result.errors.push(...roleResult.errors); + } + } + + // Every reaction on the message is attempted rather than paging the reactor lists first: + // deregistrations are rare and one-by-one, and the delete is a no-op if they never reacted. + const reactionResult = await this.removeReactions( + [...pingsMessage.reactions.cache.values()], + discordId, + false, + ); + result.reactionsRemoved = reactionResult.removed; + result.errors.push(...reactionResult.errors); + + if (result.rolesRemoved.length > 0 || result.reactionsRemoved > 0) { + await responseChannel.send( + `${ALBION_GUILD_EMOJI} ๐Ÿงน Removed ${result.rolesRemoved.length} content role(s) and ${result.reactionsRemoved} content ping reaction(s) from <@${discordId}>.`, + ); + } + + for (const error of result.errors) { + await responseChannel.send(`โš ๏ธ ${error} Pinging <@${this.config.get('discord.devUserId')}>!`); + } + + return result; + } + + // The reconciliation sweep. Clears historic drift the deregistration hook can't reach: people + // who left before the hook existed, and people who left the Discord server outright. + async reconcile( + scanMessage: Message, + dryRun = false, + ): Promise { + const emoji = ALBION_GUILD_EMOJI; + const guild = scanMessage.guild; + const pingsMessage = await this.getPingsMessage(); + + if (!pingsMessage) { + await getChannel(scanMessage).send(`${emoji} โš ๏ธ Content role sweep skipped: the content pings message could not be read.`); + return false; + } + + const contentRoleSet = await this.getContentRoles(guild); + + if (!contentRoleSet) { + await getChannel(scanMessage).send(`${emoji} โš ๏ธ Content role sweep skipped: no content roles could be resolved from the pings message.`); + return false; + } + + const { roles: contentRoles, unresolved } = contentRoleSet; + + if (unresolved.length > 0) { + await getChannel(scanMessage).send( + `${emoji} โš ๏ธ The content pings message names ${unresolved.length} role(s) that don't exist on the server: **${unresolved.join('**, **')}**. Fix the message or the role names.`, + ); + } + + // Full member list rather than role.members, which only reflects whatever happens to be + // cached and quietly undercounts. + const members = await guild.members.fetch(); + const reactors = await this.fetchReactors(pingsMessage); + + const registeredIds = new Set( + (await this.albionRegistrationsRepository.find({ guildId: this.config.get('albion.guildId') })) + .map((registration) => registration.discordId), + ); + + const candidates = new Set(reactors.keys()); + + // Bots are dropped in the candidate loop below, which is the one place both sources meet + for (const [discordId, member] of members) { + if (contentRoles.some((role) => member.roles.cache.has(role.id))) { + candidates.add(discordId); + } + } + + const dryRunText = dryRun ? ' (DRY RUN)' : ''; + const changes: string[] = []; + const errors: string[] = []; + let reactionsRemoved = 0; + let capped = 0; + + for (const discordId of candidates) { + const member = members.get(discordId) ?? null; + + if (member?.user.bot) { + continue; + } + if (registeredIds.has(discordId)) { + continue; + } + if (this.isExempt(member)) { + continue; + } + + const rolesRemoved = member + ? await this.removeContentRoles(member, contentRoles, dryRun) + : { removed: [], errors: [] }; + errors.push(...rolesRemoved.errors); + + const memberReactions = reactors.get(discordId) ?? []; + let reactionCount = 0; + + if (reactionsRemoved + memberReactions.length > MAX_REACTION_REMOVALS_PER_RUN) { + capped += memberReactions.length; + } + else { + const reactionResult = await this.removeReactions(memberReactions, discordId, dryRun); + reactionCount = reactionResult.removed; + reactionsRemoved += reactionCount; + errors.push(...reactionResult.errors); + } + + if (rolesRemoved.removed.length === 0 && reactionCount === 0) { + continue; + } + + const status = member ? 'is not registered' : 'has left the Discord server'; + changes.push( + `- ${emoji} ๐Ÿงน${dryRunText} <@${discordId}> ${status}: removed ${rolesRemoved.removed.length} content role(s) and ${reactionCount} reaction(s).`, + ); + } + + await this.report(scanMessage, changes, errors, capped, contentRoles, members, dryRun); + + return changes.length > 0; + } + + private async report( + scanMessage: Message, + changes: string[], + errors: string[], + capped: number, + contentRoles: Collection, + members: Collection, + dryRun: boolean, + ): Promise { + const emoji = ALBION_GUILD_EMOJI; + const dryRunText = dryRun ? ' (DRY RUN)' : ''; + + if (changes.length === 0) { + await getChannel(scanMessage).send(`${emoji} โœ… No content role inconsistencies were detected.`); + } + else { + await getChannel(scanMessage).send(`## ${emoji} ๐Ÿงน${dryRunText} ${changes.length} member(s) stripped of content roles!`); + + for (const change of changes) { + const lineMessage = await getChannel(scanMessage).send('.'); + await lineMessage.edit(change); + } + } + + if (capped > 0) { + await getChannel(scanMessage).send( + `${emoji} โ„น๏ธ ${capped} reaction(s) were left for the next run to avoid hammering Discord's API.`, + ); + } + + for (const error of errors) { + await getChannel(scanMessage).send(`โš ๏ธ ${error} Pinging <@${this.config.get('discord.devUserId')}>!`); + } + + // The whole point of the sweep: the counts on the roles are now a live membership count. + const counts = contentRoles + .map((role) => `- **${role.name}**: ${members.filter((member) => member.roles.cache.has(role.id)).size}`) + .join('\n'); + + await getChannel(scanMessage).send(`### ${emoji} Content role membership\n${counts}`); + } +} diff --git a/src/albion/services/albion.deregistration.service.spec.ts b/src/albion/services/albion.deregistration.service.spec.ts index 3a8120d67..621e3481b 100644 --- a/src/albion/services/albion.deregistration.service.spec.ts +++ b/src/albion/services/albion.deregistration.service.spec.ts @@ -9,6 +9,7 @@ import { EntityRepository } from '@mikro-orm/core'; import { AlbionDeregistrationService } from './albion.deregistration.service'; import { DiscordService } from '../../discord/discord.service'; import { AlbionDeregisterDto } from '../dto/albion.deregister.dto'; +import { AlbionContentRoleService } from './albion.content.role.service'; import { Role } from 'discord.js'; let mockAlbionRegistrationsRepository: jest.Mocked>; @@ -22,6 +23,7 @@ describe('AlbionDeregistrationService', () => { let service: AlbionDeregistrationService; let discordService: jest.Mocked; let configService: jest.Mocked; + let albionContentRoleService: jest.Mocked; beforeEach(async () => { mockCharacter = TestBootstrapper.getMockAlbionCharacter(); @@ -59,6 +61,12 @@ describe('AlbionDeregistrationService', () => { getRoleViaMember: jest.fn(), }, }, + { + provide: AlbionContentRoleService, + useValue: { + stripForDeregistration: jest.fn(), + }, + }, { provide: getRepositoryToken(AlbionRegistrationsEntity), useValue: mockAlbionRegistrationsRepository, @@ -71,6 +79,7 @@ describe('AlbionDeregistrationService', () => { service = moduleRef.get(AlbionDeregistrationService); discordService = moduleRef.get(DiscordService) as any; configService = moduleRef.get(ConfigService) as any; + albionContentRoleService = moduleRef.get(AlbionContentRoleService) as any; // Ensure albion.roleMap + devUserId available for role stripping tests (configService.get as jest.Mock).mockImplementation((key: string) => { @@ -174,6 +183,40 @@ describe('AlbionDeregistrationService', () => { expect(stripRolesSpy).not.toHaveBeenCalled(); }); + it('should strip content roles and reactions for the member', async () => { + const dto: AlbionDeregisterDto = { discordMember: mockDiscordMember.user }; + + await service.deregister(mockChannel, dto); + + expect(albionContentRoleService.stripForDeregistration).toHaveBeenCalledWith( + mockRegistration.discordId, + mockDiscordMember, + mockChannel, + ); + }); + + it('should still clear content ping reactions when the member has left the server', async () => { + const dto: AlbionDeregisterDto = { discordMember: mockDiscordMember.user }; + discordService.getGuildMember.mockRejectedValueOnce(new Error('Gone')); + + await service.deregister(mockChannel, dto); + + expect(albionContentRoleService.stripForDeregistration).toHaveBeenCalledWith( + mockRegistration.discordId, + null, + mockChannel, + ); + }); + + it('should not strip content roles when there was no registration to remove', async () => { + const dto: AlbionDeregisterDto = { discordMember: mockDiscordMember.user }; + mockAlbionRegistrationsRepository.findOne.mockResolvedValueOnce(null); + + await service.deregister(mockChannel, dto); + + expect(albionContentRoleService.stripForDeregistration).not.toHaveBeenCalled(); + }); + it('should skip role stripping if member fetch fails (character path)', async () => { const dto: AlbionDeregisterDto = { character: mockRegistration.characterName }; mockAlbionRegistrationsRepository.findOne.mockResolvedValueOnce(mockRegistration); diff --git a/src/albion/services/albion.deregistration.service.ts b/src/albion/services/albion.deregistration.service.ts index e6d6ae597..3fdee0bb8 100644 --- a/src/albion/services/albion.deregistration.service.ts +++ b/src/albion/services/albion.deregistration.service.ts @@ -7,6 +7,7 @@ import { GuildMember, GuildTextBasedChannel } from 'discord.js'; import { AlbionRoleMapInterface } from '../../config/albion.app.config'; import { DiscordService } from '../../discord/discord.service'; import { AlbionDeregisterDto } from '../dto/albion.deregister.dto'; +import { AlbionContentRoleService } from './albion.content.role.service'; @Injectable() export class AlbionDeregistrationService { @@ -15,6 +16,7 @@ export class AlbionDeregistrationService { constructor( private readonly config: ConfigService, private readonly discordService: DiscordService, + private readonly albionContentRoleService: AlbionContentRoleService, @InjectRepository(AlbionRegistrationsEntity) private readonly albionRegistrationsRepository: EntityRepository, ) { } @@ -67,6 +69,14 @@ export class AlbionDeregistrationService { await this.stripRegistration(registration, responseChannel); + // Runs whether or not they're still on the server: their reactions on the content pings + // message outlive them, and Discord never prunes reactions of people who have left. + await this.albionContentRoleService.stripForDeregistration( + registration.discordId, + discordMember, + responseChannel, + ); + if (!discordMember) { const error = `Discord Member with ID "${registration.discordId}" not found in Discord! They have likely left the server, so no roles can be stripped.`; this.logger.warn(error); diff --git a/src/albion/services/albion.scanning.service.spec.ts b/src/albion/services/albion.scanning.service.spec.ts index 5fca947a9..cd4c27636 100644 --- a/src/albion/services/albion.scanning.service.spec.ts +++ b/src/albion/services/albion.scanning.service.spec.ts @@ -11,6 +11,7 @@ import { AlbionApiService } from './albion.api.service'; import { AlbionUtilities } from '../utilities/albion.utilities'; import { TestBootstrapper } from '../../test.bootstrapper'; import { AlbionDeregistrationService } from './albion.deregistration.service'; +import { AlbionContentRoleService } from './albion.content.role.service'; const mockDevUserId = TestBootstrapper.mockConfig.discord.devUserId; const mockScanUserId = '1337'; @@ -33,6 +34,7 @@ describe('AlbionScanningService', () => { let service: AlbionScanningService; let albionApiService: AlbionApiService; let albionDeregistrationService: AlbionDeregistrationService; + let albionContentRoleService: jest.Mocked; let mockDiscordUser: any; let mockDiscordMessage: any; @@ -83,6 +85,12 @@ describe('AlbionScanningService', () => { deregister: jest.fn(), }, }, + { + provide: AlbionContentRoleService, + useValue: { + reconcile: jest.fn().mockResolvedValue(false), + }, + }, { provide: getRepositoryToken(AlbionRegistrationsEntity), useValue: mockAlbionRegistrationsRepository, @@ -93,6 +101,7 @@ describe('AlbionScanningService', () => { service = moduleRef.get(AlbionScanningService); albionApiService = moduleRef.get(AlbionApiService); albionDeregistrationService = moduleRef.get(AlbionDeregistrationService); + albionContentRoleService = moduleRef.get(AlbionContentRoleService) as any; const albionMerged = { ...TestBootstrapper.mockConfig.albion, @@ -220,10 +229,11 @@ describe('AlbionScanningService', () => { await service.startScan(mockDiscordMessage, false); expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Starting scan...'); - expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [1/4] Gathering 1 characters from the ALB API...'); - expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [2/4] Checking 1 characters for membership status...'); - expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [3/4] Performing reverse role scan...'); - expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [4/4] Checking for role inconsistencies...'); + expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [1/5] Gathering 1 characters from the ALB API...'); + expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [2/5] Checking 1 characters for membership status...'); + expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [3/5] Performing reverse role scan...'); + expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [4/5] Checking for role inconsistencies...'); + expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [5/5] Sweeping content roles...'); expect(mockDiscordMessage.channel.send).toHaveBeenCalledWith('## ๐Ÿ‡ช๐Ÿ‡บ Scan complete!'); expect(mockDiscordMessage.channel.send).toHaveBeenCalledWith('------------------------------------------'); expect(mockDiscordMessage.delete).toHaveBeenCalled(); @@ -243,6 +253,19 @@ describe('AlbionScanningService', () => { expect(service.removeLeavers).toHaveBeenCalledTimes(1); expect(service.reverseRoleScan).toHaveBeenCalledTimes(1); expect(service.roleInconsistencies).toHaveBeenCalledTimes(1); + expect(albionContentRoleService.reconcile).toHaveBeenCalledTimes(1); + }); + + it('should pass the dry run flag through to the content role sweep', async () => { + mockAlbionRegistrationsRepository.find = jest.fn().mockResolvedValueOnce([mockRegisteredMember]); + service.gatherCharacters = jest.fn().mockResolvedValueOnce([mockCharacter]); + service.removeLeavers = jest.fn().mockResolvedValueOnce([]); + service.reverseRoleScan = jest.fn().mockResolvedValueOnce([]); + service.roleInconsistencies = jest.fn().mockResolvedValueOnce([]); + + await service.startScan(mockDiscordMessage, true); + + expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(mockDiscordMessage, true); }); it('should properly ping the correct leaders when action is required', async () => { diff --git a/src/albion/services/albion.scanning.service.ts b/src/albion/services/albion.scanning.service.ts index 50eeef567..475a36181 100644 --- a/src/albion/services/albion.scanning.service.ts +++ b/src/albion/services/albion.scanning.service.ts @@ -10,6 +10,7 @@ import { AlbionRoleMapInterface } from '../../config/albion.app.config'; import { AlbionUtilities } from '../utilities/albion.utilities'; import { getChannel } from '../../discord/discord.hacks'; import { AlbionDeregistrationService } from './albion.deregistration.service'; +import { AlbionContentRoleService } from './albion.content.role.service'; export interface RoleInconsistencyResult { id: string, @@ -27,6 +28,7 @@ export class AlbionScanningService { private readonly albionApiService: AlbionApiService, private readonly config: ConfigService, private readonly albionDeregistrationService: AlbionDeregistrationService, + private readonly albionContentRoleService: AlbionContentRoleService, private readonly albionUtilities: AlbionUtilities, @InjectRepository(AlbionRegistrationsEntity) private readonly albionRegistrationsRepository: EntityRepository, ) {} @@ -53,7 +55,7 @@ export class AlbionScanningService { let actionRequired = false; try { - await message.edit(`# ${emoji} Task: [1/4] Gathering ${length} characters from the ALB API...`); + await message.edit(`# ${emoji} Task: [1/5] Gathering ${length} characters from the ALB API...`); characters = await this.gatherCharacters(guildMembers, message, 0); } catch (err) { @@ -68,19 +70,23 @@ export class AlbionScanningService { } try { - await message.edit(`# ${emoji} Task: [2/4] Checking ${length} characters for membership status...`); + await message.edit(`# ${emoji} Task: [2/5] Checking ${length} characters for membership status...`); if (await this.removeLeavers(characters, message, dryRun)) { actionRequired = true; } // Check if members have roles they shouldn't have - await message.edit(`# ${emoji} Task: [3/4] Performing reverse role scan...`); + await message.edit(`# ${emoji} Task: [3/5] Performing reverse role scan...`); await this.reverseRoleScan(message, dryRun); - await message.edit(`# ${emoji} Task: [4/4] Checking for role inconsistencies...`); + await message.edit(`# ${emoji} Task: [4/5] Checking for role inconsistencies...`); if (await this.roleInconsistencies(message, dryRun)) { actionRequired = true; } + + // Runs after the leaver pass so it sees the registrations that pass has just removed + await message.edit(`# ${emoji} Task: [5/5] Sweeping content roles...`); + await this.albionContentRoleService.reconcile(message, dryRun); } catch (err) { await message.edit(`## ${emoji} โŒ An error occurred while scanning!`); diff --git a/src/config/albion.app.config.ts b/src/config/albion.app.config.ts index 66fa8f435..f15cbb6ae 100644 --- a/src/config/albion.app.config.ts +++ b/src/config/albion.app.config.ts @@ -119,6 +119,13 @@ const leadershipPing = isProduction // Filtered on priority rather than name because production spells tier 3 "EldritchMager". const ELECTOR_MAX_PRIORITY = 3; +// Roles that keep their content roles without a registration, e.g. the alliance role. Alliance +// members use the pings but never register with DIG, so the sweep would otherwise strip them. +const contentRoleExemptRoles = (process.env.ALBION_CONTENT_ROLE_EXEMPT_ROLES ?? '') + .split(',') + .map((roleId) => roleId.trim()) + .filter((roleId) => roleId.length > 0); + export default () => ({ guildId: '0_zTfLfASD2Wtw6Tc-yckA', roleMap, @@ -129,6 +136,10 @@ export default () => ({ // soft-leadership, so a human makes that call even after a successful vote. autoAssignRanks: ['@ALB/Graduate'], gameActivityName: 'Albion Online', // As Discord reports it in presence data + // The content pings embed is the source of truth for which ALB/ roles are content roles. + // Rank roles share the prefix, so the name alone can't tell the two apart. + contentPingsMessageId: process.env.MESSAGE_ALBION_CONTENT_PINGS, + contentRoleExemptRoles, scanExcludedUsers: [], // Discord IDs guildLeaderRole: findRole('@ALB/Archmage'), guildOfficerRole: findRole('@ALB/Magister'), diff --git a/src/test.bootstrapper.ts b/src/test.bootstrapper.ts index 405d2dcf2..5713711bf 100644 --- a/src/test.bootstrapper.ts +++ b/src/test.bootstrapper.ts @@ -310,6 +310,92 @@ export class TestBootstrapper { } as any; } + // Content role IDs and the embed that names them. The pings message is the only source of + // truth for which ALB/ roles are content roles, so most content role tests start here. + static readonly mockContentRoleIds = { + factionWarfare: '9000000000000001', + mist: '9000000000000002', + arena: '9000000000000003', + }; + + static getMockContentRoleCollection() { + return new Collection() + .set(this.mockContentRoleIds.factionWarfare, { id: this.mockContentRoleIds.factionWarfare, name: 'ALB/FactionWarfare' } as Role) + .set(this.mockContentRoleIds.mist, { id: this.mockContentRoleIds.mist, name: 'ALB/Mist' } as Role) + .set(this.mockContentRoleIds.arena, { id: this.mockContentRoleIds.arena, name: 'ALB/Arena' } as Role); + } + + static getMockContentPingsEmbedDescription() { + return [ + 'These Pings can be used by guildmembers to look for a group to play with.', + '**Content List**', + 'โš”๏ธ ALB/FactionWarfare - for all Faction Warfare related things', + 'โ˜๏ธ ALB/Mist - to seek for a partner for Duo Mists', + '๐ŸŸ๏ธ ALB/Arena - to find people for the arenas', + ].join('\n'); + } + + // A reaction whose users.fetch() honours Discord's paging contract, so a test can prove the + // sweep pages past the first 100 users. + static getMockContentReaction(emojiName: string, userIds: string[], botIds: string[] = []) { + const allIds = [...userIds, ...botIds]; + return { + emoji: { name: emojiName, id: null }, + count: allIds.length, + users: { + fetch: jest.fn().mockImplementation(async ({ limit = 100, after } = {} as any) => { + const start = after ? allIds.indexOf(after) + 1 : 0; + const page = allIds.slice(start, start + limit); + return new Collection( + page.map((id) => [id, { id, bot: botIds.includes(id) }]), + ); + }), + remove: jest.fn().mockResolvedValue(true), + }, + } as any; + } + + static getMockContentPingsMessage(reactions: any[] = []) { + return { + id: this.mockConfig.albion.contentPingsMessageId, + content: '', + embeds: [ + { + title: 'Albion Content Pings', + description: this.getMockContentPingsEmbedDescription(), + fields: [], + }, + ], + reactions: { + cache: new Collection(reactions.map((reaction, index) => [String(index), reaction])), + }, + } as any; + } + + // A member whose role cache answers honestly rather than with a blanket jest.fn(), which + // every role sweep depends on to tell holders from non-holders. + static getMockGuildMemberWithRoles( + id: string, + roleIds: string[] = [], + isBot = false, + guild: any = undefined, + ) { + const roles = new Set(roleIds); + return { + id, + displayName: `member-${id}`, + user: { id, username: `member-${id}`, bot: isBot }, + guild: guild ?? this.getMockGuild('123456789'), + roles: { + add: jest.fn(), + remove: jest.fn().mockResolvedValue(true), + cache: { + has: jest.fn().mockImplementation((roleId: string) => roles.has(roleId)), + }, + }, + } as any; + } + static getMockDiscordVoiceState(member, channel) { return { member: member, // The GuildMember object, mocked separately @@ -448,6 +534,8 @@ export class TestBootstrapper { electorMaxPriority: 3, autoAssignRanks: ['@ALB/Graduate'], gameActivityName: 'Albion Online', + contentPingsMessageId: '1401401401401401401', + contentRoleExemptRoles: ['7070707070707070'], }, discord: { guildId: '657687978899', From d41d2b14bdce97143c91b4350d15abea1dbf92a4 Mon Sep 17 00:00:00 2001 From: Matt Cavanagh <1776058+Maelstromeous@users.noreply.github.com> Date: Mon, 3 Aug 2026 18:21:03 +0100 Subject: [PATCH 2/3] refactor(albion): Sweep content roles only via the scan, and report its progress Removes /albion-content-roles. The sweep already runs as step 5 of startScan, which the daily cron drives, so the command was a second entry point to the same work with no way to serialise concurrent runs. Adds throttled progress to the sweep: the scan message is edited at most once every two seconds with members processed and reactions removed, so a run clearing hundreds of reactions doesn't look hung. The clock starts before the first member, so a short sweep emits no edits at all, and a failed edit is logged rather than taking the sweep down. Co-Authored-By: Claude Opus 5 --- src/albion/albion.module.ts | 2 - .../commands/content-roles.command.spec.ts | 75 ------------------- src/albion/commands/content-roles.command.ts | 42 ----------- .../albion.content.role.service.spec.ts | 49 +++++++++++- .../services/albion.content.role.service.ts | 50 +++++++++++++ .../services/albion.scanning.service.spec.ts | 6 +- .../services/albion.scanning.service.ts | 5 +- 7 files changed, 106 insertions(+), 123 deletions(-) delete mode 100644 src/albion/commands/content-roles.command.spec.ts delete mode 100644 src/albion/commands/content-roles.command.ts diff --git a/src/albion/albion.module.ts b/src/albion/albion.module.ts index f8c067b7c..8b1650ee3 100644 --- a/src/albion/albion.module.ts +++ b/src/albion/albion.module.ts @@ -7,7 +7,6 @@ import { AlbionRegistrationService } from './services/albion.registration.servic import { DiscordModule } from '../discord/discord.module'; import { AlbionScanningService } from './services/albion.scanning.service'; import { AlbionScanCommand } from './commands/scan.command'; -import { AlbionContentRolesCommand } from './commands/content-roles.command'; import { AlbionContentRoleService } from './services/albion.content.role.service'; import { AlbionCronService } from './services/albion.cron.service'; import { AlbionUtilities } from './utilities/albion.utilities'; @@ -32,7 +31,6 @@ import { GeneralModule } from '../general/general.module'; providers: [ AlbionApiService, AlbionContentRoleService, - AlbionContentRolesCommand, AlbionCronService, AlbionDeregisterCommand, AlbionDeregistrationService, diff --git a/src/albion/commands/content-roles.command.spec.ts b/src/albion/commands/content-roles.command.spec.ts deleted file mode 100644 index f8d4f2482..000000000 --- a/src/albion/commands/content-roles.command.spec.ts +++ /dev/null @@ -1,75 +0,0 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ -import { Test, TestingModule } from '@nestjs/testing'; -import { ConfigService } from '@nestjs/config'; -import { AlbionContentRolesCommand } from './content-roles.command'; -import { AlbionContentRoleService } from '../services/albion.content.role.service'; -import { TestBootstrapper } from '../../test.bootstrapper'; - -const scanChannelId = TestBootstrapper.mockConfig.discord.channels.albionScans; - -describe('AlbionContentRolesCommand', () => { - let command: AlbionContentRolesCommand; - let albionContentRoleService: AlbionContentRoleService; - let mockDiscordInteraction: any; - - beforeEach(async () => { - const moduleRef: TestingModule = await Test.createTestingModule({ - providers: [ - AlbionContentRolesCommand, - { - provide: ConfigService, - useValue: { get: jest.fn() }, - }, - { - provide: AlbionContentRoleService, - useValue: { reconcile: jest.fn() }, - }, - ], - }).compile(); - TestBootstrapper.setupConfig(moduleRef); - - command = moduleRef.get(AlbionContentRolesCommand); - albionContentRoleService = moduleRef.get(AlbionContentRoleService); - - mockDiscordInteraction = TestBootstrapper.getMockDiscordInteraction( - scanChannelId, - TestBootstrapper.getMockDiscordUser(), - ); - }); - - it('should be defined', () => { - expect(command).toBeDefined(); - }); - - it('should start the sweep when the channel is correct', async () => { - await command.onAlbionContentRolesCommand({ dryRun: false }, mockDiscordInteraction); - - expect(mockDiscordInteraction[0].deferReply).toHaveBeenCalled(); - expect(mockDiscordInteraction[0].channel.send).toHaveBeenCalledWith('Starting Albion content role sweep...'); - expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(expect.anything(), false); - }); - - it('should refuse to run outside the scans channel', async () => { - mockDiscordInteraction[0].channelId = 'wrongChannelId'; - - const response = await command.onAlbionContentRolesCommand({ dryRun: false }, mockDiscordInteraction); - - expect(response).toBe(`Please use the <#${scanChannelId}> channel to perform Scans.`); - expect(mockDiscordInteraction[0].channel.send).not.toHaveBeenCalled(); - expect(albionContentRoleService.reconcile).not.toHaveBeenCalled(); - }); - - it('should flag a dry run in the response', async () => { - const response = await command.onAlbionContentRolesCommand({ dryRun: true }, mockDiscordInteraction); - - expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(expect.anything(), true); - expect(response).toBe('Albion content role sweep initiated! [DRY RUN, NO CHANGES WILL ACTUALLY BE PERFORMED]'); - }); - - it('should default to a live run when the option is omitted', async () => { - const response = await command.onAlbionContentRolesCommand({} as any, mockDiscordInteraction); - - expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(expect.anything(), false); - expect(response).toBe('Albion content role sweep initiated!'); - }); -}); diff --git a/src/albion/commands/content-roles.command.ts b/src/albion/commands/content-roles.command.ts deleted file mode 100644 index aab307c3d..000000000 --- a/src/albion/commands/content-roles.command.ts +++ /dev/null @@ -1,42 +0,0 @@ -import { Context, Options, SlashCommand, SlashCommandContext } from 'necord'; -import { Injectable, Logger } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; -import { AlbionScanDto } from '../dto/albion.scan.dto'; -import { AlbionContentRoleService } from '../services/albion.content.role.service'; -import { replyTo } from '../../discord/discord.hacks'; - -@Injectable() -export class AlbionContentRolesCommand { - private readonly logger = new Logger(AlbionContentRolesCommand.name); - - constructor( - private readonly config: ConfigService, - private readonly albionContentRoleService: AlbionContentRoleService, - ) {} - - @SlashCommand({ - name: 'albion-content-roles', - description: 'Strip Albion content roles and ping reactions from members who are no longer registered', - }) - async onAlbionContentRolesCommand( - @Options() dto: AlbionScanDto, - @Context() [interaction]: SlashCommandContext, - ): Promise { - this.logger.debug('Received Albion Content Roles Command'); - // Fetching the member list and paging every reaction blows Discord's 3s reply window. - await interaction.deferReply(); - const dryRun = dto.dryRun ?? false; - - const scanChannelId = this.config.get('discord.channels.albionScans'); - - if (interaction.channelId !== scanChannelId) { - return replyTo(interaction, `Please use the <#${scanChannelId}> channel to perform Scans.`); - } - - const message = await interaction.channel.send('Starting Albion content role sweep...'); - - this.albionContentRoleService.reconcile(message, dryRun); - - return replyTo(interaction, `Albion content role sweep initiated!${dryRun ? ' [DRY RUN, NO CHANGES WILL ACTUALLY BE PERFORMED]' : ''}`); - } -} diff --git a/src/albion/services/albion.content.role.service.spec.ts b/src/albion/services/albion.content.role.service.spec.ts index 9389e4397..79cbbbd9a 100644 --- a/src/albion/services/albion.content.role.service.spec.ts +++ b/src/albion/services/albion.content.role.service.spec.ts @@ -3,7 +3,11 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ConfigService } from '@nestjs/config'; import { getRepositoryToken } from '@mikro-orm/nestjs'; import { Collection } from 'discord.js'; -import { AlbionContentRoleService, MAX_REACTION_REMOVALS_PER_RUN } from './albion.content.role.service'; +import { + AlbionContentRoleService, + MAX_REACTION_REMOVALS_PER_RUN, + PROGRESS_EDIT_INTERVAL_MS, +} from './albion.content.role.service'; import { AlbionRegistrationsEntity } from '../../database/entities/albion.registrations.entity'; import { DiscordService } from '../../discord/discord.service'; import { TestBootstrapper } from '../../test.bootstrapper'; @@ -661,6 +665,49 @@ describe('AlbionContentRoleService', () => { expect(sentMessages().join()).toContain(`Discord says no Pinging <@${devUserId}>!`); }); + describe('progress reporting', () => { + const addLeavers = (count: number) => { + for (let index = 0; index < count; index++) { + members.set(`leaver-${index}`, TestBootstrapper.getMockGuildMemberWithRoles(`leaver-${index}`, [contentRoleIds.mist])); + } + }; + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should not edit the scan message when the sweep is quick', async () => { + addLeavers(3); + + await service.reconcile(scanMessage); + + expect(scanMessage.edit).not.toHaveBeenCalled(); + }); + + it('should report progress against the heading it was given once the interval passes', async () => { + addLeavers(3); + // Every check of the clock reads two seconds later, so each member trips the throttle + let clock = Date.now(); + jest.spyOn(Date, 'now').mockImplementation(() => (clock += PROGRESS_EDIT_INTERVAL_MS)); + + await service.reconcile(scanMessage, false, '# Sweeping content roles...'); + + const progressEdits = (scanMessage.edit as jest.Mock).mock.calls.map((call) => call[0]); + expect(progressEdits[0]).toContain('# Sweeping content roles... [0/3] (0%)'); + // The last edit is the finishing one, so it always reads as complete + expect(progressEdits[progressEdits.length - 1]).toContain('[3/3] (100%)'); + }); + + it('should carry on when a progress edit fails', async () => { + addLeavers(3); + let clock = Date.now(); + jest.spyOn(Date, 'now').mockImplementation(() => (clock += PROGRESS_EDIT_INTERVAL_MS)); + scanMessage.edit.mockRejectedValue(new Error('Unknown Message')); + + expect(await service.reconcile(scanMessage)).toBe(true); + }); + }); + it('should sweep a member who reacted but holds no content role', async () => { const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', []); members.set('leaver', member); diff --git a/src/albion/services/albion.content.role.service.ts b/src/albion/services/albion.content.role.service.ts index a518a62fd..69a43cdbf 100644 --- a/src/albion/services/albion.content.role.service.ts +++ b/src/albion/services/albion.content.role.service.ts @@ -32,6 +32,9 @@ const MAX_REACTION_PAGES = 50; // over is picked up by the next run rather than hammering the API in one go. export const MAX_REACTION_REMOVALS_PER_RUN = 500; +// Progress edits cost an API call of their own, on the tightest bucket in the sweep +export const PROGRESS_EDIT_INTERVAL_MS = 2000; + export interface ContentRoleSet { roles: Collection; // Role names in the embed that match no Discord role. Reported rather than dropped, since @@ -311,6 +314,7 @@ export class AlbionContentRoleService { async reconcile( scanMessage: Message, dryRun = false, + heading = `# ${ALBION_GUILD_EMOJI} Sweeping content roles...`, ): Promise { const emoji = ALBION_GUILD_EMOJI; const guild = scanMessage.guild; @@ -360,10 +364,17 @@ export class AlbionContentRoleService { const errors: string[] = []; let reactionsRemoved = 0; let capped = 0; + let processed = 0; + + const progress = this.progressReporter(scanMessage, heading, candidates.size); for (const discordId of candidates) { const member = members.get(discordId) ?? null; + // Reports what is finished, before starting this one - so the counts never run ahead + await progress.update(processed, reactionsRemoved); + processed++; + if (member?.user.bot) { continue; } @@ -402,11 +413,50 @@ export class AlbionContentRoleService { ); } + await progress.finish(processed, reactionsRemoved); + await this.report(scanMessage, changes, errors, capped, contentRoles, members, dryRun); return changes.length > 0; } + // Editing the scan message as it goes, so a sweep clearing hundreds of reactions doesn't look + // hung. Throttled, and the clock starts before the first member, so a short sweep edits nothing. + private progressReporter(scanMessage: Message, heading: string, total: number) { + let lastEditAt = Date.now(); + let emitted = false; + + const edit = async (processed: number, reactionsRemoved: number): Promise => { + const percent = total > 0 ? Math.floor((processed / total) * 100) : 100; + + try { + await scanMessage.edit(`${heading} [${processed}/${total}] (${percent}%) โ€” ${reactionsRemoved} reaction(s) removed`); + } + catch (err) { + // A failed progress edit must never take the sweep down with it + this.logger.warn(`Content Roles: Failed to update sweep progress. Err: ${err.message}`); + } + }; + + return { + update: async (processed: number, reactionsRemoved: number): Promise => { + if (Date.now() - lastEditAt < PROGRESS_EDIT_INTERVAL_MS) { + return; + } + + lastEditAt = Date.now(); + emitted = true; + await edit(processed, reactionsRemoved); + }, + // Only when progress was reported at all, otherwise the heading is still accurate + finish: async (processed: number, reactionsRemoved: number): Promise => { + if (emitted) { + await edit(processed, reactionsRemoved); + } + }, + }; + } + private async report( scanMessage: Message, changes: string[], diff --git a/src/albion/services/albion.scanning.service.spec.ts b/src/albion/services/albion.scanning.service.spec.ts index cd4c27636..850a74361 100644 --- a/src/albion/services/albion.scanning.service.spec.ts +++ b/src/albion/services/albion.scanning.service.spec.ts @@ -265,7 +265,11 @@ describe('AlbionScanningService', () => { await service.startScan(mockDiscordMessage, true); - expect(albionContentRoleService.reconcile).toHaveBeenCalledWith(mockDiscordMessage, true); + expect(albionContentRoleService.reconcile).toHaveBeenCalledWith( + mockDiscordMessage, + true, + expect.stringContaining('Sweeping content roles'), + ); }); it('should properly ping the correct leaders when action is required', async () => { diff --git a/src/albion/services/albion.scanning.service.ts b/src/albion/services/albion.scanning.service.ts index 475a36181..873db7cac 100644 --- a/src/albion/services/albion.scanning.service.ts +++ b/src/albion/services/albion.scanning.service.ts @@ -85,8 +85,9 @@ export class AlbionScanningService { } // Runs after the leaver pass so it sees the registrations that pass has just removed - await message.edit(`# ${emoji} Task: [5/5] Sweeping content roles...`); - await this.albionContentRoleService.reconcile(message, dryRun); + const sweepHeading = `# ${emoji} Task: [5/5] Sweeping content roles...`; + await message.edit(sweepHeading); + await this.albionContentRoleService.reconcile(message, dryRun, sweepHeading); } catch (err) { await message.edit(`## ${emoji} โŒ An error occurred while scanning!`); From 3e05b33f0eb990474de45355829aa5b17de5a137 Mon Sep 17 00:00:00 2001 From: Matt Cavanagh <1776058+Maelstromeous@users.noreply.github.com> Date: Thu, 6 Aug 2026 01:37:19 +0100 Subject: [PATCH 3/3] feat(albion): Sweep both pings messages, not just the content one There are two pings embeds, not one: content pings and guild pings. The guild pings roles - Proclamations, Event, CTA, Gatherer, Crafter - drift exactly the same way, so sweeping only the content embed left half the problem in place. MESSAGE_ALBION_PINGS is a comma separated list, so a third embed is config rather than a deploy. Roles are unioned across every message and reactions are paged on all of them. One unreadable message no longer kills the run. It only means less gets swept - never that the wrong person is stripped - so the sweep continues on what it could read and shouts about what it could not. Reading none of them is still a hard abort. Renames content role to ping role throughout. Half these roles were never content roles, and a name that is wrong about half its subject is worse than the churn of fixing it. Co-Authored-By: Claude Opus 5 --- digletbot.env.example | 10 +- src/albion/albion.module.ts | 4 +- .../albion.deregistration.service.spec.ts | 20 +- .../services/albion.deregistration.service.ts | 6 +- ...ec.ts => albion.ping.role.service.spec.ts} | 328 ++++++++++++------ ...service.ts => albion.ping.role.service.ts} | 165 +++++---- .../services/albion.scanning.service.spec.ts | 18 +- .../services/albion.scanning.service.ts | 8 +- src/config/albion.app.config.ts | 17 +- src/test.bootstrapper.ts | 58 +++- 10 files changed, 398 insertions(+), 236 deletions(-) rename src/albion/services/{albion.content.role.service.spec.ts => albion.ping.role.service.spec.ts} (65%) rename src/albion/services/{albion.content.role.service.ts => albion.ping.role.service.ts} (68%) diff --git a/digletbot.env.example b/digletbot.env.example index 42548be47..ec326f7af 100644 --- a/digletbot.env.example +++ b/digletbot.env.example @@ -25,11 +25,11 @@ ROLE_ALBION_US_REGISTERED=12345 ROLE_ALBION_EU_REGISTERED=12345 ROLE_ALBION_US_ANNOUNCEMENTS=12345 ROLE_ALBION_EU_ANNOUNCEMENTS=12345 -# The content pings embed, in CHANNEL_ALBION_EU_ROLES. Its ALB/ role names drive the content -# role sweep; leave it unset to disable the sweep entirely. -MESSAGE_ALBION_CONTENT_PINGS=12345 -# Comma separated role IDs that keep content roles without an Albion registration (alliance). -ALBION_CONTENT_ROLE_EXEMPT_ROLES=12345,67890 +# Comma separated IDs of the pings embeds in CHANNEL_ALBION_EU_ROLES (content pings and guild +# pings). Their ALB/ role names drive the sweep; leave unset to disable the sweep entirely. +MESSAGE_ALBION_PINGS=12345,67890 +# Comma separated role IDs that keep ping roles without an Albion registration (alliance). +ALBION_PING_ROLE_EXEMPT_ROLES=12345,67890 # PS2 Channels & Roles CHANNEL_PS2_VERIFY=1234567 diff --git a/src/albion/albion.module.ts b/src/albion/albion.module.ts index 8b1650ee3..5b230196e 100644 --- a/src/albion/albion.module.ts +++ b/src/albion/albion.module.ts @@ -7,7 +7,7 @@ import { AlbionRegistrationService } from './services/albion.registration.servic import { DiscordModule } from '../discord/discord.module'; import { AlbionScanningService } from './services/albion.scanning.service'; import { AlbionScanCommand } from './commands/scan.command'; -import { AlbionContentRoleService } from './services/albion.content.role.service'; +import { AlbionPingRoleService } from './services/albion.ping.role.service'; import { AlbionCronService } from './services/albion.cron.service'; import { AlbionUtilities } from './utilities/albion.utilities'; import { AlbionLogCommand } from './commands/log.command'; @@ -30,7 +30,7 @@ import { GeneralModule } from '../general/general.module'; imports: [ConfigModule, DatabaseModule, DiscordModule, GeneralModule], providers: [ AlbionApiService, - AlbionContentRoleService, + AlbionPingRoleService, AlbionCronService, AlbionDeregisterCommand, AlbionDeregistrationService, diff --git a/src/albion/services/albion.deregistration.service.spec.ts b/src/albion/services/albion.deregistration.service.spec.ts index 621e3481b..df57e3e02 100644 --- a/src/albion/services/albion.deregistration.service.spec.ts +++ b/src/albion/services/albion.deregistration.service.spec.ts @@ -9,7 +9,7 @@ import { EntityRepository } from '@mikro-orm/core'; import { AlbionDeregistrationService } from './albion.deregistration.service'; import { DiscordService } from '../../discord/discord.service'; import { AlbionDeregisterDto } from '../dto/albion.deregister.dto'; -import { AlbionContentRoleService } from './albion.content.role.service'; +import { AlbionPingRoleService } from './albion.ping.role.service'; import { Role } from 'discord.js'; let mockAlbionRegistrationsRepository: jest.Mocked>; @@ -23,7 +23,7 @@ describe('AlbionDeregistrationService', () => { let service: AlbionDeregistrationService; let discordService: jest.Mocked; let configService: jest.Mocked; - let albionContentRoleService: jest.Mocked; + let albionPingRoleService: jest.Mocked; beforeEach(async () => { mockCharacter = TestBootstrapper.getMockAlbionCharacter(); @@ -62,7 +62,7 @@ describe('AlbionDeregistrationService', () => { }, }, { - provide: AlbionContentRoleService, + provide: AlbionPingRoleService, useValue: { stripForDeregistration: jest.fn(), }, @@ -79,7 +79,7 @@ describe('AlbionDeregistrationService', () => { service = moduleRef.get(AlbionDeregistrationService); discordService = moduleRef.get(DiscordService) as any; configService = moduleRef.get(ConfigService) as any; - albionContentRoleService = moduleRef.get(AlbionContentRoleService) as any; + albionPingRoleService = moduleRef.get(AlbionPingRoleService) as any; // Ensure albion.roleMap + devUserId available for role stripping tests (configService.get as jest.Mock).mockImplementation((key: string) => { @@ -183,38 +183,38 @@ describe('AlbionDeregistrationService', () => { expect(stripRolesSpy).not.toHaveBeenCalled(); }); - it('should strip content roles and reactions for the member', async () => { + it('should strip ping roles and reactions for the member', async () => { const dto: AlbionDeregisterDto = { discordMember: mockDiscordMember.user }; await service.deregister(mockChannel, dto); - expect(albionContentRoleService.stripForDeregistration).toHaveBeenCalledWith( + expect(albionPingRoleService.stripForDeregistration).toHaveBeenCalledWith( mockRegistration.discordId, mockDiscordMember, mockChannel, ); }); - it('should still clear content ping reactions when the member has left the server', async () => { + it('should still clear ping reactions when the member has left the server', async () => { const dto: AlbionDeregisterDto = { discordMember: mockDiscordMember.user }; discordService.getGuildMember.mockRejectedValueOnce(new Error('Gone')); await service.deregister(mockChannel, dto); - expect(albionContentRoleService.stripForDeregistration).toHaveBeenCalledWith( + expect(albionPingRoleService.stripForDeregistration).toHaveBeenCalledWith( mockRegistration.discordId, null, mockChannel, ); }); - it('should not strip content roles when there was no registration to remove', async () => { + it('should not strip ping roles when there was no registration to remove', async () => { const dto: AlbionDeregisterDto = { discordMember: mockDiscordMember.user }; mockAlbionRegistrationsRepository.findOne.mockResolvedValueOnce(null); await service.deregister(mockChannel, dto); - expect(albionContentRoleService.stripForDeregistration).not.toHaveBeenCalled(); + expect(albionPingRoleService.stripForDeregistration).not.toHaveBeenCalled(); }); it('should skip role stripping if member fetch fails (character path)', async () => { diff --git a/src/albion/services/albion.deregistration.service.ts b/src/albion/services/albion.deregistration.service.ts index 3fdee0bb8..0bed59740 100644 --- a/src/albion/services/albion.deregistration.service.ts +++ b/src/albion/services/albion.deregistration.service.ts @@ -7,7 +7,7 @@ import { GuildMember, GuildTextBasedChannel } from 'discord.js'; import { AlbionRoleMapInterface } from '../../config/albion.app.config'; import { DiscordService } from '../../discord/discord.service'; import { AlbionDeregisterDto } from '../dto/albion.deregister.dto'; -import { AlbionContentRoleService } from './albion.content.role.service'; +import { AlbionPingRoleService } from './albion.ping.role.service'; @Injectable() export class AlbionDeregistrationService { @@ -16,7 +16,7 @@ export class AlbionDeregistrationService { constructor( private readonly config: ConfigService, private readonly discordService: DiscordService, - private readonly albionContentRoleService: AlbionContentRoleService, + private readonly albionPingRoleService: AlbionPingRoleService, @InjectRepository(AlbionRegistrationsEntity) private readonly albionRegistrationsRepository: EntityRepository, ) { } @@ -71,7 +71,7 @@ export class AlbionDeregistrationService { // Runs whether or not they're still on the server: their reactions on the content pings // message outlive them, and Discord never prunes reactions of people who have left. - await this.albionContentRoleService.stripForDeregistration( + await this.albionPingRoleService.stripForDeregistration( registration.discordId, discordMember, responseChannel, diff --git a/src/albion/services/albion.content.role.service.spec.ts b/src/albion/services/albion.ping.role.service.spec.ts similarity index 65% rename from src/albion/services/albion.content.role.service.spec.ts rename to src/albion/services/albion.ping.role.service.spec.ts index 79cbbbd9a..ee08fc8f4 100644 --- a/src/albion/services/albion.content.role.service.spec.ts +++ b/src/albion/services/albion.ping.role.service.spec.ts @@ -4,37 +4,38 @@ import { ConfigService } from '@nestjs/config'; import { getRepositoryToken } from '@mikro-orm/nestjs'; import { Collection } from 'discord.js'; import { - AlbionContentRoleService, + AlbionPingRoleService, MAX_REACTION_REMOVALS_PER_RUN, PROGRESS_EDIT_INTERVAL_MS, -} from './albion.content.role.service'; +} from './albion.ping.role.service'; import { AlbionRegistrationsEntity } from '../../database/entities/albion.registrations.entity'; import { DiscordService } from '../../discord/discord.service'; import { TestBootstrapper } from '../../test.bootstrapper'; -const contentRoleIds = TestBootstrapper.mockContentRoleIds; -const exemptRoleId = TestBootstrapper.mockConfig.albion.contentRoleExemptRoles[0]; +const pingRoleIds = TestBootstrapper.mockPingRoleIds; +const exemptRoleId = TestBootstrapper.mockConfig.albion.pingRoleExemptRoles[0]; const rankRoleId = TestBootstrapper.mockConfig.albion.roleMap[0].discordRoleId; const albionGuildId = TestBootstrapper.mockConfig.albion.guildId; const devUserId = TestBootstrapper.mockConfig.discord.devUserId; -// Every role on the server, not just the content ones: the service has to pick the content -// roles out of a list that also holds rank roles sharing the same ALB/ prefix. -const allGuildRoles = () => TestBootstrapper.getMockContentRoleCollection() +// Every role on the server, not just the ping ones: the service has to pick the ping roles +// out of a list that also holds rank roles sharing the same ALB/ prefix. +const allGuildRoles = () => TestBootstrapper.getMockPingRoleCollection() .set(rankRoleId, { id: rankRoleId, name: 'ALB/Archmage' } as any) .set(exemptRoleId, { id: exemptRoleId, name: 'ALB/Alliance' } as any); -describe('AlbionContentRoleService', () => { - let service: AlbionContentRoleService; +describe('AlbionPingRoleService', () => { + let service: AlbionPingRoleService; let discordService: jest.Mocked; let mockRepository: any; let mockChannel: any; let mockPingsMessage: any; + let mockGuildPingsMessage: any; const setupModule = async (configOverride?: any) => { const moduleRef: TestingModule = await Test.createTestingModule({ providers: [ - AlbionContentRoleService, + AlbionPingRoleService, { provide: ConfigService, useValue: { get: jest.fn() }, @@ -55,7 +56,7 @@ describe('AlbionContentRoleService', () => { TestBootstrapper.setupConfig(moduleRef, configOverride); - service = moduleRef.get(AlbionContentRoleService); + service = moduleRef.get(AlbionPingRoleService); discordService = moduleRef.get(DiscordService) as any; discordService.getTextChannel.mockResolvedValue(mockChannel); @@ -70,9 +71,22 @@ describe('AlbionContentRoleService', () => { findOne: jest.fn(), }; + // Production has two pings embeds, so the fetch answers by ID rather than handing the same + // message back for both - otherwise every reaction would be counted twice. mockPingsMessage = TestBootstrapper.getMockContentPingsMessage(); + mockGuildPingsMessage = TestBootstrapper.getMockGuildPingsMessage(); mockChannel = { - messages: { fetch: jest.fn().mockResolvedValue(mockPingsMessage) }, + messages: { + fetch: jest.fn().mockImplementation(async (messageId: string) => { + const message = [mockPingsMessage, mockGuildPingsMessage].find((candidate) => candidate.id === messageId); + + if (!message) { + throw new Error('Unknown Message'); + } + + return message; + }), + }, }; await setupModule(); @@ -82,35 +96,45 @@ describe('AlbionContentRoleService', () => { expect(service).toBeDefined(); }); - describe('getPingsMessage', () => { - it('should fetch the configured message from the configured channel', async () => { - const message = await service.getPingsMessage(); + describe('getPingsMessages', () => { + it('should fetch every configured message from the configured channel', async () => { + const result = await service.getPingsMessages(); expect(discordService.getTextChannel).toHaveBeenCalledWith(TestBootstrapper.mockConfig.discord.channels.albionRoles); - expect(mockChannel.messages.fetch).toHaveBeenCalledWith(TestBootstrapper.mockConfig.albion.contentPingsMessageId); - expect(message).toBe(mockPingsMessage); + expect(mockChannel.messages.fetch).toHaveBeenCalledTimes(2); + expect(result.messages).toEqual([mockPingsMessage, mockGuildPingsMessage]); + expect(result.failed).toEqual([]); }); - it('should return null when no message ID is configured', async () => { + it.each([[[]], [undefined]])('should return nothing when no message IDs are configured (%p)', async (pingsMessageIds) => { await setupModule({ - albion: { ...TestBootstrapper.mockConfig.albion, contentPingsMessageId: undefined }, + albion: { ...TestBootstrapper.mockConfig.albion, pingsMessageIds }, discord: TestBootstrapper.mockConfig.discord, }); - expect(await service.getPingsMessage()).toBeNull(); + const result = await service.getPingsMessages(); + + expect(result.messages).toEqual([]); + expect(result.failed).toEqual([]); expect(discordService.getTextChannel).not.toHaveBeenCalled(); }); - it('should return null when the channel cannot be fetched', async () => { - discordService.getTextChannel.mockRejectedValueOnce(new Error('No channel')); + it('should keep the messages it could read and report the ones it could not', async () => { + mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); - expect(await service.getPingsMessage()).toBeNull(); + const result = await service.getPingsMessages(); + + expect(result.messages).toEqual([mockGuildPingsMessage]); + expect(result.failed).toEqual([TestBootstrapper.mockConfig.albion.pingsMessageIds[0]]); }); - it('should return null when the message cannot be fetched', async () => { - mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + it('should report every message when the channel cannot be fetched', async () => { + discordService.getTextChannel.mockRejectedValue(new Error('No channel')); + + const result = await service.getPingsMessages(); - expect(await service.getPingsMessage()).toBeNull(); + expect(result.messages).toEqual([]); + expect(result.failed).toEqual(TestBootstrapper.mockConfig.albion.pingsMessageIds); }); }); @@ -153,9 +177,9 @@ describe('AlbionContentRoleService', () => { }); }); - describe('getContentRoles', () => { - it('should resolve every content role named in the embed', async () => { - const result = await service.getContentRoles({} as any); + describe('getPingRoles', () => { + it('should resolve every ping role named in the embed', async () => { + const result = await service.getPingRoles({} as any, [mockPingsMessage]); expect(result.roles.size).toBe(3); expect(result.roles.map((role) => role.name).sort()).toEqual([ @@ -166,10 +190,31 @@ describe('AlbionContentRoleService', () => { expect(result.unresolved).toEqual([]); }); - it('should never treat a rank role as a content role, even when the embed names one', async () => { + it('should union the roles named across every pings message', async () => { + const result = await service.getPingRoles({} as any, [mockPingsMessage, mockGuildPingsMessage]); + + expect(result.roles.map((role) => role.name).sort()).toEqual([ + 'ALB/Arena', + 'ALB/CTA', + 'ALB/FactionWarfare', + 'ALB/Gatherer', + 'ALB/Mist', + ]); + expect(result.unresolved).toEqual([]); + }); + + it('should de-duplicate a role named in two different messages', async () => { + mockGuildPingsMessage.embeds[0].description += '\nโ˜๏ธ ALB/Mist - named in both by mistake'; + + const result = await service.getPingRoles({} as any, [mockPingsMessage, mockGuildPingsMessage]); + + expect(result.roles.size).toBe(5); + }); + + it('should never treat a rank role as a ping role, even when the embed names one', async () => { mockPingsMessage.embeds[0].description += '\n๐Ÿ‘‘ ALB/Archmage - the guild leader'; - const result = await service.getContentRoles({} as any); + const result = await service.getPingRoles({} as any, [mockPingsMessage]); expect(result.roles.has(rankRoleId)).toBe(false); expect(result.roles.size).toBe(3); @@ -179,7 +224,7 @@ describe('AlbionContentRoleService', () => { it('should report role names in the embed that do not exist on the server', async () => { mockPingsMessage.embeds[0].description += '\n๐Ÿพ ALB/Tracking - if you want to go hunting'; - const result = await service.getContentRoles({} as any); + const result = await service.getPingRoles({} as any, [mockPingsMessage]); expect(result.unresolved).toEqual(['ALB/Tracking']); expect(result.roles.size).toBe(3); @@ -188,56 +233,70 @@ describe('AlbionContentRoleService', () => { it('should de-duplicate a role named twice in the embed', async () => { mockPingsMessage.embeds[0].description += '\nโ˜๏ธ ALB/Mist - mentioned again by mistake'; - const result = await service.getContentRoles({} as any); + const result = await service.getPingRoles({} as any, [mockPingsMessage]); expect(result.roles.size).toBe(3); }); - it('should return null when the pings message cannot be read', async () => { - mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); - - expect(await service.getContentRoles({} as any)).toBeNull(); + it('should return null when there are no messages to read', async () => { + expect(await service.getPingRoles({} as any, [])).toBeNull(); + expect(discordService.getAllRolesFromGuild).not.toHaveBeenCalled(); }); it('should return null when the embed names no ALB/ roles at all', async () => { mockPingsMessage.embeds[0].description = 'Someone has emptied the message'; - expect(await service.getContentRoles({} as any)).toBeNull(); + expect(await service.getPingRoles({} as any, [mockPingsMessage])).toBeNull(); }); it('should return null when none of the named roles resolve', async () => { discordService.getAllRolesFromGuild.mockResolvedValue(new Collection() as any); - expect(await service.getContentRoles({} as any)).toBeNull(); + expect(await service.getPingRoles({} as any, [mockPingsMessage])).toBeNull(); }); it('should return null when the embed only names rank roles', async () => { mockPingsMessage.embeds[0].description = '๐Ÿ‘‘ ALB/Archmage - the guild leader'; - expect(await service.getContentRoles({} as any)).toBeNull(); + expect(await service.getPingRoles({} as any, [mockPingsMessage])).toBeNull(); }); }); describe('fetchReactors', () => { it('should map each user to the reactions they placed', async () => { const message = TestBootstrapper.getMockContentPingsMessage([ - TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1', '2']), - TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['2']), + TestBootstrapper.getMockPingReaction('โš”๏ธ', ['1', '2']), + TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['2']), ]); - const reactors = await service.fetchReactors(message); + const reactors = await service.fetchReactors([message]); expect(reactors.size).toBe(2); expect(reactors.get('1')).toHaveLength(1); expect(reactors.get('2')).toHaveLength(2); }); + it('should gather a user\'s reactions from every pings message', async () => { + const content = TestBootstrapper.getMockContentPingsMessage([ + TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['1']), + ]); + const guild = TestBootstrapper.getMockGuildPingsMessage([ + TestBootstrapper.getMockPingReaction('๐Ÿ“ฏ', ['1', '2']), + ]); + + const reactors = await service.fetchReactors([content, guild]); + + expect(reactors.size).toBe(2); + expect(reactors.get('1')).toHaveLength(2); + expect(reactors.get('2')).toHaveLength(1); + }); + it('should page past the first 100 users', async () => { const userIds = Array.from({ length: 250 }, (_, index) => `user-${index}`); - const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', userIds); + const reaction = TestBootstrapper.getMockPingReaction('โš”๏ธ', userIds); const message = TestBootstrapper.getMockContentPingsMessage([reaction]); - const reactors = await service.fetchReactors(message); + const reactors = await service.fetchReactors([message]); expect(reactors.size).toBe(250); expect(reaction.users.fetch).toHaveBeenCalledTimes(3); @@ -246,9 +305,9 @@ describe('AlbionContentRoleService', () => { it('should stop paging on an exact multiple of the page size', async () => { const userIds = Array.from({ length: 100 }, (_, index) => `user-${index}`); - const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', userIds); + const reaction = TestBootstrapper.getMockPingReaction('โš”๏ธ', userIds); - const reactors = await service.fetchReactors(TestBootstrapper.getMockContentPingsMessage([reaction])); + const reactors = await service.fetchReactors([TestBootstrapper.getMockContentPingsMessage([reaction])]); expect(reactors.size).toBe(100); // One full page, then one empty page proving there is no more @@ -256,16 +315,16 @@ describe('AlbionContentRoleService', () => { }); it('should ignore bots', async () => { - const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1'], ['bot-1']); + const reaction = TestBootstrapper.getMockPingReaction('โš”๏ธ', ['1'], ['bot-1']); - const reactors = await service.fetchReactors(TestBootstrapper.getMockContentPingsMessage([reaction])); + const reactors = await service.fetchReactors([TestBootstrapper.getMockContentPingsMessage([reaction])]); expect(reactors.size).toBe(1); expect(reactors.has('bot-1')).toBe(false); }); it('should return nothing for a message with no reactions', async () => { - const reactors = await service.fetchReactors(TestBootstrapper.getMockContentPingsMessage()); + const reactors = await service.fetchReactors([TestBootstrapper.getMockContentPingsMessage()]); expect(reactors.size).toBe(0); }); @@ -280,13 +339,13 @@ describe('AlbionContentRoleService', () => { expect(service.isExempt(TestBootstrapper.getMockGuildMemberWithRoles('1', [exemptRoleId]))).toBe(true); }); - it('should be false for a member holding only content roles', () => { - expect(service.isExempt(TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]))).toBe(false); + it('should be false for a member holding only ping roles', () => { + expect(service.isExempt(TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist]))).toBe(false); }); it('should be false when no exempt roles are configured', async () => { await setupModule({ - albion: { ...TestBootstrapper.mockConfig.albion, contentRoleExemptRoles: undefined }, + albion: { ...TestBootstrapper.mockConfig.albion, pingRoleExemptRoles: undefined }, discord: TestBootstrapper.mockConfig.discord, }); @@ -294,41 +353,41 @@ describe('AlbionContentRoleService', () => { }); }); - describe('removeContentRoles', () => { - it('should remove only the content roles the member actually holds', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + describe('removePingRoles', () => { + it('should remove only the ping roles the member actually holds', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist]); - const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), false); + const result = await service.removePingRoles(member, TestBootstrapper.getMockPingRoleCollection(), false); expect(result.removed).toEqual(['ALB/Mist']); expect(member.roles.remove).toHaveBeenCalledTimes(1); - expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.mist); }); it('should report but not perform removals on a dry run', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist, contentRoleIds.arena]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist, pingRoleIds.arena]); - const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), true); + const result = await service.removePingRoles(member, TestBootstrapper.getMockPingRoleCollection(), true); expect(result.removed).toHaveLength(2); expect(member.roles.remove).not.toHaveBeenCalled(); }); it('should capture an error without abandoning the remaining roles', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.factionWarfare, contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.factionWarfare, pingRoleIds.mist]); member.roles.remove.mockRejectedValueOnce(new Error('Discord says no')); - const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), false); + const result = await service.removePingRoles(member, TestBootstrapper.getMockPingRoleCollection(), false); expect(result.errors).toHaveLength(1); expect(result.errors[0]).toContain('Discord says no'); expect(member.roles.remove).toHaveBeenCalledTimes(2); }); - it('should do nothing for a member with no content roles', async () => { + it('should do nothing for a member with no ping roles', async () => { const member = TestBootstrapper.getMockGuildMemberWithRoles('1', []); - const result = await service.removeContentRoles(member, TestBootstrapper.getMockContentRoleCollection(), false); + const result = await service.removePingRoles(member, TestBootstrapper.getMockPingRoleCollection(), false); expect(result.removed).toEqual([]); expect(member.roles.remove).not.toHaveBeenCalled(); @@ -337,8 +396,8 @@ describe('AlbionContentRoleService', () => { describe('removeReactions', () => { it('should remove the user from each reaction', async () => { - const first = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1']); - const second = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['1']); + const first = TestBootstrapper.getMockPingReaction('โš”๏ธ', ['1']); + const second = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['1']); const result = await service.removeReactions([first, second], '1', false); @@ -348,7 +407,7 @@ describe('AlbionContentRoleService', () => { }); it('should report but not perform removals on a dry run', async () => { - const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1']); + const reaction = TestBootstrapper.getMockPingReaction('โš”๏ธ', ['1']); const result = await service.removeReactions([reaction], '1', true); @@ -357,7 +416,7 @@ describe('AlbionContentRoleService', () => { }); it('should not count a failed removal as removed', async () => { - const reaction = TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1']); + const reaction = TestBootstrapper.getMockPingReaction('โš”๏ธ', ['1']); reaction.users.remove.mockRejectedValueOnce(new Error('Missing Permissions')); const result = await service.removeReactions([reaction], '1', false); @@ -373,20 +432,20 @@ describe('AlbionContentRoleService', () => { beforeEach(() => { responseChannel = { send: jest.fn() }; mockPingsMessage.reactions.cache = new Collection([ - ['0', TestBootstrapper.getMockContentReaction('โš”๏ธ', ['1'])], + ['0', TestBootstrapper.getMockPingReaction('โš”๏ธ', ['1'])], ]); }); it('should strip roles and reactions and report what it did', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist]); const result = await service.stripForDeregistration('1', member, responseChannel); expect(result.rolesRemoved).toEqual(['ALB/Mist']); expect(result.reactionsRemoved).toBe(1); - expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.mist); expect(responseChannel.send).toHaveBeenCalledWith( - expect.stringContaining('Removed 1 content role(s) and 1 content ping reaction(s) from <@1>'), + expect.stringContaining('Removed 1 ping role(s) and 1 ping reaction(s) from <@1>'), ); }); @@ -399,7 +458,7 @@ describe('AlbionContentRoleService', () => { }); it('should leave an exempt member entirely alone', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [exemptRoleId, contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [exemptRoleId, pingRoleIds.mist]); const result = await service.stripForDeregistration('1', member, responseChannel); @@ -408,17 +467,30 @@ describe('AlbionContentRoleService', () => { expect(mockChannel.messages.fetch).not.toHaveBeenCalled(); }); - it('should do nothing when the pings message cannot be read', async () => { - mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + it('should do nothing when no pings message can be read', async () => { + mockChannel.messages.fetch.mockRejectedValue(new Error('Unknown Message')); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist]); expect(await service.stripForDeregistration('1', member, responseChannel)).toBeNull(); expect(member.roles.remove).not.toHaveBeenCalled(); }); - it('should still clear reactions when the content roles cannot be resolved', async () => { + it('should clear reactions from every pings message', async () => { + const guildPingsReaction = TestBootstrapper.getMockPingReaction('๐Ÿ“ฏ', ['1']); + mockGuildPingsMessage.reactions.cache = new Collection([['0', guildPingsReaction]]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.cta]); + + const result = await service.stripForDeregistration('1', member, responseChannel); + + expect(result.reactionsRemoved).toBe(2); + expect(guildPingsReaction.users.remove).toHaveBeenCalledWith('1'); + // A role that only the guild pings embed names still has to come off + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.cta); + }); + + it('should still clear reactions when the ping roles cannot be resolved', async () => { discordService.getAllRolesFromGuild.mockResolvedValue(new Collection() as any); - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist]); const result = await service.stripForDeregistration('1', member, responseChannel); @@ -439,7 +511,7 @@ describe('AlbionContentRoleService', () => { }); it('should report an error and ping the dev', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('1', [pingRoleIds.mist]); member.roles.remove.mockRejectedValueOnce(new Error('Discord says no')); await service.stripForDeregistration('1', member, responseChannel); @@ -450,8 +522,8 @@ describe('AlbionContentRoleService', () => { }); it('should attempt every reaction rather than paging the reactor lists', async () => { - const first = TestBootstrapper.getMockContentReaction('โš”๏ธ', []); - const second = TestBootstrapper.getMockContentReaction('โ˜๏ธ', []); + const first = TestBootstrapper.getMockPingReaction('โš”๏ธ', []); + const second = TestBootstrapper.getMockPingReaction('โ˜๏ธ', []); mockPingsMessage.reactions.cache = new Collection([['0', first], ['1', second]]); await service.stripForDeregistration('1', null, responseChannel); @@ -500,18 +572,44 @@ describe('AlbionContentRoleService', () => { scanMessage = buildScanMessage(); }); - it('should skip and say so when the pings message cannot be read', async () => { - mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + it('should skip and say so when no pings message can be read', async () => { + mockChannel.messages.fetch.mockRejectedValue(new Error('Unknown Message')); expect(await service.reconcile(scanMessage)).toBe(false); - expect(sentMessages().join()).toContain('the content pings message could not be read'); + expect(sentMessages().join()).toContain('no pings messages could be read'); + expect(scanMessage.guild.members.fetch).not.toHaveBeenCalled(); + }); + + it('should carry on but shout when only one pings message can be read', async () => { + mockChannel.messages.fetch.mockRejectedValueOnce(new Error('Unknown Message')); + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [pingRoleIds.cta]); + members.set('leaver', member); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(true); + expect(sentMessages().join()).toContain('1 configured pings message(s) could not be read'); + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.cta); + }); + + it('should strip a role that only the guild pings message names', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [pingRoleIds.gatherer]); + members.set('leaver', member); + const reaction = TestBootstrapper.getMockPingReaction('๐Ÿ‘จโ€๐ŸŒพ', ['leaver']); + mockGuildPingsMessage.reactions.cache = new Collection([['0', reaction]]); + + const actionRequired = await service.reconcile(scanMessage); + + expect(actionRequired).toBe(true); + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.gatherer); + expect(reaction.users.remove).toHaveBeenCalledWith('leaver'); }); - it('should skip and say so when no content roles resolve', async () => { + it('should skip and say so when no ping roles resolve', async () => { discordService.getAllRolesFromGuild.mockResolvedValue(new Collection() as any); expect(await service.reconcile(scanMessage)).toBe(false); - expect(sentMessages().join()).toContain('no content roles could be resolved'); + expect(sentMessages().join()).toContain('no ping roles could be resolved'); expect(scanMessage.guild.members.fetch).not.toHaveBeenCalled(); }); @@ -523,24 +621,24 @@ describe('AlbionContentRoleService', () => { expect(sentMessages().join()).toContain('**ALB/Tracking**'); }); - it('should strip an unregistered member of their content roles and reactions', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [contentRoleIds.mist]); + it('should strip an unregistered member of their ping roles and reactions', async () => { + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [pingRoleIds.mist]); members.set('leaver', member); - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['leaver']); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['leaver']); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); const actionRequired = await service.reconcile(scanMessage); expect(actionRequired).toBe(true); - expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.mist); expect(reaction.users.remove).toHaveBeenCalledWith('leaver'); - expect(sentMessages().join()).toContain('1 member(s) stripped of content roles'); + expect(sentMessages().join()).toContain('1 member(s) stripped of ping roles'); }); it('should leave a registered member alone', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('keeper', [contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('keeper', [pingRoleIds.mist]); members.set('keeper', member); - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['keeper']); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['keeper']); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); mockRepository.find.mockResolvedValue([{ discordId: 'keeper' } as AlbionRegistrationsEntity]); @@ -553,9 +651,9 @@ describe('AlbionContentRoleService', () => { }); it('should leave an unregistered alliance member alone', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('ally', [exemptRoleId, contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('ally', [exemptRoleId, pingRoleIds.mist]); members.set('ally', member); - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['ally']); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['ally']); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); const actionRequired = await service.reconcile(scanMessage); @@ -565,8 +663,8 @@ describe('AlbionContentRoleService', () => { expect(reaction.users.remove).not.toHaveBeenCalled(); }); - it('should ignore bots holding a content role', async () => { - const bot = TestBootstrapper.getMockGuildMemberWithRoles('bot', [contentRoleIds.mist], true); + it('should ignore bots holding a ping role', async () => { + const bot = TestBootstrapper.getMockGuildMemberWithRoles('bot', [pingRoleIds.mist], true); members.set('bot', bot); const actionRequired = await service.reconcile(scanMessage); @@ -576,7 +674,7 @@ describe('AlbionContentRoleService', () => { }); it('should clear the reactions of someone who has left the Discord server', async () => { - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['ghost']); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['ghost']); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); const actionRequired = await service.reconcile(scanMessage); @@ -588,19 +686,19 @@ describe('AlbionContentRoleService', () => { it('should never strip a rank role', async () => { mockPingsMessage.embeds[0].description += '\n๐Ÿ‘‘ ALB/Archmage - the guild leader'; - const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [rankRoleId, contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [rankRoleId, pingRoleIds.mist]); members.set('leaver', member); await service.reconcile(scanMessage); - expect(member.roles.remove).toHaveBeenCalledWith(contentRoleIds.mist); + expect(member.roles.remove).toHaveBeenCalledWith(pingRoleIds.mist); expect(member.roles.remove).not.toHaveBeenCalledWith(rankRoleId); }); it('should change nothing on a dry run but still report', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [pingRoleIds.mist]); members.set('leaver', member); - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['leaver']); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['leaver']); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); const actionRequired = await service.reconcile(scanMessage, true); @@ -613,12 +711,12 @@ describe('AlbionContentRoleService', () => { it('should say so when nothing needs doing', async () => { expect(await service.reconcile(scanMessage)).toBe(false); - expect(sentMessages().join()).toContain('No content role inconsistencies were detected'); + expect(sentMessages().join()).toContain('No ping role inconsistencies were detected'); }); - it('should report a live membership count for every content role', async () => { - members.set('a', TestBootstrapper.getMockGuildMemberWithRoles('a', [contentRoleIds.mist])); - members.set('b', TestBootstrapper.getMockGuildMemberWithRoles('b', [contentRoleIds.mist])); + it('should report a live membership count for every ping role', async () => { + members.set('a', TestBootstrapper.getMockGuildMemberWithRoles('a', [pingRoleIds.mist])); + members.set('b', TestBootstrapper.getMockGuildMemberWithRoles('b', [pingRoleIds.mist])); mockRepository.find.mockResolvedValue([ { discordId: 'a' } as AlbionRegistrationsEntity, { discordId: 'b' } as AlbionRegistrationsEntity, @@ -632,8 +730,8 @@ describe('AlbionContentRoleService', () => { }); it('should count only what survives the sweep', async () => { - members.set('a', TestBootstrapper.getMockGuildMemberWithRoles('a', [contentRoleIds.mist])); - const leaver = TestBootstrapper.getMockGuildMemberWithRoles('b', [contentRoleIds.mist]); + members.set('a', TestBootstrapper.getMockGuildMemberWithRoles('a', [pingRoleIds.mist])); + const leaver = TestBootstrapper.getMockGuildMemberWithRoles('b', [pingRoleIds.mist]); // The sweep removes the role, so the live count must not include them any more leaver.roles.remove.mockImplementation(async () => leaver.roles.cache.has.mockReturnValue(false)); members.set('b', leaver); @@ -646,7 +744,7 @@ describe('AlbionContentRoleService', () => { it('should defer reactions beyond the per-run cap to the next run', async () => { const ghostIds = Array.from({ length: MAX_REACTION_REMOVALS_PER_RUN + 10 }, (_, index) => `ghost-${index}`); - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ghostIds); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ghostIds); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); await service.reconcile(scanMessage); @@ -656,7 +754,7 @@ describe('AlbionContentRoleService', () => { }); it('should report removal errors and ping the dev', async () => { - const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [contentRoleIds.mist]); + const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', [pingRoleIds.mist]); member.roles.remove.mockRejectedValueOnce(new Error('Discord says no')); members.set('leaver', member); @@ -668,7 +766,7 @@ describe('AlbionContentRoleService', () => { describe('progress reporting', () => { const addLeavers = (count: number) => { for (let index = 0; index < count; index++) { - members.set(`leaver-${index}`, TestBootstrapper.getMockGuildMemberWithRoles(`leaver-${index}`, [contentRoleIds.mist])); + members.set(`leaver-${index}`, TestBootstrapper.getMockGuildMemberWithRoles(`leaver-${index}`, [pingRoleIds.mist])); } }; @@ -690,10 +788,10 @@ describe('AlbionContentRoleService', () => { let clock = Date.now(); jest.spyOn(Date, 'now').mockImplementation(() => (clock += PROGRESS_EDIT_INTERVAL_MS)); - await service.reconcile(scanMessage, false, '# Sweeping content roles...'); + await service.reconcile(scanMessage, false, '# Sweeping ping roles...'); const progressEdits = (scanMessage.edit as jest.Mock).mock.calls.map((call) => call[0]); - expect(progressEdits[0]).toContain('# Sweeping content roles... [0/3] (0%)'); + expect(progressEdits[0]).toContain('# Sweeping ping roles... [0/3] (0%)'); // The last edit is the finishing one, so it always reads as complete expect(progressEdits[progressEdits.length - 1]).toContain('[3/3] (100%)'); }); @@ -708,10 +806,10 @@ describe('AlbionContentRoleService', () => { }); }); - it('should sweep a member who reacted but holds no content role', async () => { + it('should sweep a member who reacted but holds no ping role', async () => { const member = TestBootstrapper.getMockGuildMemberWithRoles('leaver', []); members.set('leaver', member); - const reaction = TestBootstrapper.getMockContentReaction('โ˜๏ธ', ['leaver']); + const reaction = TestBootstrapper.getMockPingReaction('โ˜๏ธ', ['leaver']); mockPingsMessage.reactions.cache = new Collection([['0', reaction]]); const actionRequired = await service.reconcile(scanMessage); diff --git a/src/albion/services/albion.content.role.service.ts b/src/albion/services/albion.ping.role.service.ts similarity index 68% rename from src/albion/services/albion.content.role.service.ts rename to src/albion/services/albion.ping.role.service.ts index 69a43cdbf..bb9d9478a 100644 --- a/src/albion/services/albion.content.role.service.ts +++ b/src/albion/services/albion.ping.role.service.ts @@ -19,9 +19,9 @@ import { DiscordService } from '../../discord/discord.service'; import { getChannel } from '../../discord/discord.hacks'; // Every line of the pings embed reads " ALB/Name - description". Only the role token is -// load bearing: anyone who shouldn't be there loses every content role and every reaction they +// load bearing: anyone who shouldn't be there loses every ping role and every reaction they // hold, so nothing needs to know which emoji belongs to which role. -export const CONTENT_ROLE_TOKEN = /\bALB\/[A-Za-z0-9_-]+/g; +export const PING_ROLE_TOKEN = /\bALB\/[A-Za-z0-9_-]+/g; // reaction.users.fetch() returns 100 at most, so a popular ping needs paging or everyone past // the first page is invisible to the sweep. The page cap is a runaway guard, nothing more. @@ -35,14 +35,21 @@ export const MAX_REACTION_REMOVALS_PER_RUN = 500; // Progress edits cost an API call of their own, on the tightest bucket in the sweep export const PROGRESS_EDIT_INTERVAL_MS = 2000; -export interface ContentRoleSet { +export interface PingRoleSet { roles: Collection; // Role names in the embed that match no Discord role. Reported rather than dropped, since // it means the embed and the server have drifted apart. unresolved: string[]; } -export interface ContentRoleStripResult { +export interface PingsMessageSet { + messages: Message[]; + // IDs that are configured but could not be fetched. Reported rather than fatal: a message the + // sweep can't see means less gets cleaned up, never that the wrong person gets stripped. + failed: string[]; +} + +export interface PingRoleStripResult { discordId: string; rolesRemoved: string[]; reactionsRemoved: number; @@ -50,8 +57,8 @@ export interface ContentRoleStripResult { } @Injectable() -export class AlbionContentRoleService { - private readonly logger = new Logger(AlbionContentRoleService.name); +export class AlbionPingRoleService { + private readonly logger = new Logger(AlbionPingRoleService.name); constructor( private readonly config: ConfigService, @@ -59,19 +66,18 @@ export class AlbionContentRoleService { @InjectRepository(AlbionRegistrationsEntity) private readonly albionRegistrationsRepository: EntityRepository, ) {} - // Null means "content roles could not be established". Callers must treat it as do-nothing: + // Null means "ping roles could not be established". Callers must treat it as do-nothing: // a failed fetch reading as an empty role list would strip the entire server. - async getContentRoles(guild: Guild): Promise { - const message = await this.getPingsMessage(); - - if (!message) { + async getPingRoles(guild: Guild, messages: Message[]): Promise { + if (messages.length === 0) { return null; } - const tokens = [...new Set(this.extractText(message).match(CONTENT_ROLE_TOKEN) ?? [])]; + const text = messages.map((message) => this.extractText(message)).join('\n'); + const tokens = [...new Set(text.match(PING_ROLE_TOKEN) ?? [])]; if (tokens.length === 0) { - this.logger.error('Content Roles: No ALB/ role names found in the content pings message!'); + this.logger.error('Ping Roles: No ALB/ role names found in the pings messages!'); return null; } @@ -94,7 +100,7 @@ export class AlbionContentRoleService { // Rank roles share the ALB/ prefix. Excluded by ID whatever the embed happens to say, // so a stray mention of a rank in the text can never strip someone of their rank. if (rankRoleIds.has(role.id)) { - this.logger.warn(`Content Roles: Ignoring rank role "${role.name}" found in the pings message.`); + this.logger.warn(`Ping Roles: Ignoring rank role "${role.name}" found in the pings message.`); continue; } @@ -102,32 +108,37 @@ export class AlbionContentRoleService { } if (roles.size === 0) { - this.logger.error('Content Roles: None of the role names in the pings message resolved to a Discord role!'); + this.logger.error('Ping Roles: None of the role names in the pings messages resolved to a Discord role!'); return null; } return { roles, unresolved }; } - // The message is fetched fresh every time. It is edited by hand whenever a content role is - // added, and a cached copy would keep the sweep working off the old list. - async getPingsMessage(): Promise { - const messageId = this.config.get('albion.contentPingsMessageId'); + // Fetched fresh every time. The embeds are edited by hand whenever a ping role is added, and + // a cached copy would keep the sweep working off the old list. + async getPingsMessages(): Promise { + const messageIds: string[] = this.config.get('albion.pingsMessageIds') ?? []; const channelId = this.config.get('discord.channels.albionRoles'); + const result: PingsMessageSet = { messages: [], failed: [] }; - if (!messageId) { - this.logger.warn('Content Roles: No content pings message ID is configured, skipping.'); - return null; + if (messageIds.length === 0) { + this.logger.warn('Ping Roles: No pings message IDs are configured, skipping.'); + return result; } - try { - const channel = await this.discordService.getTextChannel(channelId); - return await channel.messages.fetch(messageId); - } - catch (err) { - this.logger.error(`Content Roles: Failed to fetch the content pings message ${messageId}! Err: ${err.message}`); - return null; + for (const messageId of messageIds) { + try { + const channel = await this.discordService.getTextChannel(channelId); + result.messages.push(await channel.messages.fetch(messageId)); + } + catch (err) { + result.failed.push(messageId); + this.logger.error(`Ping Roles: Failed to fetch pings message ${messageId}! Err: ${err.message}`); + } } + + return result; } extractText(message: Message): string { @@ -144,12 +155,12 @@ export class AlbionContentRoleService { return parts.join('\n'); } - // Who has reacted to the pings message, by Discord ID. Users who have left the server still + // Who has reacted to the pings messages, by Discord ID. Users who have left the server still // appear here - Discord never prunes reactions - which is exactly the drift being cleaned up. - async fetchReactors(message: Message): Promise> { + async fetchReactors(messages: Message[]): Promise> { const reactors = new Map(); - for (const reaction of message.reactions.cache.values()) { + for (const reaction of this.allReactions(messages)) { let after: Snowflake | undefined; for (let page = 0; page < MAX_REACTION_PAGES; page++) { @@ -177,25 +188,31 @@ export class AlbionContentRoleService { return reactors; } + // Every reaction across every pings message, flattened. The same emoji on two messages is two + // distinct reactions, so they can't be deduplicated by emoji. + allReactions(messages: Message[]): MessageReaction[] { + return messages.flatMap((message) => [...message.reactions.cache.values()]); + } + isExempt(member: GuildMember | null): boolean { if (!member) { return false; } - const exemptRoles: string[] = this.config.get('albion.contentRoleExemptRoles') ?? []; + const exemptRoles: string[] = this.config.get('albion.pingRoleExemptRoles') ?? []; return exemptRoles.some((roleId) => member.roles.cache.has(roleId)); } - async removeContentRoles( + async removePingRoles( member: GuildMember, - contentRoles: Collection, + pingRoles: Collection, dryRun: boolean, ): Promise<{ removed: string[], errors: string[] }> { const removed: string[] = []; const errors: string[] = []; - for (const role of contentRoles.values()) { + for (const role of pingRoles.values()) { if (!member.roles.cache.has(role.id)) { continue; } @@ -211,7 +228,7 @@ export class AlbionContentRoleService { } catch (err) { errors.push(`Failed to remove role "${role.name}" from <@${member.id}>: ${err.message}`); - this.logger.error(`Content Roles: Failed to remove role ${role.name} from ${member.id}! Err: ${err.message}`); + this.logger.error(`Ping Roles: Failed to remove role ${role.name} from ${member.id}! Err: ${err.message}`); } } @@ -241,7 +258,7 @@ export class AlbionContentRoleService { catch (err) { removed--; errors.push(`Failed to remove reaction ${reaction.emoji.name} from <@${discordId}>: ${err.message}`); - this.logger.error(`Content Roles: Failed to remove reaction from ${discordId}! Err: ${err.message}`); + this.logger.error(`Ping Roles: Failed to remove reaction from ${discordId}! Err: ${err.message}`); } } @@ -255,21 +272,21 @@ export class AlbionContentRoleService { discordId: Snowflake, discordMember: GuildMember | null, responseChannel: GuildTextBasedChannel, - ): Promise { - // Alliance members hold content roles without ever registering. Exempt uniformly rather + ): Promise { + // Alliance members hold ping roles without ever registering. Exempt uniformly rather // than only in the sweep, so leaving the guild but staying in the alliance keeps the pings. if (this.isExempt(discordMember)) { - this.logger.log(`Content Roles: ${discordId} holds an exempt role, leaving their content roles alone.`); + this.logger.log(`Ping Roles: ${discordId} holds an exempt role, leaving their ping roles alone.`); return null; } - const pingsMessage = await this.getPingsMessage(); + const { messages } = await this.getPingsMessages(); - if (!pingsMessage) { + if (messages.length === 0) { return null; } - const result: ContentRoleStripResult = { + const result: PingRoleStripResult = { discordId, rolesRemoved: [], reactionsRemoved: 0, @@ -277,19 +294,19 @@ export class AlbionContentRoleService { }; if (discordMember) { - const contentRoles = await this.getContentRoles(discordMember.guild); + const pingRoles = await this.getPingRoles(discordMember.guild, messages); - if (contentRoles) { - const roleResult = await this.removeContentRoles(discordMember, contentRoles.roles, false); + if (pingRoles) { + const roleResult = await this.removePingRoles(discordMember, pingRoles.roles, false); result.rolesRemoved = roleResult.removed; result.errors.push(...roleResult.errors); } } - // Every reaction on the message is attempted rather than paging the reactor lists first: + // Every reaction on every message is attempted rather than paging the reactor lists first: // deregistrations are rare and one-by-one, and the delete is a no-op if they never reacted. const reactionResult = await this.removeReactions( - [...pingsMessage.reactions.cache.values()], + this.allReactions(messages), discordId, false, ); @@ -298,7 +315,7 @@ export class AlbionContentRoleService { if (result.rolesRemoved.length > 0 || result.reactionsRemoved > 0) { await responseChannel.send( - `${ALBION_GUILD_EMOJI} ๐Ÿงน Removed ${result.rolesRemoved.length} content role(s) and ${result.reactionsRemoved} content ping reaction(s) from <@${discordId}>.`, + `${ALBION_GUILD_EMOJI} ๐Ÿงน Removed ${result.rolesRemoved.length} ping role(s) and ${result.reactionsRemoved} ping reaction(s) from <@${discordId}>.`, ); } @@ -314,36 +331,44 @@ export class AlbionContentRoleService { async reconcile( scanMessage: Message, dryRun = false, - heading = `# ${ALBION_GUILD_EMOJI} Sweeping content roles...`, + heading = `# ${ALBION_GUILD_EMOJI} Sweeping ping roles...`, ): Promise { const emoji = ALBION_GUILD_EMOJI; const guild = scanMessage.guild; - const pingsMessage = await this.getPingsMessage(); + const { messages, failed } = await this.getPingsMessages(); - if (!pingsMessage) { - await getChannel(scanMessage).send(`${emoji} โš ๏ธ Content role sweep skipped: the content pings message could not be read.`); + if (messages.length === 0) { + await getChannel(scanMessage).send(`${emoji} โš ๏ธ Ping role sweep skipped: no pings messages could be read.`); return false; } - const contentRoleSet = await this.getContentRoles(guild); + // One unreadable message out of several only means less gets swept, so the run continues - + // but silently sweeping half the pings is how this stops being noticed, hence the shout. + if (failed.length > 0) { + await getChannel(scanMessage).send( + `${emoji} โš ๏ธ ${failed.length} configured pings message(s) could not be read, so their roles and reactions were skipped: **${failed.join('**, **')}**.`, + ); + } + + const pingRoleSet = await this.getPingRoles(guild, messages); - if (!contentRoleSet) { - await getChannel(scanMessage).send(`${emoji} โš ๏ธ Content role sweep skipped: no content roles could be resolved from the pings message.`); + if (!pingRoleSet) { + await getChannel(scanMessage).send(`${emoji} โš ๏ธ Ping role sweep skipped: no ping roles could be resolved from the pings messages.`); return false; } - const { roles: contentRoles, unresolved } = contentRoleSet; + const { roles: pingRoles, unresolved } = pingRoleSet; if (unresolved.length > 0) { await getChannel(scanMessage).send( - `${emoji} โš ๏ธ The content pings message names ${unresolved.length} role(s) that don't exist on the server: **${unresolved.join('**, **')}**. Fix the message or the role names.`, + `${emoji} โš ๏ธ The pings messages name ${unresolved.length} role(s) that don't exist on the server: **${unresolved.join('**, **')}**. Fix the message or the role names.`, ); } // Full member list rather than role.members, which only reflects whatever happens to be // cached and quietly undercounts. const members = await guild.members.fetch(); - const reactors = await this.fetchReactors(pingsMessage); + const reactors = await this.fetchReactors(messages); const registeredIds = new Set( (await this.albionRegistrationsRepository.find({ guildId: this.config.get('albion.guildId') })) @@ -354,7 +379,7 @@ export class AlbionContentRoleService { // Bots are dropped in the candidate loop below, which is the one place both sources meet for (const [discordId, member] of members) { - if (contentRoles.some((role) => member.roles.cache.has(role.id))) { + if (pingRoles.some((role) => member.roles.cache.has(role.id))) { candidates.add(discordId); } } @@ -386,7 +411,7 @@ export class AlbionContentRoleService { } const rolesRemoved = member - ? await this.removeContentRoles(member, contentRoles, dryRun) + ? await this.removePingRoles(member, pingRoles, dryRun) : { removed: [], errors: [] }; errors.push(...rolesRemoved.errors); @@ -409,13 +434,13 @@ export class AlbionContentRoleService { const status = member ? 'is not registered' : 'has left the Discord server'; changes.push( - `- ${emoji} ๐Ÿงน${dryRunText} <@${discordId}> ${status}: removed ${rolesRemoved.removed.length} content role(s) and ${reactionCount} reaction(s).`, + `- ${emoji} ๐Ÿงน${dryRunText} <@${discordId}> ${status}: removed ${rolesRemoved.removed.length} ping role(s) and ${reactionCount} reaction(s).`, ); } await progress.finish(processed, reactionsRemoved); - await this.report(scanMessage, changes, errors, capped, contentRoles, members, dryRun); + await this.report(scanMessage, changes, errors, capped, pingRoles, members, dryRun); return changes.length > 0; } @@ -434,7 +459,7 @@ export class AlbionContentRoleService { } catch (err) { // A failed progress edit must never take the sweep down with it - this.logger.warn(`Content Roles: Failed to update sweep progress. Err: ${err.message}`); + this.logger.warn(`Ping Roles: Failed to update sweep progress. Err: ${err.message}`); } }; @@ -462,7 +487,7 @@ export class AlbionContentRoleService { changes: string[], errors: string[], capped: number, - contentRoles: Collection, + pingRoles: Collection, members: Collection, dryRun: boolean, ): Promise { @@ -470,10 +495,10 @@ export class AlbionContentRoleService { const dryRunText = dryRun ? ' (DRY RUN)' : ''; if (changes.length === 0) { - await getChannel(scanMessage).send(`${emoji} โœ… No content role inconsistencies were detected.`); + await getChannel(scanMessage).send(`${emoji} โœ… No ping role inconsistencies were detected.`); } else { - await getChannel(scanMessage).send(`## ${emoji} ๐Ÿงน${dryRunText} ${changes.length} member(s) stripped of content roles!`); + await getChannel(scanMessage).send(`## ${emoji} ๐Ÿงน${dryRunText} ${changes.length} member(s) stripped of ping roles!`); for (const change of changes) { const lineMessage = await getChannel(scanMessage).send('.'); @@ -492,10 +517,10 @@ export class AlbionContentRoleService { } // The whole point of the sweep: the counts on the roles are now a live membership count. - const counts = contentRoles + const counts = pingRoles .map((role) => `- **${role.name}**: ${members.filter((member) => member.roles.cache.has(role.id)).size}`) .join('\n'); - await getChannel(scanMessage).send(`### ${emoji} Content role membership\n${counts}`); + await getChannel(scanMessage).send(`### ${emoji} Ping role membership\n${counts}`); } } diff --git a/src/albion/services/albion.scanning.service.spec.ts b/src/albion/services/albion.scanning.service.spec.ts index 850a74361..81387695f 100644 --- a/src/albion/services/albion.scanning.service.spec.ts +++ b/src/albion/services/albion.scanning.service.spec.ts @@ -11,7 +11,7 @@ import { AlbionApiService } from './albion.api.service'; import { AlbionUtilities } from '../utilities/albion.utilities'; import { TestBootstrapper } from '../../test.bootstrapper'; import { AlbionDeregistrationService } from './albion.deregistration.service'; -import { AlbionContentRoleService } from './albion.content.role.service'; +import { AlbionPingRoleService } from './albion.ping.role.service'; const mockDevUserId = TestBootstrapper.mockConfig.discord.devUserId; const mockScanUserId = '1337'; @@ -34,7 +34,7 @@ describe('AlbionScanningService', () => { let service: AlbionScanningService; let albionApiService: AlbionApiService; let albionDeregistrationService: AlbionDeregistrationService; - let albionContentRoleService: jest.Mocked; + let albionPingRoleService: jest.Mocked; let mockDiscordUser: any; let mockDiscordMessage: any; @@ -86,7 +86,7 @@ describe('AlbionScanningService', () => { }, }, { - provide: AlbionContentRoleService, + provide: AlbionPingRoleService, useValue: { reconcile: jest.fn().mockResolvedValue(false), }, @@ -101,7 +101,7 @@ describe('AlbionScanningService', () => { service = moduleRef.get(AlbionScanningService); albionApiService = moduleRef.get(AlbionApiService); albionDeregistrationService = moduleRef.get(AlbionDeregistrationService); - albionContentRoleService = moduleRef.get(AlbionContentRoleService) as any; + albionPingRoleService = moduleRef.get(AlbionPingRoleService) as any; const albionMerged = { ...TestBootstrapper.mockConfig.albion, @@ -233,7 +233,7 @@ describe('AlbionScanningService', () => { expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [2/5] Checking 1 characters for membership status...'); expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [3/5] Performing reverse role scan...'); expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [4/5] Checking for role inconsistencies...'); - expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [5/5] Sweeping content roles...'); + expect(mockDiscordMessage.edit).toHaveBeenCalledWith('# ๐Ÿ‡ช๐Ÿ‡บ Task: [5/5] Sweeping ping roles...'); expect(mockDiscordMessage.channel.send).toHaveBeenCalledWith('## ๐Ÿ‡ช๐Ÿ‡บ Scan complete!'); expect(mockDiscordMessage.channel.send).toHaveBeenCalledWith('------------------------------------------'); expect(mockDiscordMessage.delete).toHaveBeenCalled(); @@ -253,10 +253,10 @@ describe('AlbionScanningService', () => { expect(service.removeLeavers).toHaveBeenCalledTimes(1); expect(service.reverseRoleScan).toHaveBeenCalledTimes(1); expect(service.roleInconsistencies).toHaveBeenCalledTimes(1); - expect(albionContentRoleService.reconcile).toHaveBeenCalledTimes(1); + expect(albionPingRoleService.reconcile).toHaveBeenCalledTimes(1); }); - it('should pass the dry run flag through to the content role sweep', async () => { + it('should pass the dry run flag through to the ping role sweep', async () => { mockAlbionRegistrationsRepository.find = jest.fn().mockResolvedValueOnce([mockRegisteredMember]); service.gatherCharacters = jest.fn().mockResolvedValueOnce([mockCharacter]); service.removeLeavers = jest.fn().mockResolvedValueOnce([]); @@ -265,10 +265,10 @@ describe('AlbionScanningService', () => { await service.startScan(mockDiscordMessage, true); - expect(albionContentRoleService.reconcile).toHaveBeenCalledWith( + expect(albionPingRoleService.reconcile).toHaveBeenCalledWith( mockDiscordMessage, true, - expect.stringContaining('Sweeping content roles'), + expect.stringContaining('Sweeping ping roles'), ); }); diff --git a/src/albion/services/albion.scanning.service.ts b/src/albion/services/albion.scanning.service.ts index 873db7cac..7a91525b2 100644 --- a/src/albion/services/albion.scanning.service.ts +++ b/src/albion/services/albion.scanning.service.ts @@ -10,7 +10,7 @@ import { AlbionRoleMapInterface } from '../../config/albion.app.config'; import { AlbionUtilities } from '../utilities/albion.utilities'; import { getChannel } from '../../discord/discord.hacks'; import { AlbionDeregistrationService } from './albion.deregistration.service'; -import { AlbionContentRoleService } from './albion.content.role.service'; +import { AlbionPingRoleService } from './albion.ping.role.service'; export interface RoleInconsistencyResult { id: string, @@ -28,7 +28,7 @@ export class AlbionScanningService { private readonly albionApiService: AlbionApiService, private readonly config: ConfigService, private readonly albionDeregistrationService: AlbionDeregistrationService, - private readonly albionContentRoleService: AlbionContentRoleService, + private readonly albionPingRoleService: AlbionPingRoleService, private readonly albionUtilities: AlbionUtilities, @InjectRepository(AlbionRegistrationsEntity) private readonly albionRegistrationsRepository: EntityRepository, ) {} @@ -85,9 +85,9 @@ export class AlbionScanningService { } // Runs after the leaver pass so it sees the registrations that pass has just removed - const sweepHeading = `# ${emoji} Task: [5/5] Sweeping content roles...`; + const sweepHeading = `# ${emoji} Task: [5/5] Sweeping ping roles...`; await message.edit(sweepHeading); - await this.albionContentRoleService.reconcile(message, dryRun, sweepHeading); + await this.albionPingRoleService.reconcile(message, dryRun, sweepHeading); } catch (err) { await message.edit(`## ${emoji} โŒ An error occurred while scanning!`); diff --git a/src/config/albion.app.config.ts b/src/config/albion.app.config.ts index f15cbb6ae..e392e5312 100644 --- a/src/config/albion.app.config.ts +++ b/src/config/albion.app.config.ts @@ -119,13 +119,20 @@ const leadershipPing = isProduction // Filtered on priority rather than name because production spells tier 3 "EldritchMager". const ELECTOR_MAX_PRIORITY = 3; -// Roles that keep their content roles without a registration, e.g. the alliance role. Alliance +// Roles that keep their ping roles without a registration, e.g. the alliance role. Alliance // members use the pings but never register with DIG, so the sweep would otherwise strip them. -const contentRoleExemptRoles = (process.env.ALBION_CONTENT_ROLE_EXEMPT_ROLES ?? '') +const pingRoleExemptRoles = (process.env.ALBION_PING_ROLE_EXEMPT_ROLES ?? '') .split(',') .map((roleId) => roleId.trim()) .filter((roleId) => roleId.length > 0); +// There is one embed per ping category โ€” content pings and guild pings โ€” and more can be added +// without a deploy, so this is a list rather than a single ID. +const pingsMessageIds = (process.env.MESSAGE_ALBION_PINGS ?? '') + .split(',') + .map((messageId) => messageId.trim()) + .filter((messageId) => messageId.length > 0); + export default () => ({ guildId: '0_zTfLfASD2Wtw6Tc-yckA', roleMap, @@ -136,10 +143,10 @@ export default () => ({ // soft-leadership, so a human makes that call even after a successful vote. autoAssignRanks: ['@ALB/Graduate'], gameActivityName: 'Albion Online', // As Discord reports it in presence data - // The content pings embed is the source of truth for which ALB/ roles are content roles. + // The pings embeds are the source of truth for which ALB/ roles are ping roles. // Rank roles share the prefix, so the name alone can't tell the two apart. - contentPingsMessageId: process.env.MESSAGE_ALBION_CONTENT_PINGS, - contentRoleExemptRoles, + pingsMessageIds, + pingRoleExemptRoles, scanExcludedUsers: [], // Discord IDs guildLeaderRole: findRole('@ALB/Archmage'), guildOfficerRole: findRole('@ALB/Magister'), diff --git a/src/test.bootstrapper.ts b/src/test.bootstrapper.ts index 5713711bf..57bacb394 100644 --- a/src/test.bootstrapper.ts +++ b/src/test.bootstrapper.ts @@ -310,19 +310,24 @@ export class TestBootstrapper { } as any; } - // Content role IDs and the embed that names them. The pings message is the only source of - // truth for which ALB/ roles are content roles, so most content role tests start here. - static readonly mockContentRoleIds = { + // Ping role IDs and the embeds that name them. The pings messages are the only source of + // truth for which ALB/ roles are ping roles, so most ping role tests start here. Two embeds, + // as in production: content pings and guild pings. + static readonly mockPingRoleIds = { factionWarfare: '9000000000000001', mist: '9000000000000002', arena: '9000000000000003', + cta: '9000000000000004', + gatherer: '9000000000000005', }; - static getMockContentRoleCollection() { + static getMockPingRoleCollection() { return new Collection() - .set(this.mockContentRoleIds.factionWarfare, { id: this.mockContentRoleIds.factionWarfare, name: 'ALB/FactionWarfare' } as Role) - .set(this.mockContentRoleIds.mist, { id: this.mockContentRoleIds.mist, name: 'ALB/Mist' } as Role) - .set(this.mockContentRoleIds.arena, { id: this.mockContentRoleIds.arena, name: 'ALB/Arena' } as Role); + .set(this.mockPingRoleIds.factionWarfare, { id: this.mockPingRoleIds.factionWarfare, name: 'ALB/FactionWarfare' } as Role) + .set(this.mockPingRoleIds.mist, { id: this.mockPingRoleIds.mist, name: 'ALB/Mist' } as Role) + .set(this.mockPingRoleIds.arena, { id: this.mockPingRoleIds.arena, name: 'ALB/Arena' } as Role) + .set(this.mockPingRoleIds.cta, { id: this.mockPingRoleIds.cta, name: 'ALB/CTA' } as Role) + .set(this.mockPingRoleIds.gatherer, { id: this.mockPingRoleIds.gatherer, name: 'ALB/Gatherer' } as Role); } static getMockContentPingsEmbedDescription() { @@ -335,9 +340,18 @@ export class TestBootstrapper { ].join('\n'); } + static getMockGuildPingsEmbedDescription() { + return [ + 'Proclamations, Event and the CTA ping can only be used by leadership members.', + '**Ping List**', + '๐Ÿ“ฏ ALB/CTA - Call To Arms for instant action', + '๐Ÿ‘จโ€๐ŸŒพ ALB/Gatherer - to be pinged for gathering requests', + ].join('\n'); + } + // A reaction whose users.fetch() honours Discord's paging contract, so a test can prove the // sweep pages past the first 100 users. - static getMockContentReaction(emojiName: string, userIds: string[], botIds: string[] = []) { + static getMockPingReaction(emojiName: string, userIds: string[], botIds: string[] = []) { const allIds = [...userIds, ...botIds]; return { emoji: { name: emojiName, id: null }, @@ -356,13 +370,31 @@ export class TestBootstrapper { } static getMockContentPingsMessage(reactions: any[] = []) { + return this.getMockPingsMessage( + this.mockConfig.albion.pingsMessageIds[0], + 'Albion Content Pings', + this.getMockContentPingsEmbedDescription(), + reactions, + ); + } + + static getMockGuildPingsMessage(reactions: any[] = []) { + return this.getMockPingsMessage( + this.mockConfig.albion.pingsMessageIds[1], + 'Albion Guild Pings', + this.getMockGuildPingsEmbedDescription(), + reactions, + ); + } + + private static getMockPingsMessage(id: string, title: string, description: string, reactions: any[]) { return { - id: this.mockConfig.albion.contentPingsMessageId, + id, content: '', embeds: [ { - title: 'Albion Content Pings', - description: this.getMockContentPingsEmbedDescription(), + title, + description, fields: [], }, ], @@ -534,8 +566,8 @@ export class TestBootstrapper { electorMaxPriority: 3, autoAssignRanks: ['@ALB/Graduate'], gameActivityName: 'Albion Online', - contentPingsMessageId: '1401401401401401401', - contentRoleExemptRoles: ['7070707070707070'], + pingsMessageIds: ['1401401401401401401', '1402402402402402402'], + pingRoleExemptRoles: ['7070707070707070'], }, discord: { guildId: '657687978899',