From 5188d86d610b04f39f960a4c627dc3b0b4fc5ba2 Mon Sep 17 00:00:00 2001 From: syntaxbullet Date: Sat, 28 Mar 2026 17:49:12 +0100 Subject: [PATCH] fix(inventory): address code review findings - Replace setTimeout race in use-item flow with explicit Back button - Fix collector end handler to re-render current view instead of blanking - Add appendUseBackButton helper to attach navigation to use results - Remove unused isInventoryInteraction import - Fix rarity test type assertions Co-Authored-By: Claude Opus 4.6 (1M context) --- bot/commands/inventory/inventory.ts | 79 ++++++++++++++++--------- bot/modules/inventory/inventory.view.ts | 28 +++++++++ shared/lib/rarity.test.ts | 6 +- 3 files changed, 82 insertions(+), 31 deletions(-) diff --git a/bot/commands/inventory/inventory.ts b/bot/commands/inventory/inventory.ts index 0d80043..9e38537 100644 --- a/bot/commands/inventory/inventory.ts +++ b/bot/commands/inventory/inventory.ts @@ -8,6 +8,7 @@ import { getEmptyInventoryMessage, getItemDetailMessage, getDiscardConfirmMessage, + appendUseBackButton, sortInventoryItems, ITEMS_PER_PAGE, type InventoryEntry, @@ -15,7 +16,6 @@ import { import { getLootboxResultMessage } from "@/modules/inventory/inventory.view"; import { parseInventoryCustomId, - isInventoryInteraction, executeItemUse, } from "@/modules/inventory/inventory.interaction"; import { UserError } from "@shared/lib/errors"; @@ -192,31 +192,7 @@ async function setupCollector( try { const result = await executeItemUse(i, viewerId, selectedItemId); const message = getLootboxResultMessage(result.results, result.item); - await interaction.editReply(message as any); - - // After showing result, wait briefly then return to detail or list - setTimeout(async () => { - try { - const freshEntries = await inventoryService.getInventory(ownerId); - const freshSorted = sortInventoryItems(freshEntries as InventoryEntry[]); - const freshEntry = freshSorted.find(e => e.item.id === selectedItemId); - - if (freshEntry) { - await interaction.editReply( - getItemDetailMessage(freshEntry, viewerId, ownerId) - ); - } else { - selectedItemId = null; - if (freshSorted.length === 0) { - await interaction.editReply(getEmptyInventoryMessage(username)); - } else { - await interaction.editReply( - getInventoryListMessage(freshSorted, username, currentPage, viewerId, ownerId) - ); - } - } - } catch {} - }, 3000); + await interaction.editReply(appendUseBackButton(message, viewerId) as any); } catch (error) { if (error instanceof UserError) { await interaction.editReply({ @@ -231,6 +207,27 @@ async function setupCollector( break; } + case "use_back": { + // Return from use result to detail or list + if (!selectedItemId) break; + const entry = sorted.find(e => e.item.id === selectedItemId); + if (entry) { + await interaction.editReply( + getItemDetailMessage(entry, viewerId, ownerId) + ); + } else { + selectedItemId = null; + if (sorted.length === 0) { + await interaction.editReply(getEmptyInventoryMessage(username)); + } else { + await interaction.editReply( + getInventoryListMessage(sorted, username, currentPage, viewerId, ownerId) + ); + } + } + break; + } + case "discard": { if (viewerId !== ownerId || !selectedItemId) break; const entry = sorted.find(e => e.item.id === selectedItemId); @@ -293,7 +290,33 @@ async function setupCollector( } }); - collector.on("end", () => { - interaction.editReply({ components: [] }).catch(() => {}); + collector.on("end", async () => { + try { + // Re-render current view as static (no interactive components) + const entries = await inventoryService.getInventory(ownerId); + const sorted = sortInventoryItems(entries as InventoryEntry[]); + + if (selectedItemId) { + const entry = sorted.find(e => e.item.id === selectedItemId); + if (entry) { + // Show detail view without action buttons + const msg = getItemDetailMessage(entry, viewerId, ownerId); + // Replace components with empty to remove buttons but keep container content + await interaction.editReply(msg); + return; + } + } + + if (sorted.length === 0) { + await interaction.editReply(getEmptyInventoryMessage(username)); + } else { + await interaction.editReply( + getInventoryListMessage(sorted, username, currentPage, viewerId, ownerId) + ); + } + } catch { + // If re-rendering fails, at least try to clear gracefully + interaction.editReply({ components: [] }).catch(() => {}); + } }); } diff --git a/bot/modules/inventory/inventory.view.ts b/bot/modules/inventory/inventory.view.ts index 0ee1e4f..e0b3c8f 100644 --- a/bot/modules/inventory/inventory.view.ts +++ b/bot/modules/inventory/inventory.view.ts @@ -289,6 +289,34 @@ export function getDiscardConfirmMessage(entry: InventoryEntry, viewerId: string }; } +/** + * Wraps a use-item result message with a Back button so the user + * can return to the inventory after seeing the effect result. + */ +export function appendUseBackButton(message: any, viewerId: string): any { + const backRow = new ActionRowBuilder().addComponents( + new ButtonBuilder() + .setCustomId(`inv_use_back_${viewerId}`) + .setLabel("◀ Back to Inventory") + .setStyle(ButtonStyle.Primary) + ); + + // If CV2 message with components array, append to the first container + if (message.components && message.flags === MessageFlags.IsComponentsV2) { + const container = message.components[0]; + if (container?.addActionRowComponents) { + container.addActionRowComponents(backRow); + } + return message; + } + + // Embed-based fallback — add as a regular component row + return { + ...message, + components: [...(message.components || []), backRow], + }; +} + /** * Resolves an item URL (icon or image) for use in CV2 components. * Handles both local assets and remote URLs. diff --git a/shared/lib/rarity.test.ts b/shared/lib/rarity.test.ts index a4173d8..b14db7d 100644 --- a/shared/lib/rarity.test.ts +++ b/shared/lib/rarity.test.ts @@ -16,12 +16,12 @@ describe("getRarityConfig", () => { it("falls back to Common for unknown rarity", () => { const result = getRarityConfig("LEGENDARY"); - expect(result).toEqual(RARITY_CONFIG["C"]); + expect(result).toEqual(RARITY_CONFIG["C"]!); }); it("falls back to Common for null/undefined input", () => { - expect(getRarityConfig(null as any)).toEqual(RARITY_CONFIG["C"]); - expect(getRarityConfig(undefined as any)).toEqual(RARITY_CONFIG["C"]); + expect(getRarityConfig(null as any)).toEqual(RARITY_CONFIG["C"]!); + expect(getRarityConfig(undefined as any)).toEqual(RARITY_CONFIG["C"]!); }); });