feat(role-menu): replace reaction-role with button-only role-menu - #11
Merged
Merged
Conversation
- Remove all reaction-mode code (listeners, emoji helpers, URL parser) - Drop emoji/button_id DB columns; add label column - New /role-menu command: create, add (≤5 roles + labels), remove, edit, delete, list - remove uses message-id + role instead of mapping-id - list shows message ID + Discord link for easy copy-paste
📝 WalkthroughWalkthroughThis PR replaces an emoji-based reaction role system with a button-based role menu system. It introduces a new ChangesRole-Menu Feature Implementation
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@prisma/migrations/20260526000000_role_menu_cleanup/migration.sql`:
- Around line 2-7: Add a dedupe step before creating the unique index on
reaction_role_mappings(menu_id, role_id): identify rows having the same
(menu_id, role_id) and delete duplicates while keeping a single canonical row
per pair (for example by keeping the row with the lowest primary key/id or
earliest ctid), then proceed to CREATE UNIQUE INDEX
"reaction_role_mappings_menu_id_role_id_key". This ensures the CREATE UNIQUE
INDEX on reaction_role_mappings(menu_id, role_id) will not fail due to legacy
duplicate rows.
In `@src/commands/role-menu/index.ts`:
- Around line 178-181: In the for loop that iterates i from 1 to 5 and calls
interaction.options.getRole(`role${i}`), don't stop parsing when one optional
slot is missing: replace the break that exits the loop with continue so later
roles (e.g., role3 when role2 is absent) are still processed; keep the existing
call to ensureRoleAssignable(role, interaction) for non-null role values and
only skip to the next iteration when role is null.
- Around line 191-196: The current .catch(() => null) on the addRoleButton call
masks all DB errors; instead catch only Prisma unique-constraint P2002 and
return null for that case, rethrow any other errors. Modify the code around the
addRoleButton invocation so it awaits addRoleButton(...) inside a try/catch, and
in the catch check if err is a PrismaClientKnownRequestError and err.code ===
'P2002' then set mapping = null (to indicate "已存在"), otherwise throw err;
reference addRoleButton and ReactionRoleMapping/@@unique([menuId, roleId]) in
your change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 842a0ced-3508-4669-95cf-5fe91dfb3c4d
📒 Files selected for processing (15)
.gitignoreprisma/migrations/20260526000000_role_menu_cleanup/migration.sqlprisma/schema.prismasrc/commands/index.tssrc/commands/info/help.tssrc/commands/reaction-role/index.tssrc/commands/role-menu/index.tssrc/core/command/types.tssrc/features/reaction-role/emoji.tssrc/features/reaction-role/listeners.tssrc/features/reaction-role/service.tssrc/features/reaction-role/url.tssrc/features/role-menu/listeners.tssrc/features/role-menu/service.tssrc/index.ts
💤 Files with no reviewable changes (5)
- src/commands/reaction-role/index.ts
- src/features/reaction-role/url.ts
- src/features/reaction-role/listeners.ts
- src/features/reaction-role/emoji.ts
- src/features/reaction-role/service.ts
Comment on lines
+2
to
+7
| ALTER TABLE "reaction_role_mappings" DROP COLUMN "button_id", | ||
| DROP COLUMN "emoji", | ||
| ADD COLUMN "label" TEXT; | ||
|
|
||
| -- CreateIndex | ||
| CREATE UNIQUE INDEX "reaction_role_mappings_menu_id_role_id_key" ON "reaction_role_mappings"("menu_id", "role_id"); |
There was a problem hiding this comment.
Guard migration against pre-existing duplicate (menu_id, role_id) rows.
Line 7 can fail at deploy time if legacy data has multiple rows for the same menu/role pair (previous schema allowed that through different emoji/button values). Add a dedupe step before creating the unique index.
💡 Suggested migration hardening
ALTER TABLE "reaction_role_mappings" DROP COLUMN "button_id",
DROP COLUMN "emoji",
ADD COLUMN "label" TEXT;
+-- Deduplicate rows that would violate the new unique key
+WITH ranked AS (
+ SELECT id,
+ ROW_NUMBER() OVER (PARTITION BY menu_id, role_id ORDER BY id) AS rn
+ FROM "reaction_role_mappings"
+)
+DELETE FROM "reaction_role_mappings" r
+USING ranked x
+WHERE r.id = x.id
+ AND x.rn > 1;
+
-- CreateIndex
CREATE UNIQUE INDEX "reaction_role_mappings_menu_id_role_id_key" ON "reaction_role_mappings"("menu_id", "role_id");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ALTER TABLE "reaction_role_mappings" DROP COLUMN "button_id", | |
| DROP COLUMN "emoji", | |
| ADD COLUMN "label" TEXT; | |
| -- CreateIndex | |
| CREATE UNIQUE INDEX "reaction_role_mappings_menu_id_role_id_key" ON "reaction_role_mappings"("menu_id", "role_id"); | |
| ALTER TABLE "reaction_role_mappings" DROP COLUMN "button_id", | |
| DROP COLUMN "emoji", | |
| ADD COLUMN "label" TEXT; | |
| -- Deduplicate rows that would violate the new unique key | |
| WITH ranked AS ( | |
| SELECT id, | |
| ROW_NUMBER() OVER (PARTITION BY menu_id, role_id ORDER BY id) AS rn | |
| FROM "reaction_role_mappings" | |
| ) | |
| DELETE FROM "reaction_role_mappings" r | |
| USING ranked x | |
| WHERE r.id = x.id | |
| AND x.rn > 1; | |
| -- CreateIndex | |
| CREATE UNIQUE INDEX "reaction_role_mappings_menu_id_role_id_key" ON "reaction_role_mappings"("menu_id", "role_id"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@prisma/migrations/20260526000000_role_menu_cleanup/migration.sql` around
lines 2 - 7, Add a dedupe step before creating the unique index on
reaction_role_mappings(menu_id, role_id): identify rows having the same
(menu_id, role_id) and delete duplicates while keeping a single canonical row
per pair (for example by keeping the row with the lowest primary key/id or
earliest ctid), then proceed to CREATE UNIQUE INDEX
"reaction_role_mappings_menu_id_role_id_key". This ensures the CREATE UNIQUE
INDEX on reaction_role_mappings(menu_id, role_id) will not fail due to legacy
duplicate rows.
Comment on lines
+178
to
+181
| for (let i = 1; i <= 5; i++) { | ||
| const role = interaction.options.getRole(`role${i}`) as Role | null; | ||
| if (!role) break; | ||
| ensureRoleAssignable(role, interaction); |
There was a problem hiding this comment.
Do not stop parsing at first missing optional role slot.
Line 180 uses break; this drops later provided roles (e.g., role1 + role3). Use continue instead.
💡 Suggested fix
for (let i = 1; i <= 5; i++) {
const role = interaction.options.getRole(`role${i}`) as Role | null;
- if (!role) break;
+ if (!role) continue;
ensureRoleAssignable(role, interaction);
const label = interaction.options.getString(`label${i}`) ?? undefined;
pairs.push({ role, label });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (let i = 1; i <= 5; i++) { | |
| const role = interaction.options.getRole(`role${i}`) as Role | null; | |
| if (!role) break; | |
| ensureRoleAssignable(role, interaction); | |
| for (let i = 1; i <= 5; i++) { | |
| const role = interaction.options.getRole(`role${i}`) as Role | null; | |
| if (!role) continue; | |
| ensureRoleAssignable(role, interaction); | |
| const label = interaction.options.getString(`label${i}`) ?? undefined; | |
| pairs.push({ role, label }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/role-menu/index.ts` around lines 178 - 181, In the for loop that
iterates i from 1 to 5 and calls interaction.options.getRole(`role${i}`), don't
stop parsing when one optional slot is missing: replace the break that exits the
loop with continue so later roles (e.g., role3 when role2 is absent) are still
processed; keep the existing call to ensureRoleAssignable(role, interaction) for
non-null role values and only skip to the next iteration when role is null.
Comment on lines
+191
to
+196
| const mapping = await addRoleButton({ menuId: menu.id, roleId: role.id, label }).catch( | ||
| () => null, | ||
| ); | ||
| if (!mapping) { | ||
| lines.push(`⚠️ <@&${role.id}> 已存在，跳過`); | ||
| continue; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file under review and show the relevant region
echo "=== src/commands/role-menu/index.ts (lines ~160-230) ==="
nl -ba src/commands/role-menu/index.ts | sed -n '160,230p'
# Find where addRoleButton is defined and/or imported
echo
echo "=== References to addRoleButton ==="
rg -n "addRoleButton" -S src || true
# If it's imported from another module, print that module's contents near the function
# Use rg to capture import path(s)
echo
echo "=== addRoleButton import statements (in role-menu file) ==="
rg -n "import .*addRoleButton|from .*addRoleButton" src/commands/role-menu/index.ts || true
# Try to locate addRoleButton implementation in the repo
echo
echo "=== Possible addRoleButton definition ==="
rg -n "function addRoleButton|const addRoleButton|async function addRoleButton" -S src || trueRepository: HyperSoWeak/yuzu-bot
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/commands/role-menu/index.ts"
echo "=== Exists? $file ==="
test -f "$file" && echo "yes" || (echo "missing"; exit 1)
echo "=== src/commands/role-menu/index.ts (lines ~160-230) ==="
# Print with line numbers using awk (nl not guaranteed)
awk 'NR>=160 && NR<=230 {printf "%d\t%s\n", NR, $0}' "$file"
echo
echo "=== References to addRoleButton ==="
rg -n "addRoleButton" -S src/commands/role-menu/index.ts || true
echo
echo "=== addRoleButton imports in role-menu file ==="
rg -n "import .*addRoleButton|from .*addRoleButton" -S "$file" || true
echo
echo "=== addRoleButton definition candidates in repo ==="
rg -n "function addRoleButton|const addRoleButton|async function addRoleButton" -S src || trueRepository: HyperSoWeak/yuzu-bot
Length of output: 3558
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/features/role-menu/service.ts (around addRoleButton) ==="
awk 'NR>=1 && NR<=220 {print NR "\t" $0}' src/features/role-menu/service.ts | sed -n '1,220p'
echo
echo "=== Search for Prisma client usage in service.ts ==="
rg -n "Prisma|prisma|P2002|PrismaClientKnownRequestError" src/features/role-menu/service.ts || true
echo
echo "=== Find prisma schema file(s) ==="
fd -e prisma schema . 2>/dev/null || true
fd -e prisma . 2>/dev/null || true
# Try typical location
if [ -f prisma/schema.prisma ]; then
echo
echo "=== prisma/schema.prisma (models mentioning role_menu / mapping) ==="
rg -n "reaction_role_menu|role_menu|menuId|roleId|Mapping|mapping|P2002" prisma/schema.prisma || true
fi
# Also search entire repo for "P2002" occurrences
echo
echo "=== Search whole repo for P2002 usage ==="
rg -n "P2002|PrismaClientKnownRequestError" src prisma || trueRepository: HyperSoWeak/yuzu-bot
Length of output: 3734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/features/achievement/service.ts (around P2002 handling) ==="
awk 'NR>=1 && NR<=120 {print NR "\t" $0}' src/features/achievement/service.ts | sed -n '1,120p'
echo
echo "=== Search for PrismaClientKnownRequestError usage across src ==="
rg -n "PrismaClientKnownRequestError|`@prisma/client`.*Prisma|from '`@prisma/client`'.*Prisma" src || true
echo
echo "=== Search for pattern (err as { code?: string }).code ==="
rg -n "\\(err as \\{ code\\?: string \\}\\)\\.code|err\\.code === 'P2002'" src || trueRepository: HyperSoWeak/yuzu-bot
Length of output: 3083
Only suppress Prisma unique-conflict (P2002) errors in role-menu add flow
In src/commands/role-menu/index.ts (lines 191-196), the .catch(() => null) on addRoleButton masks all Prisma/DB failures as “already exists”. addRoleButton uses prisma.reactionRoleMapping.create, and ReactionRoleMapping has @@unique([menuId, roleId]), so only Prisma error code P2002 for that unique constraint should map to “已存在”; rethrow other errors (consistent with the repo’s P2002 handling in src/features/achievement/service.ts).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/role-menu/index.ts` around lines 191 - 196, The current
.catch(() => null) on the addRoleButton call masks all DB errors; instead catch
only Prisma unique-constraint P2002 and return null for that case, rethrow any
other errors. Modify the code around the addRoleButton invocation so it awaits
addRoleButton(...) inside a try/catch, and in the catch check if err is a
PrismaClientKnownRequestError and err.code === 'P2002' then set mapping = null
(to indicate "已存在"), otherwise throw err; reference addRoleButton and
ReactionRoleMapping/@@unique([menuId, roleId]) in your change.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/role-menu/service.ts (1)
56-61: ⚡ Quick winUse
findUniqueinstead offindFirstfor unique constraint.Since the schema enforces a unique constraint on
(menuId, roleId), usefindUniqueinstead offindFirstfor correctness, clarity, and potential performance benefits.♻️ Proposed refactor
export async function findMappingByRole( menuId: string, roleId: string, ): Promise<ReactionRoleMapping | null> { - return prisma.reactionRoleMapping.findFirst({ where: { menuId, roleId } }); + return prisma.reactionRoleMapping.findUnique({ where: { menuId_roleId: { menuId, roleId } } }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/role-menu/service.ts` around lines 56 - 61, The function findMappingByRole should use Prisma's findUnique for the (menuId, roleId) unique constraint: replace prisma.reactionRoleMapping.findFirst(...) in findMappingByRole with prisma.reactionRoleMapping.findUnique(...) and pass the composite unique key object (e.g. where: { menuId_roleId: { menuId, roleId } }) so the query uses the schema-defined unique identifier instead of findFirst.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/role-menu/service.ts`:
- Around line 20-26: The upsert call using only messageId can return a menu
belonging to a different guild; fix by verifying guildId before returning: first
fetch existing = await prisma.reactionRoleMenu.findUnique({ where: { messageId:
input.messageId }, include: { mappings: true } }); if existing and
existing.guildId !== input.guildId throw an authorization/validation error;
otherwise proceed to create or update (or call prisma.reactionRoleMenu.upsert as
before) and return the record. Reference the prisma.reactionRoleMenu.upsert call
and input.messageId/input.guildId in your changes.
---
Nitpick comments:
In `@src/features/role-menu/service.ts`:
- Around line 56-61: The function findMappingByRole should use Prisma's
findUnique for the (menuId, roleId) unique constraint: replace
prisma.reactionRoleMapping.findFirst(...) in findMappingByRole with
prisma.reactionRoleMapping.findUnique(...) and pass the composite unique key
object (e.g. where: { menuId_roleId: { menuId, roleId } }) so the query uses the
schema-defined unique identifier instead of findFirst.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 81497ed0-3133-42d8-9e96-94671fdf0220
📒 Files selected for processing (1)
src/features/role-menu/service.ts
Comment on lines
+20
to
+26
| return prisma.reactionRoleMenu.upsert({ | ||
| where: { messageId: input.messageId }, | ||
| update: {}, | ||
| create: input, | ||
| include: { mappings: true }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Verify guildId after upsert to prevent cross-guild access.
The upsert uses only messageId in the where clause and update: {}, so if a menu with the same messageId but different guildId already exists, it will return that menu without updating guildId. This violates guild isolation and could allow cross-guild access if a messageId from one guild is referenced in another guild's context.
While Discord message IDs are globally unique and downstream code filters by guildId, the function's contract (accepting guildId as input) suggests it should enforce guild matching.
🛡️ Proposed fix to add guild verification
export async function getOrCreateMenu(input: {
guildId: string;
channelId: string;
messageId: string;
}): Promise<MenuWithMappings> {
- return prisma.reactionRoleMenu.upsert({
+ const menu = await prisma.reactionRoleMenu.upsert({
where: { messageId: input.messageId },
update: {},
create: input,
include: { mappings: true },
});
+ if (menu.guildId !== input.guildId) {
+ throw new Error(`Message ${input.messageId} belongs to a different guild`);
+ }
+ return menu;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return prisma.reactionRoleMenu.upsert({ | |
| where: { messageId: input.messageId }, | |
| update: {}, | |
| create: input, | |
| include: { mappings: true }, | |
| }); | |
| } | |
| const menu = await prisma.reactionRoleMenu.upsert({ | |
| where: { messageId: input.messageId }, | |
| update: {}, | |
| create: input, | |
| include: { mappings: true }, | |
| }); | |
| if (menu.guildId !== input.guildId) { | |
| throw new Error(`Message ${input.messageId} belongs to a different guild`); | |
| } | |
| return menu; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/role-menu/service.ts` around lines 20 - 26, The upsert call
using only messageId can return a menu belonging to a different guild; fix by
verifying guildId before returning: first fetch existing = await
prisma.reactionRoleMenu.findUnique({ where: { messageId: input.messageId },
include: { mappings: true } }); if existing and existing.guildId !==
input.guildId throw an authorization/validation error; otherwise proceed to
create or update (or call prisma.reactionRoleMenu.upsert as before) and return
the record. Reference the prisma.reactionRoleMenu.upsert call and
input.messageId/input.guildId in your changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the
/reaction-rolecommand with a new/role-menucommand that supports buttons only.Changes
New command:
/role-menucreatechannelcontentaddmessagerole1-5label1-5removemessageroleeditmessagecontentdeletemessagelistRemoved
/reaction-rolecommand and all subcommandsemoji.ts,url.ts, reaction event listeners)GatewayIntentBits.GuildMessageReactions,Partials.Message,Partials.ReactionDB migration
emojicolumn fromreaction_role_mappingsbutton_idcolumn (now computed on-the-fly asrr:<menuId>:<roleId>)label TEXTcolumn (nullable, overrides role name on button)(menu_id, emoji, button_id)→(menu_id, role_id)Why buttons only
Button interactions are reliable through bot restarts. Reaction events are gateway-only and missed while the bot is offline.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Chores