Block exchanges between user A to B #630
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request introduces a comprehensive user blocking feature for the Telegram bot. The changes span multiple files across different modules, adding functionality to block and unblock users, check blocking status, and prevent order interactions between blocked users. The implementation includes new models, message handling, command configurations, and localized messages across multiple languages to support user blocking scenarios. Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (4)
bot/modules/block/customContext.ts (1)
3-9: Add JSDoc comments to document the interface.Adding documentation will help other developers understand the purpose and usage of this interface.
+/** + * Extended context for block-related commands. + * Adds user information and internationalization support. + */ export interface CustomContext extends Context { user?: { id: string; tg_id: string; }, i18n?: any; }models/block.ts (1)
3-7: Add JSDoc comments to document the interface.Adding documentation will help other developers understand the purpose and usage of this interface.
+/** + * Represents a block relationship between two users. + * @interface IBlock + */ export interface IBlock extends Document { blocker_tg_id: string; blocked_tg_id: string; created_at: Date; }bot/modules/block/index.ts (1)
3-4: Use ES6 imports instead of require.For better type safety and consistency with TypeScript, use ES6 imports.
-const commands = require('./commands'); -const { userMiddleware } = require('../../middleware/user'); +import * as commands from './commands'; +import { userMiddleware } from '../../middleware/user';bot/modules/block/messages.ts (1)
6-12: Improve error handling by notifying the user.The function should notify the user when message sending fails, rather than silently logging the error.
Apply this diff to improve error handling:
const ordersInProcess = async (ctx: CustomContext) => { try { ctx.reply(ctx.i18n.t('orders_in_process')); } catch (error) { logger.error(error); + await ctx.reply(ctx.i18n.t('error_sending_message')).catch(() => {}); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
bot/messages.ts(2 hunks)bot/modules/block/commands.ts(1 hunks)bot/modules/block/customContext.ts(1 hunks)bot/modules/block/index.ts(1 hunks)bot/modules/block/messages.ts(1 hunks)bot/modules/orders/takeOrder.js(3 hunks)bot/start.ts(2 hunks)locales/de.yaml(1 hunks)locales/en.yaml(1 hunks)locales/es.yaml(1 hunks)locales/fa.yaml(1 hunks)locales/fr.yaml(1 hunks)locales/it.yaml(1 hunks)locales/ko.yaml(1 hunks)locales/pt.yaml(1 hunks)locales/ru.yaml(1 hunks)locales/uk.yaml(1 hunks)models/block.ts(1 hunks)models/index.ts(2 hunks)
🔇 Additional comments (14)
models/index.ts (1)
7-7: LGTM!The Block model import and export follow the established pattern in the file.
Also applies to: 16-16
bot/modules/block/messages.ts (4)
14-20: Same error handling issue as inordersInProcess.
22-28: Same error handling issue as inordersInProcess.
30-36: Same error handling issue as inordersInProcess.
47-53: Same error handling issue as inordersInProcess.bot/modules/orders/takeOrder.js (1)
89-99: Same code duplication issue as intakebuy.bot/start.ts (1)
32-32: LGTM!The BlockModule is correctly imported and configured in the appropriate order.
Also applies to: 261-261
bot/messages.ts (1)
758-778: LGTM! Well-structured blocking notification functions.The new functions follow consistent patterns with proper error handling, clear naming, and correct typing.
locales/ko.yaml (1)
632-638: LGTM! Clear and consistent blocking-related messages.The messages provide clear user feedback for various blocking scenarios and align well with the blocking feature implementation.
locales/ru.yaml (1)
637-643: LGTM! Clear and consistent blocking-related messages.The messages provide clear user feedback for various blocking scenarios and align well with the blocking feature implementation.
locales/uk.yaml (1)
633-639: LGTM! Ukrainian translations for user blocking functionality.The translations are accurate and maintain consistent meaning with other locale files. All required messages for the blocking feature are present.
locales/en.yaml (1)
638-644: LGTM! Base English messages for user blocking functionality.The messages are clear, concise, and provide good user feedback for the blocking feature.
locales/es.yaml (1)
637-643: LGTM! Spanish translations are accurate and well-formatted.The translations maintain consistent meaning with other language versions while using natural Spanish expressions.
locales/de.yaml (1)
637-643: LGTM! The message structure is complete and consistent.The messages cover all necessary user interactions for the blocking feature and follow a consistent style with other messages in the file.
| id: string; | ||
| tg_id: string; | ||
| }, | ||
| i18n?: any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any type with a more specific type for i18n.
Using any type reduces type safety. Consider defining a proper interface for the i18n object to catch potential type-related issues at compile time.
- i18n?: any;
+ i18n?: {
+ t: (key: string, params?: Record<string, unknown>) => string;
+ };📝 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.
| i18n?: any; | |
| i18n?: { | |
| t: (key: string, params?: Record<string, unknown>) => string; | |
| }; |
| const blockSchema = new Schema<IBlock>({ | ||
| blocker_tg_id: { type: String }, | ||
| blocked_tg_id: { type: String }, | ||
| created_at: { type: Date, default: Date.now }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add indexes and field validation to the schema.
The current schema could benefit from:
- Adding required field validation
- Adding indexes for efficient querying
- Adding a unique compound index to prevent duplicate blocks
const blockSchema = new Schema<IBlock>({
- blocker_tg_id: { type: String },
- blocked_tg_id: { type: String },
+ blocker_tg_id: { type: String, required: true, index: true },
+ blocked_tg_id: { type: String, required: true, index: true },
created_at: { type: Date, default: Date.now },
});
+
+// Prevent duplicate blocks
+blockSchema.index({ blocker_tg_id: 1, blocked_tg_id: 1 }, { unique: true });📝 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.
| const blockSchema = new Schema<IBlock>({ | |
| blocker_tg_id: { type: String }, | |
| blocked_tg_id: { type: String }, | |
| created_at: { type: Date, default: Date.now }, | |
| }); | |
| const blockSchema = new Schema<IBlock>({ | |
| blocker_tg_id: { type: String, required: true, index: true }, | |
| blocked_tg_id: { type: String, required: true, index: true }, | |
| created_at: { type: Date, default: Date.now }, | |
| }); | |
| // Prevent duplicate blocks | |
| blockSchema.index({ blocker_tg_id: 1, blocked_tg_id: 1 }, { unique: true }); |
| if (args.length !== 2) return next(); | ||
| commands.unblock(ctx, args[1]); | ||
| }); | ||
| bot.command('blocklist', userMiddleware, commands.blocklist); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to blocklist command.
Wrap the command execution in try-catch.
- bot.command('blocklist', userMiddleware, commands.blocklist);
+ bot.command('blocklist', userMiddleware, rateLimiter, async (ctx: CustomContext) => {
+ try {
+ await commands.blocklist(ctx);
+ } catch (error) {
+ console.error('Error in blocklist command:', error);
+ await ctx.reply(ctx.i18n?.t('blocklist_error') ?? 'Failed to fetch block list');
+ }
+ });📝 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.
| bot.command('blocklist', userMiddleware, commands.blocklist); | |
| bot.command('blocklist', userMiddleware, rateLimiter, async (ctx: CustomContext) => { | |
| try { | |
| await commands.blocklist(ctx); | |
| } catch (error) { | |
| console.error('Error in blocklist command:', error); | |
| await ctx.reply(ctx.i18n?.t('blocklist_error') ?? 'Failed to fetch block list'); | |
| } | |
| }); |
| bot.command('unblock', userMiddleware, async (ctx, next) => { | ||
| const args = ctx.message.text.split(' ') || []; | ||
| if (args.length !== 2) return next(); | ||
| commands.unblock(ctx, args[1]); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and input validation to unblock command.
Apply the same improvements as the block command.
- bot.command('unblock', userMiddleware, async (ctx, next) => {
+ bot.command('unblock', userMiddleware, rateLimiter, async (ctx: CustomContext, next) => {
const args = ctx.message.text.split(' ') || [];
if (args.length !== 2) return next();
- commands.unblock(ctx, args[1]);
+ const username = args[1];
+ if (!username.match(/^@?[\w\d]{5,32}$/)) {
+ await ctx.reply(ctx.i18n?.t('invalid_username_format') ?? 'Invalid username format');
+ return;
+ }
+ try {
+ await commands.unblock(ctx, username);
+ } catch (error) {
+ console.error('Error in unblock command:', error);
+ await ctx.reply(ctx.i18n?.t('unblock_error') ?? 'Failed to unblock user');
+ }
});📝 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.
| bot.command('unblock', userMiddleware, async (ctx, next) => { | |
| const args = ctx.message.text.split(' ') || []; | |
| if (args.length !== 2) return next(); | |
| commands.unblock(ctx, args[1]); | |
| }); | |
| bot.command('unblock', userMiddleware, rateLimiter, async (ctx: CustomContext, next) => { | |
| const args = ctx.message.text.split(' ') || []; | |
| if (args.length !== 2) return next(); | |
| const username = args[1]; | |
| if (!username.match(/^@?[\w\d]{5,32}$/)) { | |
| await ctx.reply(ctx.i18n?.t('invalid_username_format') ?? 'Invalid username format'); | |
| return; | |
| } | |
| try { | |
| await commands.unblock(ctx, username); | |
| } catch (error) { | |
| console.error('Error in unblock command:', error); | |
| await ctx.reply(ctx.i18n?.t('unblock_error') ?? 'Failed to unblock user'); | |
| } | |
| }); |
| bot.command('block', userMiddleware, async (ctx, next) => { | ||
| const args = ctx.message.text.split(' ') || []; | ||
| if (args.length !== 2) return next(); | ||
| commands.block(ctx, args[1]); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and input validation.
The command handler should:
- Include try-catch for error handling
- Validate username format
- Add rate limiting to prevent abuse
- bot.command('block', userMiddleware, async (ctx, next) => {
+ bot.command('block', userMiddleware, rateLimiter, async (ctx: CustomContext, next) => {
const args = ctx.message.text.split(' ') || [];
if (args.length !== 2) return next();
- commands.block(ctx, args[1]);
+ const username = args[1];
+ if (!username.match(/^@?[\w\d]{5,32}$/)) {
+ await ctx.reply(ctx.i18n?.t('invalid_username_format') ?? 'Invalid username format');
+ return;
+ }
+ try {
+ await commands.block(ctx, username);
+ } catch (error) {
+ console.error('Error in block command:', error);
+ await ctx.reply(ctx.i18n?.t('block_error') ?? 'Failed to block user');
+ }
});Committable suggestion skipped: line range outside the PR's diff.
| user_already_blocked: User is already blocked | ||
| user_blocked: User successfully blocked | ||
| user_unblocked: User successfully unblocked | ||
| blocklist_empty: You do not have any blocked user | ||
| orders_in_process: There are orders in process with this user | ||
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | ||
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you |
There was a problem hiding this comment.
Translate blocking messages to Persian/Farsi.
The new blocking-related messages are currently in English and need to be translated to Persian/Farsi to maintain consistency with the rest of the locale file.
| user_already_blocked: User is already blocked | ||
| user_blocked: User successfully blocked | ||
| user_unblocked: User successfully unblocked | ||
| blocklist_empty: You do not have any blocked user | ||
| orders_in_process: There are orders in process with this user | ||
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | ||
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you |
There was a problem hiding this comment.
Missing Portuguese translations for user blocking messages.
The new messages are currently in English and need to be translated to Portuguese to maintain consistency with the rest of the file.
Here are the messages that need translation:
-user_already_blocked: User is already blocked
-user_blocked: User successfully blocked
-user_unblocked: User successfully unblocked
-blocklist_empty: You do not have any blocked user
-orders_in_process: There are orders in process with this user
-user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker
-user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you
+user_already_blocked: Usuário já está bloqueado
+user_blocked: Usuário bloqueado com sucesso
+user_unblocked: Usuário desbloqueado com sucesso
+blocklist_empty: Você não tem nenhum usuário bloqueado
+orders_in_process: Existem ordens em andamento com este usuário
+user_order_is_blocked_by_user_taker: Você não pode aceitar esta oferta porque bloqueou seu criador
+user_taker_is_blocked_by_user_order: Você não pode aceitar esta oferta porque seu criador bloqueou você📝 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.
| user_already_blocked: User is already blocked | |
| user_blocked: User successfully blocked | |
| user_unblocked: User successfully unblocked | |
| blocklist_empty: You do not have any blocked user | |
| orders_in_process: There are orders in process with this user | |
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | |
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you | |
| user_already_blocked: Usuário já está bloqueado | |
| user_blocked: Usuário bloqueado com sucesso | |
| user_unblocked: Usuário desbloqueado com sucesso | |
| blocklist_empty: Você não tem nenhum usuário bloqueado | |
| orders_in_process: Existem ordens em andamento com este usuário | |
| user_order_is_blocked_by_user_taker: Você não pode aceitar esta oferta porque bloqueou seu criador | |
| user_taker_is_blocked_by_user_order: Você não pode aceitar esta oferta porque seu criador bloqueou você |
| user_already_blocked: User is already blocked | ||
| user_blocked: User successfully blocked | ||
| user_unblocked: User successfully unblocked | ||
| blocklist_empty: You do not have any blocked user | ||
| orders_in_process: There are orders in process with this user | ||
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | ||
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you |
There was a problem hiding this comment.
Missing Italian translations for user blocking messages.
The new messages are currently in English and need to be translated to Italian to maintain consistency with the rest of the file.
Here are the messages that need translation:
-user_already_blocked: User is already blocked
-user_blocked: User successfully blocked
-user_unblocked: User successfully unblocked
-blocklist_empty: You do not have any blocked user
-orders_in_process: There are orders in process with this user
-user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker
-user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you
+user_already_blocked: L'utente è già bloccato
+user_blocked: Utente bloccato con successo
+user_unblocked: Utente sbloccato con successo
+blocklist_empty: Non hai alcun utente bloccato
+orders_in_process: Ci sono ordini in corso con questo utente
+user_order_is_blocked_by_user_taker: Non puoi accettare questo ordine perché hai bloccato il suo creatore
+user_taker_is_blocked_by_user_order: Non puoi accettare questo ordine perché il suo creatore ti ha bloccato📝 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.
| user_already_blocked: User is already blocked | |
| user_blocked: User successfully blocked | |
| user_unblocked: User successfully unblocked | |
| blocklist_empty: You do not have any blocked user | |
| orders_in_process: There are orders in process with this user | |
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | |
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you | |
| user_already_blocked: L'utente è già bloccato | |
| user_blocked: Utente bloccato con successo | |
| user_unblocked: Utente sbloccato con successo | |
| blocklist_empty: Non hai alcun utente bloccato | |
| orders_in_process: Ci sono ordini in corso con questo utente | |
| user_order_is_blocked_by_user_taker: Non puoi accettare questo ordine perché hai bloccato il suo creatore | |
| user_taker_is_blocked_by_user_order: Non puoi accettare questo ordine perché il suo creatore ti ha bloccato |
| user_already_blocked: User is already blocked | ||
| user_blocked: User successfully blocked | ||
| user_unblocked: User successfully unblocked | ||
| blocklist_empty: You do not have any blocked user | ||
| orders_in_process: There are orders in process with this user | ||
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | ||
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you |
There was a problem hiding this comment.
Missing French translations for user blocking messages.
The new messages are currently in English and need to be translated to French to maintain consistency with the rest of the file.
Here are the messages that need translation:
-user_already_blocked: User is already blocked
-user_blocked: User successfully blocked
-user_unblocked: User successfully unblocked
-blocklist_empty: You do not have any blocked user
-orders_in_process: There are orders in process with this user
-user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker
-user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you
+user_already_blocked: L'utilisateur est déjà bloqué
+user_blocked: Utilisateur bloqué avec succès
+user_unblocked: Utilisateur débloqué avec succès
+blocklist_empty: Vous n'avez aucun utilisateur bloqué
+orders_in_process: Il y a des ordres en cours avec cet utilisateur
+user_order_is_blocked_by_user_taker: Vous ne pouvez pas accepter cette offre car vous avez bloqué son créateur
+user_taker_is_blocked_by_user_order: Vous ne pouvez pas accepter cette offre car son créateur vous a bloqué📝 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.
| user_already_blocked: User is already blocked | |
| user_blocked: User successfully blocked | |
| user_unblocked: User successfully unblocked | |
| blocklist_empty: You do not have any blocked user | |
| orders_in_process: There are orders in process with this user | |
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | |
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you | |
| user_already_blocked: L'utilisateur est déjà bloqué | |
| user_blocked: Utilisateur bloqué avec succès | |
| user_unblocked: Utilisateur débloqué avec succès | |
| blocklist_empty: Vous n'avez aucun utilisateur bloqué | |
| orders_in_process: Il y a des ordres en cours avec cet utilisateur | |
| user_order_is_blocked_by_user_taker: Vous ne pouvez pas accepter cette offre car vous avez bloqué son créateur | |
| user_taker_is_blocked_by_user_order: Vous ne pouvez pas accepter cette offre car son créateur vous a bloqué |
| user_already_blocked: User is already blocked | ||
| user_blocked: User successfully blocked | ||
| user_unblocked: User successfully unblocked | ||
| blocklist_empty: You do not have any blocked user | ||
| orders_in_process: There are orders in process with this user | ||
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | ||
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you |
There was a problem hiding this comment.
Translate the new messages to German to maintain consistency.
The new messages for user blocking functionality are in English while this file is for German translations.
Apply this diff to add German translations:
-user_already_blocked: User is already blocked
-user_blocked: User successfully blocked
-user_unblocked: User successfully unblocked
-blocklist_empty: You do not have any blocked user
-orders_in_process: There are orders in process with this user
-user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker
-user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you
+user_already_blocked: Benutzer ist bereits gesperrt
+user_blocked: Benutzer erfolgreich gesperrt
+user_unblocked: Benutzer erfolgreich entsperrt
+blocklist_empty: Du hast keine gesperrten Benutzer
+orders_in_process: Es gibt laufende Aufträge mit diesem Benutzer
+user_order_is_blocked_by_user_taker: Du kannst diesen Auftrag nicht annehmen, da du seinen Ersteller gesperrt hast
+user_taker_is_blocked_by_user_order: Du kannst diesen Auftrag nicht annehmen, da dich sein Ersteller gesperrt hat📝 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.
| user_already_blocked: User is already blocked | |
| user_blocked: User successfully blocked | |
| user_unblocked: User successfully unblocked | |
| blocklist_empty: You do not have any blocked user | |
| orders_in_process: There are orders in process with this user | |
| user_order_is_blocked_by_user_taker: You can't take this order because you blocked its maker | |
| user_taker_is_blocked_by_user_order: You can't take this order because its maker blocked you | |
| user_already_blocked: Benutzer ist bereits gesperrt | |
| user_blocked: Benutzer erfolgreich gesperrt | |
| user_unblocked: Benutzer erfolgreich entsperrt | |
| blocklist_empty: Du hast keine gesperrten Benutzer | |
| orders_in_process: Es gibt laufende Aufträge mit diesem Benutzer | |
| user_order_is_blocked_by_user_taker: Du kannst diesen Auftrag nicht annehmen, da du seinen Ersteller gesperrt hast | |
| user_taker_is_blocked_by_user_order: Du kannst diesen Auftrag nicht annehmen, da dich sein Ersteller gesperrt hat |
This PR closes #438
Sorry for being late!
I migrated the feature to TS, I didn't split the PR because it's difficult for me to split it in different but usable PR.
I'm open to feedback / suggestions for improvements or bugs
Summary by CodeRabbit
Release Notes: User Blocking Feature
New Features
Improvements
Localization