Add interactive wizard for /dispute (button-based order selection) #731
Conversation
…heck in dispute command.
WalkthroughExtracts dispute initiation logic into a new handler Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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
🤖 Fix all issues with AI agents
In `@bot/modules/dispute/commands.ts`:
- Around line 116-122: In the conditional in the command handler, remove the
redundant "return" statements after the awaited calls—specifically eliminate the
trailing returns after messages.listOrdersForDispute(ctx, orders) and after
ctx.reply(ctx.i18n.t('you_have_no_orders'))—so the branch ends with the awaited
calls only; keep the if/else logic intact and ensure no other control flow needs
those returns.
In `@bot/modules/dispute/messages.ts`:
- Line 4: The file imports the symbol User but never uses it; remove User from
the import statement (the import line that currently reads "import { User } from
...") so the file no longer imports the unused symbol and run/verify linting to
ensure no remaining unused-import warnings.
🧹 Nitpick comments (3)
locales/it.yaml (1)
188-198: Consider translating remaining English strings inorder_detail.Several labels within
order_detailremain in English:
- Line 188:
Days in operation \\(buyer\\)- Line 190:
Successful trades \\(buyer\\)- Line 196:
Days in operation \\(seller\\)- Line 198:
Successful trades \\(seller\\)For consistency with the rest of the Italian locale, these should be translated.
bot/modules/dispute/actions.ts (1)
58-64: LGTM with minor suggestion.The implementation is clean and properly delegates to
handleDispute. For consistency withtakeDisputeabove (which has explicitPromise<void>return type), consider adding the same type annotation.Suggested improvement
-export const initiateDispute = async (ctx: MainContext) => { +export const initiateDispute = async (ctx: MainContext): Promise<void> => {bot/modules/dispute/messages.ts (1)
8-32: Consider simplifying the button mapping.The function works correctly, but
Promise.allwithasynccallback is unnecessary here since there are no async operations inside the map. This can be simplified:Suggested refactor
export const listOrdersForDispute = async ( ctx: MainContext, orders: IOrder[], ) => { try { - const buttons = await Promise.all( - orders.map(async order => { - return [ - { - text: `${order._id.toString().substring(0, 2)}..${order._id.toString().slice(-2)} - ${order.type} - ${order.fiat_code} ${order.fiat_amount}`, - callback_data: `initiateDispute_${order._id}`, - }, - ]; - }), - ); + const buttons = orders.map(order => [ + { + text: `${order._id.toString().substring(0, 2)}..${order._id.toString().slice(-2)} - ${order.type} - ${order.fiat_code} ${order.fiat_amount}`, + callback_data: `initiateDispute_${order._id}`, + }, + ]); await ctx.reply(ctx.i18n.t('choose_order_for_dispute'), {
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/modules/dispute/commands.ts (1)
23-31:⚠️ Potential issue | 🟠 MajorValidate
DISPUTE_START_WINDOWto avoid bypassing the time gate.
parseIntcan yieldNaN(or a negative value), which makes the comparison againstorder.taken_atfail open and allows immediate disputes. Validate the parsed value before using it.🔧 Suggested fix
- const disputStartWindow = process.env.DISPUTE_START_WINDOW; - if (disputStartWindow === undefined) + const disputeStartWindow = process.env.DISPUTE_START_WINDOW; + if (disputeStartWindow === undefined) throw new Error('DISPUTE_START_WINDOW environment variable not defined'); - const secsUntilDispute = parseInt(disputStartWindow); + const secsUntilDispute = Number.parseInt(disputeStartWindow, 10); + if (!Number.isFinite(secsUntilDispute) || secsUntilDispute < 0) { + throw new Error('DISPUTE_START_WINDOW must be a non-negative integer'); + } const time = new Date(); time.setSeconds(time.getSeconds() - secsUntilDispute);
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@bot/modules/dispute/commands.ts`:
- Around line 97-100: The conditional in the dispute command uses
ctx.state.command.args.length and then checks ctx.state.command.args[0] !==
'undefined' which tests the literal string "undefined" (redundant since length>0
proves an element exists); either remove the "'undefined'" comparison and rely
on the length check, or if you actually intend to treat the user typing the word
"undefined" as invalid, add a clarifying comment above this condition explaining
that special-case; update the condition in the handler that reads
ctx.state.command.args (the if block containing args.length and args[0] !==
'undefined') accordingly.
🧹 Nitpick comments (4)
bot/modules/dispute/commands.ts (4)
54-61: UsecountDocuments()instead of deprecatedcount().Mongoose 6.x deprecated
Model.count()in favor ofcountDocuments(). Since this project uses mongoose 6.13.6, update these calls to avoid deprecation warnings.Proposed fix
const buyerDisputes = - (await Dispute.count({ + (await Dispute.countDocuments({ $or: [{ buyer_id: buyer._id }, { seller_id: buyer._id }], })) + 1; const sellerDisputes = - (await Dispute.count({ + (await Dispute.countDocuments({ $or: [{ buyer_id: seller._id }, { seller_id: seller._id }], })) + 1;
38-38: Use strict equality or.equals()for ObjectId comparison.Comparing Mongoose ObjectIds with
==may produce unexpected results. Prefer===with.toString()or use the.equals()method for reliable comparison.Proposed fix
- if (user._id == order.buyer_id) initiator = 'buyer'; + if (user._id.equals(order.buyer_id)) initiator = 'buyer';
23-23: Minor typo in variable name.
disputStartWindowis missing an 'e' — should bedisputeStartWindowfor clarity.Proposed fix
- const disputStartWindow = process.env.DISPUTE_START_WINDOW; - if (disputStartWindow === undefined) + const disputeStartWindow = process.env.DISPUTE_START_WINDOW; + if (disputeStartWindow === undefined) throw new Error('DISPUTE_START_WINDOW environment variable not defined'); - const secsUntilDispute = parseInt(disputStartWindow); + const secsUntilDispute = parseInt(disputeStartWindow);
108-114: Inconsistent ObjectId query pattern — consider removing.toString().The query uses
user._id.toString()for comparisons, but throughout the codebase (bot/validations.ts, bot/start.ts, bot/ordersActions.ts, and bot/modules/orders/takeOrder.ts) all other seller_id and buyer_id queries useuser._iddirectly without conversion. For consistency, remove the explicit string conversion.Suggested change
const orders = await Order.find({ $or: [ - { seller_id: user._id.toString() }, - { buyer_id: user._id.toString() }, + { seller_id: user._id }, + { buyer_id: user._id }, ], status: { $in: ['ACTIVE', 'FIAT_SENT'] }, });
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/modules/dispute/commands.ts (1)
54-61:⚠️ Potential issue | 🟡 MinorReplace deprecated
Model.count()withcountDocuments().Mongoose 6.x deprecated
Model.count()in favor ofcountDocuments(). Since both usages here include filters, usecountDocuments()to avoid deprecation warnings.Proposed fix
const buyerDisputes = - (await Dispute.count({ + (await Dispute.countDocuments({ $or: [{ buyer_id: buyer._id }, { seller_id: buyer._id }], })) + 1; const sellerDisputes = - (await Dispute.count({ + (await Dispute.countDocuments({ $or: [{ buyer_id: seller._id }, { seller_id: seller._id }], })) + 1;
🧹 Nitpick comments (1)
bot/modules/dispute/commands.ts (1)
23-26: Typo in variable name:disputStartWindow→disputeStartWindow.Minor typo - missing 'e' in the variable name.
Proposed fix
- const disputStartWindow = process.env.DISPUTE_START_WINDOW; - if (disputStartWindow === undefined) + const disputeStartWindow = process.env.DISPUTE_START_WINDOW; + if (disputeStartWindow === undefined) throw new Error('DISPUTE_START_WINDOW environment variable not defined'); - const secsUntilDispute = parseInt(disputStartWindow); + const secsUntilDispute = parseInt(disputeStartWindow);
3 participants
Adds an interactive wizard for /dispute so users can pick their order via buttons instead of typing /dispute , reducing errors and friction.
Summary by CodeRabbit