refactor: index.tsのclient.onを分割し、マジックナンバーを削減 - #50
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request extracts the inline ChangesMessage Reaction Removal Handler Extraction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 2
🤖 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/handlers/events/onMessageReactionRemove.ts`:
- Around line 17-24: The handler references reactionRoleMessage from index but
that variable is no longer in scope; update the onMessageReactionRemove handler
(in onMessageReactionRemove.ts) to accept an extra parameter (e.g.,
reactionRoleMessage) alongside the existing parameters used by
removeReactionRole, update its exported function signature and types, and then
modify the event registration where you wire the handler (the
client.on("messageReactionRemove", ...) registration in index.ts) to pass
reactionRoleMessage into the handler via a closure (e.g., client.on(...,
(reaction, user) => onMessageReactionRemove(reaction, user,
reactionRoleMessage))). Ensure imports/exports for onMessageReactionRemove are
adjusted and any TypeScript types for member/reaction parameters remain correct.
- Line 8: The log in the onMessageReactionRemove handler is incorrect: update
the console.log call inside onMessageReactionRemove (currently logging
"messageReactionAdded") to a correct, descriptive message such as
"messageReactionRemoved" or "messageReactionRemove" so logs reflect reaction
removals; locate the console.log in onMessageReactionRemove and change only the
message string.
🪄 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: ee1da512-1047-48ba-821f-efa6faf8d316
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/handlers/events/onMessageReactionRemove.tssrc/index.tssrc/services/reactionRole/addReactionRole.tssrc/services/reactionRole/removeReactionRole.ts
| const message = reaction.message; | ||
| const member = message?.guild?.members.resolve(user.id); | ||
|
|
||
| console.log("[INFO] messageReactionAdded"); |
There was a problem hiding this comment.
Incorrect log message — copy-paste from the "add" handler.
The log says "messageReactionAdded" but this handler processes reaction removals.
✏️ Proposed fix
- console.log("[INFO] messageReactionAdded");
+ console.log("[INFO] messageReactionRemoved");📝 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.
| console.log("[INFO] messageReactionAdded"); | |
| console.log("[INFO] messageReactionRemoved"); |
🤖 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/handlers/events/onMessageReactionRemove.ts` at line 8, The log in the
onMessageReactionRemove handler is incorrect: update the console.log call inside
onMessageReactionRemove (currently logging "messageReactionAdded") to a correct,
descriptive message such as "messageReactionRemoved" or "messageReactionRemove"
so logs reflect reaction removals; locate the console.log in
onMessageReactionRemove and change only the message string.
| if (message.id === reactionRoleMessage) { | ||
| try { | ||
| await removeReactionRole(member, reaction.emoji.name); | ||
| } catch (e) { | ||
| console.error(e); | ||
| // この先通知処理も追加 | ||
| } | ||
| } |
There was a problem hiding this comment.
Shared root cause: reactionRoleMessage is inaccessible after extraction.
The handler in src/handlers/events/onMessageReactionRemove.ts references reactionRoleMessage (line 17), but this variable is defined in src/index.ts (line 51) and not passed to the handler when wired at line 301. Both files need coordinated changes: the handler signature must accept reactionRoleMessage as a parameter, and the event registration must pass it via a closure.
🤖 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/handlers/events/onMessageReactionRemove.ts` around lines 17 - 24, The
handler references reactionRoleMessage from index but that variable is no longer
in scope; update the onMessageReactionRemove handler (in
onMessageReactionRemove.ts) to accept an extra parameter (e.g.,
reactionRoleMessage) alongside the existing parameters used by
removeReactionRole, update its exported function signature and types, and then
modify the event registration where you wire the handler (the
client.on("messageReactionRemove", ...) registration in index.ts) to pass
reactionRoleMessage into the handler via a closure (e.g., client.on(...,
(reaction, user) => onMessageReactionRemove(reaction, user,
reactionRoleMessage))). Ensure imports/exports for onMessageReactionRemove are
adjusted and any TypeScript types for member/reaction parameters remain correct.
1b119c8 to
c59b0e5
Compare
sora81dev
changed the title
| import dotenv from "dotenv" | ||
|
|
||
| dotenv.config() | ||
|
|
||
| export const env = { | ||
| tokens: { | ||
| discordToken: process.env.DISCORD_TOKEN | ||
| } | ||
| } |
There was a problem hiding this comment.
| import dotenv from "dotenv" | |
| dotenv.config() | |
| export const env = { | |
| tokens: { | |
| discordToken: process.env.DISCORD_TOKEN | |
| } | |
| } | |
| import dotenv from "dotenv"; | |
| dotenv.config(); | |
| export const env = { | |
| tokens: { | |
| discordToken: process.env.DISCORD_TOKEN | |
| } | |
| } |
他のファイルの行末にはセミコロンがついているのにここだけない
There was a problem hiding this comment.
| await Promise.all([ | |
| addRoleSafely(member, "1454099602641780737", "bot"), // botロールを付与 | |
| addRoleSafely(member, "1454099602641780737", "student") // 学生ロールを付与 | |
| ]); |
並列実行で効率化
|
@tanahiro2010 |
973f6f1 to
1862bd3
Compare
chore chore
chore chore chore
2 participants
About this PR
保守性・視認性を向上させることを目的としたリファクタリングです。
長すぎる
index.tsを複数ファイルに分割client.onの連なりをhandlers/events/内に移動
configファイルの作成
env.tsの作成
複数箇所でenvファイルを読み込むのではなく、1カ所で読み、他ではimport
config.tsの作成
ファイル内で共通して使用する定数(ロールidなど)
runtimeConfig.tsの作成
roleMessageIdなどready時に読み込まれる定数を管理
jobsフォルダにあるべきでないファイルの移動
servicesフォルダを作成
Progress
❌: backlog
🔄: in Progress
✅: completed
configFiles
🔄 config.ts
🔄 env.ts
✅ runtimeConfig.ts
index.ts(main refactoring target)326 Lines -> 282 Lines
✅ reactionRole
❌ autoRole
❌ VC
❌ recruitNotification
jobsFolder
❌ updateMembercount
❌ noticeNewRecruit
servicesFolder
🔄 reactionRole
❌ autoRole
❌ VC
❌ recruitNotification
Summary by CodeRabbit