Feature/121 c 005 implement faq remove

src/commands/faq/faq.ts

@@ -1,17 +1,20 @@
 // src/commands/faq/faq.ts
 //
-// Base /faq slash command.
+// Base /faq slash command (router).
 //
 // Responsibilities:
-// - Define the /faq command + subcommands (Discord API shape)
+// - Define the public Discord command shape (/faq + subcommands + options)
 // - Own the interaction lifecycle (deferReply + editReply)
-// - Route to subcommand handlers
+// - Route to subcommand handlers (thin controller)
 //
 // Non-goals:
-// - No business logic (validation, storage, permissions)
+// - No FAQ business logic
 // - No file I/O
+// - No persistence or validation rules beyond “required option” at the Discord level
 //
-// Subcommands live in: src/commands/faq/subcommands/
+// All real work is delegated to:
+// - src/commands/faq/subcommands/*   (Discord-facing handlers)
+// - src/services/faq/*              (business logic + persistence)
 
 import {
   MessageFlags,
@@ -20,7 +23,12 @@ import {
 } from "discord.js";
 import { logger } from "../../utils/logger.js";
 
+// Subcommand runners (each one must NOT reply/defer on its own)
 import { run as runAdd } from "./subcommands/add.js";
+import { run as runRemove } from "./subcommands/remove.js";
+// Future:
+// import { run as runGet } from "./subcommands/get.js";
+// import { run as runList } from "./subcommands/list.js";
 
 export const data = new SlashCommandBuilder()
   .setName("faq")
@@ -41,7 +49,10 @@ export const data = new SlashCommandBuilder()
         o.setName("body").setDescription("Answer text").setRequired(true),
       )
       .addStringOption((o) =>
-        o.setName("tags").setDescription("Comma-separated tags").setRequired(false),
+        o
+          .setName("tags")
+          .setDescription("Comma-separated tags (optional)")
+          .setRequired(false),
       )
       .addBooleanOption((o) =>
         o
@@ -57,7 +68,7 @@ export const data = new SlashCommandBuilder()
       .setName("get")
       .setDescription("Get a FAQ entry by key")
       .addStringOption((o) =>
-        o.setName("key").setDescription("FAQ key").setRequired(true),
+        o.setName("key").setDescription("Key to fetch").setRequired(true),
       )
       .addBooleanOption((o) =>
         o
@@ -72,6 +83,9 @@ export const data = new SlashCommandBuilder()
     s
       .setName("list")
       .setDescription("List FAQ entries")
+      .addStringOption((o) =>
+        o.setName("tag").setDescription("Filter by a tag (optional)").setRequired(false),
+      )
       .addBooleanOption((o) =>
         o
           .setName("ephemeral")
@@ -86,7 +100,7 @@ export const data = new SlashCommandBuilder()
       .setName("remove")
       .setDescription("Remove a FAQ entry by key")
       .addStringOption((o) =>
-        o.setName("key").setDescription("FAQ key").setRequired(true),
+        o.setName("key").setDescription("Key to remove").setRequired(true),
       )
       .addBooleanOption((o) =>
         o
@@ -96,11 +110,22 @@ export const data = new SlashCommandBuilder()
       ),
   );
 
+/**
+ * Command execution entry point.
+ *
+ * Pattern:
+ * - Read subcommand name
+ * - Defer reply immediately (avoids Discord 3s timeout)
+ * - Route to subcommand handler
+ *
+ * IMPORTANT:
+ * Subcommand handlers must only use editReply (no reply/defer).
+ */
 export async function execute(interaction: ChatInputCommandInteraction): Promise<void> {
   const sub = interaction.options.getSubcommand(true);
   const ephemeral = interaction.options.getBoolean("ephemeral") ?? false;
 
-  // Parent command owns the interaction lifecycle.
+  // Own the lifecycle here (subcommands only editReply)
   await interaction.deferReply(ephemeral ? { flags: MessageFlags.Ephemeral } : undefined);
 
   try {
@@ -109,7 +134,12 @@ export async function execute(interaction: ChatInputCommandInteraction): Promise
       return;
     }
 
-    // Placeholders for upcoming tickets
+    if (sub === "remove") {
+      await runRemove(interaction);
+      return;
+    }
+
+    // Placeholders until you wire these up
     if (sub === "get") {
       await interaction.editReply("TODO: /faq get");
       return;
@@ -120,11 +150,6 @@ export async function execute(interaction: ChatInputCommandInteraction): Promise
       return;
     }
 
-    if (sub === "remove") {
-      await interaction.editReply("TODO: /faq remove");
-      return;
-    }
-
     await interaction.editReply("Unknown subcommand.");
   } catch (err) {
     logger.error({ err, sub }, "[faq] subcommand failed");

src/commands/faq/subcommands/remove.ts

@@ -1 +1,140 @@
-// src/commands/faq/subcommand/remove.ts
+// src/commands/faq/subcommands/remove.ts
+//
+// /faq remove
+//
+// Responsibilities:
+// - Confirm intent (button) before deleting
+// - Enforce basic permissions (Manage Guild or Administrator)
+// - Delegate all real work to the FAQ service layer
+//
+// IMPORTANT:
+// - This file is NOT a slash command by itself
+// - It must NOT call reply() or deferReply()
+// - The parent command (faq.ts) owns the interaction lifecycle
+
+import type { ChatInputCommandInteraction } from "discord.js";
+import {
+  ActionRowBuilder,
+  ButtonBuilder,
+  ButtonStyle,
+  ComponentType,
+  PermissionsBitField,
+} from "discord.js";
+import { logger } from "../../../utils/logger.js";
+
+// NOTE: Import from the real service layer (src/services/faq/services.ts).
+// This path is relative from src/commands/faq/subcommands/remove.ts
+
+import { getByKey, remove } from "../services.js";
+
+function canRemoveFaq(interaction: ChatInputCommandInteraction): boolean {
+  // memberPermissions is the safe v14 way (avoids the string|PermissionsBitField union)
+  const perms = interaction.memberPermissions;
+  if (!perms) return false;
+
+  return (
+    perms.has(PermissionsBitField.Flags.ManageGuild) ||
+    perms.has(PermissionsBitField.Flags.Administrator)
+  );
+}
+
+export async function run(interaction: ChatInputCommandInteraction): Promise<void> {
+  try {
+    // Permissions only make sense in a guild context.
+    if (!interaction.inGuild()) {
+      await interaction.editReply("❌ `/faq remove` can only be used in a server.");
+      return;
+    }
+
+    if (!canRemoveFaq(interaction)) {
+      await interaction.editReply("❌ You do not have permission to remove FAQs.");
+      return;
+    }
+
+    const rawKey = interaction.options.getString("key", true);
+    const key = rawKey.trim();
+
+    if (key.length === 0) {
+      await interaction.editReply("❌ Key cannot be empty.");
+      return;
+    }
+
+    const existing = getByKey(key);
+    if (!existing) {
+      await interaction.editReply(`❌ FAQ not found: **${key}**`);
+      return;
+    }
+
+    // Use interaction.id to keep ids unique per invocation.
+    // The filter below ensures only the invoking user can click them.
+
+    const confirmId = `faq_remove_confirm:${interaction.id}`;
+    const cancelId = `faq_remove_cancel:${interaction.id}`;
+
+    const row = new ActionRowBuilder<ButtonBuilder>().addComponents(
+      new ButtonBuilder()
+        .setCustomId(confirmId)
+        .setLabel("Delete")
+        .setStyle(ButtonStyle.Danger),
+      new ButtonBuilder()
+        .setCustomId(cancelId)
+        .setLabel("Cancel")
+        .setStyle(ButtonStyle.Secondary),
+    );
+
+    const msg = await interaction.editReply({
+      content: [
+        `🗑️ Remove FAQ **${existing.key}**?`,
+        `Title: **${existing.title}**`,
+        "",
+        "This cannot be undone.",
+      ].join("\n"),
+      components: [row],
+    });
+
+    // Wait for the user to confirm or cancel.
+    // If it times out, clear buttons and exit cleanly.
+
+    let clicked;
+    try {
+      clicked = await msg.awaitMessageComponent({
+        componentType: ComponentType.Button,
+        time: 20_000,
+        filter: (i) => i.user.id === interaction.user.id,
+      });
+    } catch {
+      await interaction.editReply({
+        content: "⏱️ Timed out. Nothing was deleted.",
+        components: [],
+      });
+      return;
+    }
+
+    if (clicked.customId === cancelId) {
+      await clicked.update({
+        content: "✅ Cancelled. Nothing was deleted.",
+        components: [],
+      });
+      return;
+    }
+
+    // Confirm path
+
+    const ok = remove(existing.key);
+
+    await clicked.update({
+      content: ok
+        ? `✅ Deleted FAQ **${existing.key}**`
+        : `❌ Could not delete FAQ **${existing.key}** (it may have already been removed).`,
+      components: [],
+    });
+
+    logger.info(
+      { userId: interaction.user.id, key: existing.key },
+      "[faq/remove] removed",
+    );
+  } catch (err) {
+    logger.error({ err }, "[faq/remove] failed");
+    await interaction.editReply("❌ Could not remove FAQ right now. Try again in a bit.");
+  }
+}

src/services/discord/interactionHandler.ts

@@ -1,34 +1,61 @@
 // src/services/discord/interactionHandler.ts
+//
+// Central interaction router for Discord.
+//
+// Responsibilities:
+// - Route slash commands to their registered handlers
+// - Handle execution errors safely
+// - Ensure users always receive a response
+//
+// IMPORTANT DESIGN NOTE:
+// - Slash commands are handled globally here
+// - Buttons and other component interactions are handled locally
+//   by the command that created them (via collectors)
+//
+// This separation is REQUIRED for confirmation flows (ex: /faq remove)
 
 import type { Interaction, ChatInputCommandInteraction } from "discord.js";
 import type { CommandClient } from "./commandLoader.js";
 import { logger } from "../../utils/logger.js";
 
 /**
- * Handles incoming Discord interactions and dispatches slash commands
- * to their registered executors.
+ * Handle a single Discord interaction.
  *
- * Responsibilities:
- * - Ignore non-slash interactions
- * - Look up the command handler
- * - Execute safely with error handling
- * - Ensure the user always receives a response
+ * This function is intentionally conservative:
+ * - It only routes slash commands
+ * - It explicitly ignores buttons so collectors can handle them
  */
 export async function handleInteraction(
   interaction: Interaction,
   client: CommandClient,
 ): Promise<void> {
   /**
-   * We only care about slash commands.
-   * Other interaction types (buttons, modals, etc.) are ignored here.
+   * IMPORTANT:
+   * Button interactions (confirm / cancel, etc.) are NOT routed here.
+   *
+   * They are handled by local collectors created by the command itself
+   * (for example: /faq remove confirmation buttons).
+   *
+   * If we process buttons here, those flows will break.
+   */
+  if (interaction.isButton()) {
+    return;
+  }
+
+  /**
+   * We only care about slash commands in this router.
+   * Other interaction types are intentionally ignored.
    */
   if (!interaction.isChatInputCommand()) return;
 
+  /**
+   * Look up the registered command handler.
+   */
   const command = client.commands.get(interaction.commandName);
 
   /**
-   * No command registered for this name.
-   * This usually indicates a stale command or partial deploy.
+   * No handler registered for this command.
+   * Usually indicates a stale deploy or command mismatch.
    */
   if (!command) {
     logger.warn(
@@ -49,11 +76,11 @@ export async function handleInteraction(
   /**
    * Execute the command safely.
    *
-   * Any thrown error is:
-   * - Logged with context
-   * - Converted into a generic user-facing message
+   * Any error thrown by the command:
+   * - Is logged with context
+   * - Results in a generic user-facing error
    *
-   * We avoid leaking internal errors to Discord users.
+   * Internal details are never leaked to Discord.
    */
   try {
     await command.execute(interaction as ChatInputCommandInteraction);