fix: add validation to reply greetings only on the general chat#142
fix: add validation to reply greetings only on the general chat#142fernanduandrade wants to merge 5 commits intohe4rt:3.xfrom
Conversation
WalkthroughAdds a new env entry Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.env.example (1)
79-79: Address static analysis hints for consistency.Consider the following cosmetic improvements:
- Add a blank line at the end of the file
- Move
DISCORD_GENERAL_CHAT_IDbeforeDISCORD_TOKENto maintain alphabetical ordering within the Discord configuration sectionApply this diff:
TWITCH_OAUTH_CLIENT_SECRET= TWITCH_OAUTH_REDIRECT_URI= -DISCORD_TOKEN= DISCORD_GENERAL_CHAT_ID= +DISCORD_TOKEN= +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)app-modules/bot-discord/src/Events/GreetingsEvent.php(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 79-79: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 79-79: [UnorderedKey] The DISCORD_GENEREAL_CHAT_ID key should go before the DISCORD_TOKEN key
(UnorderedKey)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app-modules/bot-discord/src/Events/GreetingsEvent.php (1)
30-33: Channel guard via config looks correct; consider normalizing types and logging misconfigurations.Reading the general chat ID from
config('services.discord.general_chat_id')and gating early keeps the greetings scoped to the intended channel and avoids the previousenv()/non‑strict comparison issues.Two small, optional refinements:
- Ensure
$message->channel_idand the config value are the same scalar type (both strings or both ints); otherwise!==will always fail. If there’s any chance of mismatch, normalize both sides (e.g. cast to string) before comparing:- $allowedChannel = config('services.discord.general_chat_id'); - if ($message->channel_id !== $allowedChannel) { + $allowedChannel = config('services.discord.general_chat_id'); + if ((string) $message->channel_id !== (string) $allowedChannel) { return; }
- Optionally, if
general_chat_idis not configured (null/empty), you could log a warning once so it’s clear why greetings are never sent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.env.example (1)
78-79: Reorder environment variables and add trailing newline.The static analysis tool flags two minor issues:
- DISCORD_GENERAL_CHAT_ID should be ordered alphabetically before DISCORD_TOKEN
- The file is missing a blank line at the end
Apply this diff to correct both issues:
TWITCH_OAUTH_SCOPES= TWITCH_OAUTH_CLIENT_ID= TWITCH_OAUTH_CLIENT_SECRET= TWITCH_OAUTH_REDIRECT_URI= -DISCORD_TOKEN= -DISCORD_GENERAL_CHAT_ID= +DISCORD_GENERAL_CHAT_ID= +DISCORD_TOKEN=Ensure you also add a newline after line 79.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.example(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 79-79: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 79-79: [UnorderedKey] The DISCORD_GENERAL_CHAT_ID key should go before the DISCORD_TOKEN key
(UnorderedKey)
Bug Fixes
fix
chores
Summary by CodeRabbit
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.