Improve logic of paytobuyer, setinvoice, settleorder, - #677
Conversation
|
""" WalkthroughOrder status validation logic has been updated in bot command handlers to refine when certain actions can occur. New localized user messages were added in multiple languages to clarify command usage restrictions based on order status. No exported function signatures or public interfaces were changed. Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (4)
locales/ru.yaml (1)
640-641: Translation consistent; minor stylistic tweak optional.The statuses (
FROZEN,PAID_HOLD_INVOICE,DISPUTE) are intentionally left untranslated for technical accuracy.
If you prefer full Russian localisation, consider wrapping the English status names in back-quotes to visually distinguish them (e.g. «статусFROZEN»). No functional impact—optional.locales/it.yaml (1)
637-638: Italian copy OK; watch plural agreement.“ordini con stato …” is plural, which matches the message, but the command acts on a single order.
Consider singular form for consistency with other locale strings:-Il comando paytobuyer può essere utilizzato solo su ordini con stato … +Il comando paytobuyer può essere utilizzato solo su un ordine con stato …Same for
settleorder_only_dispute_orders.locales/fr.yaml (1)
639-640: Minor wording nitpick – prefix commands with “/” for consistencyEverywhere else in the file, commands are shown with the leading slash (e.g.
/release).
Consider:-paytobuyer_only_frozen_orders: La commande paytobuyer ne peut être utilisée ... -settleorder_only_dispute_orders: La commande settleorder ne peut être utilisée ... +paytobuyer_only_frozen_orders: La commande /paytobuyer ne peut être utilisée ... +settleorder_only_dispute_orders: La commande /settleorder ne peut être utilisée ...Purely cosmetic, up to you.
bot/start.ts (1)
894-897: Consider error handling for status updateThe status update from
FROZENtoPAID_HOLD_INVOICEbefore callingpayToBuyeris logical, but there's a potential issue: ifpayToBuyerfails after the status update, the order will remain inPAID_HOLD_INVOICEstatus without successful payment completion.Consider wrapping the status update and payment in a transaction or adding error handling to revert the status if payment fails:
// Set status to PAID_HOLD_INVOICE before paying (only if not already PAID_HOLD_INVOICE) if (order.status === 'FROZEN') { order.status = 'PAID_HOLD_INVOICE'; await order.save(); } -await payToBuyer(bot, order); +try { + await payToBuyer(bot, order); +} catch (error) { + // Revert status if payment fails and order was originally FROZEN + if (order.status === 'PAID_HOLD_INVOICE') { + order.status = 'FROZEN'; + await order.save(); + } + throw error; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
bot/commands.ts(1 hunks)bot/start.ts(4 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)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{ts,tsx}`: Use custom context types that extend Telegraf's base context, such as MainContext and CommunityContext, for context enhancement.
**/*.{ts,tsx}: Use custom context types that extend Telegraf's base context, such as MainContext and CommunityContext, for context enhancement.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
bot/commands.tsbot/start.ts
`bot/start.ts`: Bot initialization, command registration, and scheduled jobs should be handled in bot/start.ts.
bot/start.ts: Bot initialization, command registration, and scheduled jobs should be handled in bot/start.ts.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
bot/start.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: webwarrior-ws
PR: lnp2pBot/bot#665
File: bot/commands.ts:801-803
Timestamp: 2025-06-23T08:18:45.934Z
Learning: In the IOrder interface from bot/commands.ts, the secret property is typed such that it cannot be undefined, only null or string. Therefore, checking only for null is sufficient and checking for undefined is unnecessary.
bot/commands.ts (3)
Learnt from: webwarrior-ws
PR: lnp2pBot/bot#665
File: bot/commands.ts:801-803
Timestamp: 2025-06-23T08:18:45.934Z
Learning: In the IOrder interface from bot/commands.ts, the secret property is typed such that it cannot be undefined, only null or string. Therefore, checking only for null is sufficient and checking for undefined is unnecessary.
Learnt from: webwarrior-ws
PR: lnp2pBot/bot#665
File: bot/commands.ts:801-803
Timestamp: 2025-06-23T08:18:45.934Z
Learning: In the IOrder interface from models/order.ts, the secret property is typed as `string | null`, which means it can never be undefined in TypeScript strict mode. Therefore, checking only for null is sufficient and checking for undefined is unnecessary.
Learnt from: webwarrior-ws
PR: lnp2pBot/bot#665
File: bot/commands.ts:267-270
Timestamp: 2025-06-23T08:19:23.336Z
Learning: In the IOrder interface, the hash property is typed as `string | null` (not including undefined), so checking specifically for null with `if (order.hash === null)` is the correct and most precise approach rather than using a falsy check like `if (!order.hash)`.
bot/start.ts (3)
Learnt from: CR
PR: lnp2pBot/bot#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T20:32:02.450Z
Learning: Applies to bot/start.ts : Bot initialization, command registration, and scheduled jobs should be handled in bot/start.ts.
Learnt from: webwarrior-ws
PR: lnp2pBot/bot#665
File: bot/commands.ts:801-803
Timestamp: 2025-06-23T08:18:45.934Z
Learning: In the IOrder interface from bot/commands.ts, the secret property is typed such that it cannot be undefined, only null or string. Therefore, checking only for null is sufficient and checking for undefined is unnecessary.
Learnt from: webwarrior-ws
PR: lnp2pBot/bot#665
File: bot/commands.ts:267-270
Timestamp: 2025-06-23T08:19:23.336Z
Learning: In the IOrder interface, the hash property is typed as `string | null` (not including undefined), so checking specifically for null with `if (order.hash === null)` is the correct and most precise approach rather than using a falsy check like `if (!order.hash)`.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci_to_main
- GitHub Check: Analyze (javascript)
🔇 Additional comments (11)
locales/en.yaml (1)
646-647: New keys look good; verify they’re actually referenced in handlers.Strings are clear, placeholders are not needed, and naming follows existing pattern.
Just double-check thatpaytobuyer_only_frozen_orders/settleorder_only_dispute_ordershave been wired into the correspondingctx.i18n.t()calls in the command handlers; otherwise the user will still get the fallback English text.locales/es.yaml (1)
642-643: LGTM! Clear user guidance for command restrictions.The Spanish localization strings properly communicate the new order status restrictions for the
/paytobuyerand/settleordercommands. The messages are clear and user-friendly.bot/commands.ts (1)
585-586: LGTM! Properly expands allowed order statuses.The expansion to include
'COMPLETED_BY_ADMIN'status correctly implements the PR requirement to allow the/setinvoicecommand for admin-completed orders. The comment accurately reflects the updated logic.locales/uk.yaml (1)
636-637: LGTM! Consistent Ukrainian localization for command restrictions.The Ukrainian localization strings correctly translate the command restriction messages, maintaining consistency with other language files. The translations are clear and properly formatted.
locales/de.yaml (1)
640-641: New keys added correctly & translation looks goodKeys match the handler names (
paytobuyer_only_frozen_orders,settleorder_only_dispute_orders) and the German wording is clear while keeping status constants untranslated.
No YAML-syntax or interpolation issues detected. 👍locales/ko.yaml (1)
636-638: LGTM – keys & translations providedAll three new keys are present, Hangul text is concise, and status constants are left in English as expected. YAML syntax valid.
locales/fa.yaml (1)
639-640: LGTM: Proper localization support for command restrictionsThe new Persian translations correctly communicate the usage restrictions for the
/paytobuyerand/settleordercommands, providing clear user feedback when commands are used inappropriately.locales/pt.yaml (1)
639-640: LGTM: Consistent multilingual supportThe Portuguese translations properly implement the command restriction messages, maintaining consistency with other locale files and providing appropriate user feedback.
bot/start.ts (3)
135-135: LGTM: Logical expansion of setinvoice query conditionAdding
COMPLETED_BY_ADMINstatus to the/setinvoicecommand query makes sense, allowing buyers to update their invoice even after admin completion. This aligns with the PR objectives and maintains consistency with theaddInvoicePHIfunction expansion mentioned in the AI summary.
499-502: LGTM: Proper status validation for settleorder commandThe restriction to only allow
/settleorderon orders withDISPUTEstatus is logical and provides appropriate user feedback. This ensures the command is used only in the correct context and prevents misuse.
869-872: LGTM: Appropriate status validation for paytobuyer commandThe validation correctly restricts
/paytobuyerto orders withFROZENorPAID_HOLD_INVOICEstatuses, which aligns with the expected workflow. The error message provides clear feedback to users about the command's usage requirements.
3 participants
/paytobuyercan only be used if the order status is FROZEN or PAID_HOLD_INVOICE/settleordercan only be used if the order status is DISPUTE/setinvoiceis also allowed if the order status is COMPLETED_BY_ADMINSummary by CodeRabbit
New Features
Bug Fixes
Localization