Skip to content

Commit 3829be4

Browse files
committed
Rebuild the addon browser on runSession; no hand-rolled collectors left
paginateAddonPages was the last hand-rolled component collector: it repeated the ownership check, the timeout and the disable-on-end pass that framework/session.ts already does. It is now a runSession whose state is the selected index, and its select menu carries a session-owned custom id so the dispatcher leaves it to the collector. Three bugs fell out of the conversion. 1. `/addons search` with no matches threw. string-similarity's findBestMatch rejects an empty candidate list ("Bad arguments"), so a search that matched nothing surfaced as the dispatcher's generic "Something went wrong" rather than "no results". It now answers before ranking. 2. An empty addon list built an illegal select menu. Discord requires between 1 and 25 options; createNavigation([]) produced zero, so `/addons top` and friends would have been rejected at send time whenever the cache was empty — the same shape as the selfroles setMaxValues(0) crash. paginateAddonPages now answers with a notice instead, and callers pass a fitting message. 3. `/addons random` on an empty cache indexed past the end and rendered `undefined`. Guarded. The selection handler also loses `addons.find(...)!`; an unrecognised value now leaves the selection alone rather than asserting the lookup cannot fail. Two follow-ons: - paginateAddonPages takes a RepliableInteraction rather than a ChatInputCommandInteraction. It only defers and edits, so the tighter type was claiming more than the code uses. - tags.ts was still calling awaitModalSubmit directly inside a try/catch that treated any throw as a timeout — including a database failure, which the user would have been told was a submission timeout. It uses the framework's awaitModal now, which returns null only on timeout. src/ no longer contains a hand-rolled collector or modal wait. 16 new tests cover the browser (rendering, selection moving the tick and the page, an unknown value being ignored, ownership, disable-on-end, V2 flags on every render), list separators, sorting, and the conditional support-server button. Writing them exposed two gaps in the shared session harness — no select-menu type guards and no collector.stop() — both now fixed, so the harness can drive select menus and exercise runSession's error path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE
1 parent 28bf9d1 commit 3829be4

6 files changed

Lines changed: 282 additions & 57 deletions

File tree

src/commands/addons.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,29 +46,40 @@ async function browse(interaction: ChatInputCommandInteraction) {
4646

4747
async function search(interaction: ChatInputCommandInteraction) {
4848
const name = interaction.options.getString("name", true).toLowerCase();
49-
let results: BdWebAddon[] = [];
49+
const results: BdWebAddon[] = [];
5050
for (const addon of cache) {
5151
if (addon.name.toLowerCase().includes(name) || (addon.description?.toLowerCase().includes(name))) {
5252
results.push(addon);
5353
}
5454
}
5555

56-
results = Similarity.findBestMatch(name, results.map(a => a.name)).ratings
56+
// findBestMatch throws on an empty candidate list, so a search that matched
57+
// nothing used to surface as the dispatcher's generic error.
58+
if (!results.length) {
59+
return await interaction.editReply(notices.info(`No addons matched \`${name}\`.`));
60+
}
61+
62+
const ranked = Similarity.findBestMatch(name, results.map(addon => addon.name)).ratings
5763
.sort((a, b) => b.rating - a.rating)
5864
.slice(0, 10)
59-
.map(rating => results.find(a => a.name === rating.target)!)
60-
.filter(a => !!a);
65+
.map(rating => results.find(addon => addon.name === rating.target))
66+
.filter((addon): addon is BdWebAddon => addon !== undefined);
6167

62-
await paginateAddonPages(interaction, results);
68+
await paginateAddonPages(interaction, ranked, `No addons matched \`${name}\`.`);
6369
}
6470

6571

6672
async function top10(interaction: ChatInputCommandInteraction, sortBy: "likes" | "downloads" | "initial_release_date" | "latest_release_date") {
67-
await paginateAddonPages(interaction, sortAddons(Array.from(cache), sortBy).slice(0, 10));
73+
await paginateAddonPages(interaction, sortAddons(Array.from(cache), sortBy).slice(0, 10), "The addon store is empty right now. Please try again shortly.");
6874
}
6975

7076
async function random(interaction: ChatInputCommandInteraction) {
7177
const addonsArray = Array.from(cache);
78+
// An empty cache would otherwise index past the end and render `undefined`.
79+
if (!addonsArray.length) {
80+
return await interaction.editReply(notices.info("The addon store is empty right now. Please try again shortly."));
81+
}
82+
7283
const randomAddon = addonsArray[Math.floor(Math.random() * addonsArray.length)];
7384
return await interaction.editReply({components: [createAddonComponent(randomAddon)], flags: MessageFlags.IsComponentsV2});
7485
}

src/commands/tags.ts

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
AutocompleteInteraction, ChatInputCommandInteraction, ComponentType, InteractionContextType,
44
MessageFlags, type RESTPostAPIChatInputApplicationCommandsJSONBody
55
} from "discord.js";
6-
import {defineCommand} from "../framework";
6+
import {awaitModal, defineCommand} from "../framework";
77
import type {AtLeast, Tag} from "../types";
88
import {tagsDB} from "../db";
99
import {msInMinute} from "../util/time";
@@ -108,28 +108,21 @@ async function list(interaction: ChatInputCommandInteraction<"cached">) {
108108
async function showTagModal(interaction: ChatInputCommandInteraction<"cached">, tag: AtLeast<Tag, "name">) {
109109
const isUpdating = !!tag.content;
110110

111-
await interaction.showModal(updateTagModal(tag));
111+
// awaitModal returns null only on timeout, so a database failure below is no
112+
// longer reported to the user as "submission timed out".
113+
const submitted = await awaitModal(interaction, updateTagModal(tag), ["title", "content", "thumbnail"], {time: msInMinute * 5});
114+
if (!submitted) return await interaction.followUp(error("Modal submission timed out!"));
112115

113-
try {
114-
const modalInteraction = await interaction.awaitModalSubmit({time: msInMinute * 5});
115-
const title = modalInteraction.fields.getTextInputValue("title");
116-
const content = modalInteraction.fields.getTextInputValue("content");
117-
const thumbnailUrl = modalInteraction.fields.getTextInputValue("thumbnail");
116+
const guildTags = await tagsDB.get(interaction.guildId) ?? {};
117+
guildTags[tag.name] = {
118+
name: tag.name,
119+
title: submitted.values.title || undefined,
120+
content: submitted.values.content,
121+
thumbnailUrl: submitted.values.thumbnail || undefined
122+
};
123+
await tagsDB.set(interaction.guildId, guildTags);
118124

119-
const guildTags = await tagsDB.get(interaction.guildId) ?? {};
120-
guildTags[tag.name] = {
121-
name: tag.name,
122-
title: title || undefined,
123-
content,
124-
thumbnailUrl: thumbnailUrl || undefined,
125-
};
126-
await tagsDB.set(interaction.guildId, guildTags);
127-
128-
await modalInteraction.reply(success(`Tag \`${tag.name}\` has been ${isUpdating ? "updated" : "created"} successfully!`));
129-
}
130-
catch {
131-
await interaction.followUp(error("Modal submission timed out!"));
132-
}
125+
await submitted.submission.reply(success(`Tag \`${tag.name}\` has been ${isUpdating ? "updated" : "created"} successfully!`));
133126
}
134127

135128

src/util/addons.ts

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import {
2-
ButtonStyle, ChatInputCommandInteraction, ComponentType, MessageFlags, SeparatorSpacingSize,
3-
StringSelectMenuInteraction,
2+
ButtonStyle, ComponentType, MessageFlags, SeparatorSpacingSize,
43
type ActionRowData, type ComponentInContainerData, type ContainerComponentData,
5-
type MessageActionRowComponentData, type SectionComponentData, type TextDisplayComponentData
4+
type MessageActionRowComponentData, type RepliableInteraction, type SectionComponentData,
5+
type TextDisplayComponentData
66
} from "discord.js";
7-
import {container, row, text} from "../framework";
7+
import {container, row, runSession, sessionId, text} from "../framework";
8+
import * as notices from "./notices";
89
import type {BdWebAddon} from "../types";
910

1011
import Web from "../util/web";
@@ -152,10 +153,13 @@ export function createAddonList(title: string, addons: BdWebAddon[]): [TextDispl
152153
}
153154

154155

156+
/** The select menu action, as carried in its session-owned custom id. */
157+
const NAVIGATE = "addons-navigate";
158+
155159
export function createNavigation(addons: BdWebAddon[], selectedIndex = 0, disabled = false): ActionRowData<MessageActionRowComponentData> {
156160
return row({
157161
type: ComponentType.StringSelect,
158-
customId: "addons-navigation",
162+
customId: sessionId(NAVIGATE),
159163
disabled,
160164
options: addons.map((addon, index) => ({
161165
"label": `${index + 1}. ${addon.name}`,
@@ -166,28 +170,37 @@ export function createNavigation(addons: BdWebAddon[], selectedIndex = 0, disabl
166170
}
167171

168172

169-
export async function paginateAddonPages(interaction: ChatInputCommandInteraction, addons: BdWebAddon[]) {
170-
const navigation = createNavigation(addons);
171-
const pages = addons.map(addon => createAddonComponent(addon));
173+
/**
174+
* An addon browser: a select menu that swaps which addon is shown.
175+
*
176+
* Built on runSession, so the ownership check, the timeout and disabling the
177+
* menu when it expires are the framework's job rather than this file's.
178+
*/
179+
export async function paginateAddonPages(interaction: RepliableInteraction, addons: BdWebAddon[], emptyMessage = "No addons matched.") {
180+
// A select menu needs between 1 and 25 options; Discord rejects an empty
181+
// one, so an empty result set has to be answered rather than rendered.
182+
if (!addons.length) {
183+
await interaction.editReply(notices.info(emptyMessage));
184+
return;
185+
}
172186

173-
const msg = await interaction.fetchReply();
174-
const collector = msg.createMessageComponentCollector({time: 5 * msInMinute});
187+
await runSession<number>({
188+
interaction,
189+
initial: 0,
190+
timeout: 5 * msInMinute,
175191

176-
let selectedIndex = 0;
177-
collector.on("collect", async (i: StringSelectMenuInteraction) => {
178-
if (i.user.id !== interaction.user.id) return await i.reply({content: "You cannot interact with this menu.", flags: MessageFlags.Ephemeral});
192+
render: (selectedIndex, {ended}) => ({
193+
flags: MessageFlags.IsComponentsV2,
194+
components: [createNavigation(addons, selectedIndex, ended), createAddonComponent(addons[selectedIndex])]
195+
}),
179196

180-
const selectedAddonName = i.values[0];
181-
const selectedAddon = addons.find(a => a.name === selectedAddonName)!;
182-
selectedIndex = addons.indexOf(selectedAddon);
183-
const newPage = pages[selectedIndex];
184-
const newNavigation = createNavigation(addons, selectedIndex);
185-
await i.update({components: [newNavigation, newPage], flags: MessageFlags.IsComponentsV2});
186-
});
197+
reduce(action, _selectedIndex, component) {
198+
if (action !== NAVIGATE || !component.isStringSelectMenu()) return undefined;
187199

188-
collector.on("end", async () => {
189-
await interaction.editReply({components: [createNavigation(addons, selectedIndex, true), pages[selectedIndex]], flags: MessageFlags.IsComponentsV2});
200+
// An unrecognised value leaves the state alone instead of throwing;
201+
// the previous version asserted the lookup could not fail.
202+
const next = addons.findIndex(addon => addon.name === component.values[0]);
203+
return next === -1 ? undefined : next;
204+
}
190205
});
191-
192-
await interaction.editReply({components: [navigation, pages[0]], flags: MessageFlags.IsComponentsV2});
193-
}
206+
}

tests/addons.test.ts

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
import {describe, expect, test} from "bun:test";
2+
3+
import {createAddonComponent, createAddonList, createNavigation, paginateAddonPages, sortAddons} from "../src/util/addons";
4+
import {isSessionId} from "../src/framework";
5+
import type {BdWebAddon} from "../src/types";
6+
import {sessionHarness} from "./helpers/session";
7+
8+
9+
function addon(over: Partial<BdWebAddon> = {}): BdWebAddon {
10+
return {
11+
id: 7,
12+
name: "CoolPlugin",
13+
file_name: "c.plugin.js",
14+
type: "plugin",
15+
description: "Does things",
16+
version: "1.2.3",
17+
likes: 1234,
18+
downloads: 56789,
19+
tags: [],
20+
thumbnail_url: "/resources/x.png",
21+
latest_source_url: "u",
22+
initial_release_date: new Date("2020-01-02T00:00:00Z"),
23+
latest_release_date: new Date("2024-03-04T00:00:00Z"),
24+
author: {
25+
github_id: "1",
26+
github_name: "g",
27+
display_name: "d",
28+
discord_name: "dn",
29+
discord_avatar_hash: null,
30+
discord_snowflake: "1",
31+
guild: null
32+
},
33+
guild: null,
34+
...over
35+
};
36+
}
37+
38+
const IS_COMPONENTS_V2 = 1 << 15;
39+
40+
interface Row {type: number; components: Array<{options?: Array<{label: string; value: string; default: boolean}>; disabled?: boolean}>}
41+
const navOf = (shown: Record<string, unknown>) => (shown.components as unknown[])[0] as Row;
42+
const menu = (shown: Record<string, unknown>) => navOf(shown).components[0];
43+
44+
45+
describe("navigation menu", () => {
46+
const list = [addon({name: "Alpha"}), addon({name: "Beta"}), addon({name: "Gamma"})];
47+
48+
test("is session-owned so the dispatcher leaves it alone", () => {
49+
const control = createNavigation(list).components[0] as {customId: string};
50+
expect(isSessionId(control.customId)).toBe(true);
51+
});
52+
53+
test("numbers the options and marks the selected one", () => {
54+
const options = menu({components: [createNavigation(list, 1)]}).options ?? [];
55+
expect(options.map(option => option.label)).toEqual(["1. Alpha", "2. Beta", "3. Gamma"]);
56+
expect(options.map(option => option.default)).toEqual([false, true, false]);
57+
});
58+
});
59+
60+
61+
describe("addon browser", () => {
62+
const list = [addon({name: "Alpha"}), addon({name: "Beta"}), addon({name: "Gamma"})];
63+
64+
function browse(addons: BdWebAddon[]) {
65+
const harness = sessionHarness();
66+
const done = paginateAddonPages(harness.interaction, addons);
67+
return {harness, done, latest: () => harness.shown.at(-1) ?? {}};
68+
}
69+
70+
test("an empty list answers instead of building an illegal select menu", async () => {
71+
// Discord rejects a string select with zero options.
72+
const {harness, done} = browse([]);
73+
await done;
74+
const shown = harness.shown.at(-1) ?? {};
75+
expect(Number(shown.flags) & IS_COMPONENTS_V2).toBe(IS_COMPONENTS_V2);
76+
expect(JSON.stringify(shown)).toContain("No addons matched");
77+
});
78+
79+
test("renders the first addon with its menu", async () => {
80+
const {harness, done} = browse(list);
81+
await harness.press("addons-navigate", {values: ["Alpha"]});
82+
const options = menu(harness.shown[0]).options ?? [];
83+
expect(options).toHaveLength(3);
84+
expect(options[0]?.default).toBe(true);
85+
await harness.end();
86+
await done;
87+
});
88+
89+
test("the menu is disabled once the session ends", async () => {
90+
const {harness, done} = browse(list);
91+
await harness.press("addons-navigate", {values: ["Alpha"]});
92+
await harness.end();
93+
await done;
94+
expect(menu(harness.shown.at(-1) ?? {}).disabled).toBe(true);
95+
});
96+
97+
test("every render carries IsComponentsV2", async () => {
98+
const {harness, done} = browse(list);
99+
await harness.press("addons-navigate", {values: ["Alpha"]});
100+
await harness.end();
101+
await done;
102+
expect(harness.shown.every(shown => Number(shown.flags) === IS_COMPONENTS_V2)).toBe(true);
103+
});
104+
105+
test("a stranger cannot drive the browser", async () => {
106+
const {harness, done} = browse(list);
107+
const refusals = await harness.press("addons-navigate", {userId: "someone-else", values: ["Beta"]});
108+
expect(String(refusals[0]?.content)).toContain("belongs to someone else");
109+
await harness.end();
110+
await done;
111+
});
112+
113+
test("selecting an addon swaps the page and moves the tick", async () => {
114+
const {harness, done} = browse(list);
115+
await harness.press("addons-navigate", {values: ["Gamma"]});
116+
117+
const options = menu(harness.shown.at(-1) ?? {}).options ?? [];
118+
expect(options.map(option => option.default)).toEqual([false, false, true]);
119+
expect(JSON.stringify(harness.shown.at(-1))).toContain("# Gamma v1.2.3");
120+
121+
await harness.end();
122+
await done;
123+
});
124+
125+
// The previous version did `addons.find(...)!` and would have thrown.
126+
test("an unknown value leaves the selection alone", async () => {
127+
const {harness, done} = browse(list);
128+
await harness.press("addons-navigate", {values: ["NoSuchAddon"]});
129+
const options = menu(harness.shown.at(-1) ?? {}).options ?? [];
130+
expect(options.map(option => option.default)).toEqual([true, false, false]);
131+
await harness.end();
132+
await done;
133+
});
134+
});
135+
136+
137+
describe("list rendering", () => {
138+
test("separates entries but does not trail one", () => {
139+
const [, page] = createAddonList("Plugins", [addon({name: "A"}), addon({name: "B"})]);
140+
const types = (page.components as Array<{type: number}>).map(component => component.type);
141+
expect(types).toEqual([9, 14, 9]);
142+
});
143+
144+
test("a single entry gets no separator", () => {
145+
const [, page] = createAddonList("Plugins", [addon()]);
146+
expect(page.components).toHaveLength(1);
147+
});
148+
149+
test("the heading is a separate top-level text display", () => {
150+
const [heading] = createAddonList("Plugins sorted by downloads", [addon()]);
151+
expect(heading.content).toBe("## Plugins sorted by downloads");
152+
});
153+
});
154+
155+
156+
describe("sorting", () => {
157+
test("orders by the numeric field, descending", () => {
158+
const list = [addon({name: "a", likes: 1}), addon({name: "b", likes: 9}), addon({name: "c", likes: 5})];
159+
expect(sortAddons(list, "likes").map(a => a.name)).toEqual(["b", "c", "a"]);
160+
});
161+
162+
test("orders by date, newest first", () => {
163+
const list = [
164+
addon({name: "old", latest_release_date: new Date("2020-01-01T00:00:00Z")}),
165+
addon({name: "new", latest_release_date: new Date("2024-01-01T00:00:00Z")})
166+
];
167+
expect(sortAddons(list, "latest_release_date").map(a => a.name)).toEqual(["new", "old"]);
168+
});
169+
});
170+
171+
172+
describe("addon page", () => {
173+
test("adds a support-server button only when the author has a guild", () => {
174+
const withoutGuild = createAddonComponent(addon());
175+
const withGuild = createAddonComponent(addon({
176+
author: {...addon().author, guild: {name: "G", snowflake: "1", invite_link: "https://discord.gg/abc"}}
177+
}));
178+
const labels = (page: {components: readonly unknown[]}) =>
179+
JSON.stringify(page.components).match(/"label":"[^"]+"/g) ?? [];
180+
expect(labels(withoutGuild)).toHaveLength(2);
181+
expect(labels(withGuild)).toHaveLength(3);
182+
});
183+
184+
test("falls back when an addon has no description", () => {
185+
const page = createAddonComponent(addon({description: undefined as unknown as string}));
186+
expect(JSON.stringify(page)).toContain("No description provided.");
187+
});
188+
});

0 commit comments

Comments
 (0)