Fix bulk deletion with very-large queues (#63)

* Reword responses to some informational commands

* Don't claim we cleared the queue if we failed to clear the queue

* Respect Discord's bulk-delete limits

* v1.3.2

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "gamgee",
-	"version": "1.3.1",
+	"version": "1.3.2",
 	"description": "A Discord bot for managing a song request queue.",
 	"private": true,
 	"scripts": {
@@ -47,6 +47,7 @@
 		"discord.js": "^13.1.0",
 		"dotenv": "^8.2.0",
 		"humanize-duration": "^3.25.1",
+		"lodash": "^4.17.21",
 		"node-fetch": "^2.6.6",
 		"node-persist": "^3.1.0",
 		"reflect-metadata": "^0.1.13",
@@ -62,6 +63,7 @@
 	"devDependencies": {
 		"@types/humanize-duration": "^3.18.1",
 		"@types/jest": "^27.4.0",
+		"@types/lodash": "^4.14.179",
 		"@types/node": "^16.6.2",
 		"@types/node-fetch": "^2.5.12",
 		"@types/node-persist": "^3.1.1",

src/actions/messages/deleteMessage.test.ts

@@ -0,0 +1,64 @@
+import type Discord from "discord.js";
+import { bulkDeleteMessagesWithIds } from "./deleteMessage.js";
+
+const mockBulkDelete = jest.fn();
+const mockSingleDelete = jest.fn();
+
+describe("Bulk Message Delete", () => {
+	let channel: Discord.TextChannel;
+
+	beforeEach(() => {
+		channel = ({
+			bulkDelete: mockBulkDelete,
+			messages: {
+				delete: mockSingleDelete
+			}
+		} as unknown) as Discord.TextChannel;
+
+		mockBulkDelete.mockImplementation((ids: Array<string>) => {
+			// Discord balks when we try <2 || >100 IDs
+			if (ids.length < 2 || ids.length > 100) {
+				return Promise.reject(new Error("Can't delete fewer than 2 or more than 100"));
+			}
+			return Promise.resolve();
+		});
+	});
+
+	test.each`
+		count  | multiples | singles
+		${2}   | ${1}      | ${0}
+		${3}   | ${1}      | ${0}
+		${45}  | ${1}      | ${0}
+		${99}  | ${1}      | ${0}
+		${100} | ${1}      | ${0}
+		${101} | ${1}      | ${1}
+		${800} | ${8}      | ${0}
+	`(
+		"Clears when the queue has $count items in it",
+		async (params: { count: number; multiples: number; singles: number }) => {
+			const { count, multiples, singles } = params;
+			// Use an array of `count` instances of "a"
+			const messageIds = new Array<string>(count).fill("a");
+			await expect(bulkDeleteMessagesWithIds(messageIds, channel)).resolves.toBeTrue();
+
+			expect(mockBulkDelete).toHaveBeenCalledTimes(multiples);
+			expect(mockSingleDelete).toHaveBeenCalledTimes(singles);
+		}
+	);
+
+	test("Deletes one message individually", async () => {
+		const messageIds: Array<string> = ["a"];
+		await expect(bulkDeleteMessagesWithIds(messageIds, channel)).resolves.toBeTrue();
+
+		expect(mockBulkDelete).not.toHaveBeenCalled();
+		expect(mockSingleDelete).toHaveBeenCalledTimes(1);
+		expect(mockSingleDelete).toHaveBeenCalledWith("a");
+	});
+
+	test("Does nothing when the queue has 0 items in it", async () => {
+		const messageIds = new Array<string>();
+		await expect(bulkDeleteMessagesWithIds(messageIds, channel)).resolves.toBeTrue();
+
+		expect(mockBulkDelete).not.toHaveBeenCalled();
+	});
+});

src/actions/messages/deleteMessage.ts

@@ -1,5 +1,6 @@
 import type Discord from "discord.js";
 import type { Snowflake } from "discord.js";
+import chunk from "lodash/chunk.js";
 import isError from "../../helpers/isError.js";
 import richErrorMessage from "../../helpers/richErrorMessage.js";
 import { useLogger } from "../../logger.js";
@@ -68,8 +69,20 @@ export async function bulkDeleteMessagesWithIds(
 	messageIds: Array<Snowflake>,
 	channel: Discord.TextChannel
 ): Promise<boolean> {
+	if (messageIds.length === 0) return true;
+
 	try {
-		await channel.bulkDelete(messageIds);
+		// Batch up to 100 IDs at a time
+		for (const ids of chunk(messageIds, 100)) {
+			if (ids.length === 1) {
+				// Given one ID, just delete individually
+				const messageId = ids[0] as string;
+				await channel.messages.delete(messageId);
+			} else {
+				await channel.bulkDelete(ids);
+			}
+		}
+
 		return true;
 	} catch (error: unknown) {
 		if (isError(error) && error.code === "50034") {

src/commands/languages.ts

@@ -51,7 +51,7 @@ const languages: GlobalCommand = {
 
 		const stats = Object.entries(languages).map(([languageName, languageUse]) => {
 			const use = (languageUse ?? 0) / totalUse;
-			return `${languageName} (${(use * 100).toFixed(1)}%)`;
+			return `${(use * 100).toFixed(1)}% ${languageName}`;
 		});
 
 		logger.debug(`Gamgee is made of ${totalLanguages} languages: ${stats.join(", ")}`);

src/commands/queue/restart.test.ts

@@ -37,6 +37,7 @@ describe("Clear queue contents", () => {
 
 		mockGetAllEntries.mockResolvedValue([]);
 		mockQueueClear.mockResolvedValue(undefined);
+		mockBulkDeleteMessagesWithIds.mockResolvedValue(true);
 	});
 
 	test("does nothing when no queue is set up", async () => {
@@ -87,4 +88,12 @@ describe("Clear queue contents", () => {
 			expect(mockQueueClear).toHaveBeenCalledTimes(1);
 		}
 	);
+
+	test("Does not clear the database when clearing messages fails", async () => {
+		mockBulkDeleteMessagesWithIds.mockResolvedValueOnce(false); // something goes wrong
+
+		await expect(restart.execute(context)).resolves.toBeUndefined(); // don't throw
+
+		expect(mockQueueClear).not.toHaveBeenCalled(); // don't clear
+	});
 });

src/commands/queue/restart.ts

@@ -19,7 +19,10 @@ const restart: Subcommand = {
 		await prepareForLongRunningTasks();
 
 		const toBeDeleted = (await fetchAllEntries(queueChannel)).map(entry => entry.queueMessageId);
-		await bulkDeleteMessagesWithIds(toBeDeleted, queueChannel);
+		const didDelete = await bulkDeleteMessagesWithIds(toBeDeleted, queueChannel);
+		if (!didDelete) {
+			return reply("Something went wrong. I couldn't get that queue cleared, sorry.");
+		}
 		await clearEntries(queueChannel);
 
 		return reply("The queue has restarted.");

src/commands/version.ts

@@ -18,9 +18,11 @@ const version: Command = {
 
 		if (type === "interaction") {
 			// Discord lets bots link stuff in Markdown syntax, but it'll also embed by default. Use `<brackets>` to prevent the embed.
-			const repo = `https://github.com/AverageHelper/Gamgee/tree/v${gamgeeVersion}`;
+			const repo = "https://github.com/AverageHelper/Gamgee";
+			const source = `${repo}/tree/v${gamgeeVersion}`;
+			const changelog = `${repo}/releases/tag/v${gamgeeVersion}`;
 			return reply(
-				`I'm currently running Gamgee Core [v${gamgeeVersion}](<${repo}>)  ${celebration}`
+				`I'm currently running [Gamgee Core v${gamgeeVersion}](<${source}>)  ${celebration}\nYou can [read the changelog](<${changelog}>) on GitHub!`
 			);
 		}