Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThe block/unblock command handlers and messages were updated to accept a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bot/modules/block/commands.ts (1)
6-16: Extract sharedusernameOrIdresolution to a helper.
blockandunblockduplicate the same parse/lookup flow. A shared resolver reduces drift and keeps validation consistent.♻️ Suggested refactor
+const resolveUserByUsernameOrId = async ( + ctx: MainContext, + usernameOrId: string, +) => { + if (usernameOrId.startsWith('@')) { + return User.findOne({ username: usernameOrId.substring(1) }); + } + + if (!/^\d+$/.test(usernameOrId)) { + await globalMessages.notFoundUserMessage(ctx); + return null; + } + + return User.findOne({ tg_id: usernameOrId }); +}; const block = async (ctx: MainContext, usernameOrId: string): Promise<void> => { - let userToBlock; - if (usernameOrId.startsWith('@')) { - userToBlock = await User.findOne({ username: usernameOrId.substring(1) }); - } else { - if (!/^\d+$/.test(usernameOrId)) { - await globalMessages.notFoundUserMessage(ctx); - return; - } - userToBlock = await User.findOne({ tg_id: usernameOrId }); - } + const userToBlock = await resolveUserByUsernameOrId(ctx, usernameOrId); const unblock = async ( ctx: MainContext, usernameOrId: string, ): Promise<void> => { - let userToUnblock; - if (usernameOrId.startsWith('@')) { - userToUnblock = await User.findOne({ username: usernameOrId.substring(1) }); - } else { - if (!/^\d+$/.test(usernameOrId)) { - await globalMessages.notFoundUserMessage(ctx); - return; - } - userToUnblock = await User.findOne({ tg_id: usernameOrId }); - } + const userToUnblock = await resolveUserByUsernameOrId(ctx, usernameOrId);Also applies to: 64-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bot/modules/block/commands.ts` around lines 6 - 16, The block() and unblock() functions duplicate the usernameOrId parsing and User lookup; extract that logic into a shared helper (e.g., resolveUserByUsernameOrId(usernameOrId, ctx)) that accepts the raw string and context, validates whether it starts with '@' or is an integer, performs User.findOne({ username: ... }) or User.findOne({ tg_id: ... }), and returns the found user (or null); update block and unblock to call this helper and handle a null result by invoking globalMessages.notFoundUserMessage(ctx) and returning, keeping all validation and error handling centralized in the new resolver.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bot/modules/block/commands.ts`:
- Around line 11-16: The code converts usernameOrId with Number(...) before
querying tg_id causing a type mismatch with the schema (tg_id is String);
instead validate usernameOrId is a digits-only string (e.g., /^\d+$/) and if
valid use the raw string when querying User.findOne({ tg_id: usernameOrId });
update both the block flow that sets userToBlock (variable) and the other
similar lookup near lines 72-77 to stop using Number(...) and call
globalMessages.notFoundUserMessage(ctx) when validation fails.
---
Nitpick comments:
In `@bot/modules/block/commands.ts`:
- Around line 6-16: The block() and unblock() functions duplicate the
usernameOrId parsing and User lookup; extract that logic into a shared helper
(e.g., resolveUserByUsernameOrId(usernameOrId, ctx)) that accepts the raw string
and context, validates whether it starts with '@' or is an integer, performs
User.findOne({ username: ... }) or User.findOne({ tg_id: ... }), and returns the
found user (or null); update block and unblock to call this helper and handle a
null result by invoking globalMessages.notFoundUserMessage(ctx) and returning,
keeping all validation and error handling centralized in the new resolver.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe15124c-c6f5-4141-a91d-48fe59686d0b
📒 Files selected for processing (1)
bot/modules/block/commands.ts
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bot/modules/block/index.ts`:
- Around line 19-26: The unblock handler currently calls commands.unblock
without awaiting or handling errors; update bot.command('unblock',
userMiddleware, ...) to await commands.unblock(ctx, args[1]) inside a try/catch,
keep the existing args.length check and messages.unblockUsage(ctx) for bad
input, and in the catch block log the error (e.g., console.error or
ctx.logger.error) and notify the user via a failure message (e.g.,
messages.unblockFailed(ctx) or a suitable messages.error fallback).
- Around line 10-17: The bot.command handler for 'block' is calling
commands.block without awaiting or handling errors; update the async handler
(the function passed to bot.command('block', userMiddleware, async ctx => { ...
})) to await commands.block(ctx, args[1]) and wrap that call in a try/catch
similar to the blocklist handler: on invalid args call messages.blockUsage(ctx)
as before, and inside the try call await commands.block(...); in the catch call
messages.blockFailed(ctx) or an appropriate error message using ctx and log the
error so promise rejections are handled.
In `@locales/de.yaml`:
- Around line 700-701: The localization string for unblock_usage has a typo:
update the value of the key unblock_usage so it reads "Verwendung: /unblock
`@Username` oder /unblock telegramId" (replace the erroneous "/block telegramId"
with "/unblock telegramId") to match the documented command; change the value
associated with unblock_usage accordingly.
In `@locales/es.yaml`:
- Around line 702-703: Fix the typo in the Spanish locale entry for
unblock_usage: change the text value of the key unblock_usage (in
locales/es.yaml) from "Uso: /unblock `@Username` o /block telegramId" to use
"/unblock telegramId" so it reads "Uso: /unblock `@Username` o /unblock
telegramId"; ensure the key name remains unblock_usage and only the message
string is corrected.
In `@locales/ko.yaml`:
- Around line 697-698: Fix the typo in the localization string keyed by
unblock_usage: replace the copied "/block telegramId" text with "/unblock
telegramId" so the string reads "사용법: /unblock `@Username` 또는 /unblock
telegramId"; update the same unblock_usage key in all 10 locale files where the
copy-paste error appears to ensure consistency.
In `@locales/pt.yaml`:
- Line 257: The Portuguese translation for the message identified by the string
"/fiatsent <order id> - O comprador indica que já enviou o dinero Fiat para o
vendedor" uses the Spanish word "dinero"; update that message to use the
Portuguese word "dinheiro" so it reads "/fiatsent <order id> - O comprador
indica que já enviou o dinheiro Fiat para o vendedor", preserving surrounding
punctuation and spacing.
- Around line 699-700: The Portuguese locale has a copy-paste typo: the key
unblock_usage currently reads "/block telegramId" instead of "/unblock
telegramId"; update the value for the YAML key unblock_usage to use "/unblock
telegramId" (matching block_usage's structure) so it reads "Uso: /unblock
`@Username` ou /unblock telegramId" and ensure block_usage remains unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0289fb9c-88e1-4c0c-929b-5456631425eb
📒 Files selected for processing (12)
bot/modules/block/index.tsbot/modules/block/messages.tslocales/de.yamllocales/en.yamllocales/es.yamllocales/fa.yamllocales/fr.yamllocales/it.yamllocales/ko.yamllocales/pt.yamllocales/ru.yamllocales/uk.yaml
✅ Files skipped from review due to trivial changes (6)
- locales/en.yaml
- locales/ru.yaml
- locales/it.yaml
- locales/fa.yaml
- locales/fr.yaml
- locales/uk.yaml
Fixes #701
Now the name of the parameter of the function that handles the block operations is changed to usernameOrId
new behaviour:
now command accepts:
/ban @TelegramUsername
and
/ban 12345 (telegram user id)
If usernameOrId starts with '@' then it is interpreted as an username
else it will attempt to handle it as an id
if its not a valid telegram id, then it displays a message to the user saying the user was not found
Summary by CodeRabbit
New Features
Bug Fixes
Documentation