Conversation
…tive scenes for managing and publishing orders.
…tive scenes for managing and publishing orders.
…tive scenes for managing and publishing orders.
WalkthroughAdds an order-templates feature: new OrderTemplate model, templates module (commands, messages, wizard scene), integration into bot startup and scene stage, and localization entries across multiple languages. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
models/order_template.ts (1)
16-33: Consider using Mongoose'stimestampsschema option for automatic timestamp management.While the current
pre('save')hook works for document creations and explicit.save()calls, Mongoose's built-intimestampsoption is more robust and automatically managescreated_atandupdated_atacross all persistence methods, including update queries. This is the recommended approach in Mongoose documentation and prevents potential inconsistencies.Suggested schema update
-const orderTemplateSchema = new Schema<IOrderTemplate>({ +const orderTemplateSchema = new Schema<IOrderTemplate>({ creator_id: { type: String, required: true, index: true }, type: { type: String, enum: ['buy', 'sell'], required: true }, fiat_code: { type: String, required: true }, fiat_amount: { type: [Number], required: true }, amount: { type: Number, default: 0 }, payment_method: { type: String, required: true }, price_from_api: { type: Boolean, default: true }, price_margin: { type: Number, default: 0 }, - created_at: { type: Date, default: Date.now }, - updated_at: { type: Date, default: Date.now }, -}); - -// Update updated_at on save -orderTemplateSchema.pre('save', function (next) { - this.updated_at = new Date(); - next(); -}); +}, { + timestamps: { + createdAt: 'created_at', + updatedAt: 'updated_at', + }, +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@models/order_template.ts` around lines 16 - 33, The schema currently manages timestamps manually with created_at/updated_at fields and a pre('save') hook on orderTemplateSchema; replace this with Mongoose's built-in timestamps by passing { timestamps: { createdAt: 'created_at', updatedAt: 'updated_at' } } to the Schema constructor, remove the explicit created_at/updated_at schema definitions and the orderTemplateSchema.pre('save') hook, and ensure the rest of the schema (creator_id, type, fiat_amount, etc.) remains unchanged so Mongoose will automatically maintain those timestamp fields across saves and update queries.
🤖 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/templates/commands.ts`:
- Around line 38-67: publishFromTemplate currently trusts the passed-in template
object and can publish another user's private template; instead resolve the
template inside publishFromTemplate by loading it with both _id and creator_id =
user._id (e.g. replace use of the incoming template object with a DB lookup
using { _id: template._id, creator_id: user._id }) before using template.type,
ordersActions.createOrder, or calling
publishBuyOrderMessage/publishSellOrderMessage; update the scene to pass only
the template ID (not a hydrated document) so publishFromTemplate enforces
ownership.
In `@bot/modules/templates/scenes.ts`:
- Around line 322-329: The current parsing of fiat ranges using
text.split('-').map(Number) allows malformed tokens (e.g., "100-", "-100") and
non-finite values; update the parsing in the handler that sets state.fiatAmount
to: split on '-', trim tokens, reject if more than 2 tokens or if all tokens are
empty, convert each token with Number(...) and validate Number.isFinite for each
numeric token, ensure at least one numeric token exists, and if two numbers are
provided enforce min <= max (swap or reject based on existing manual order
logic); prefer reusing the same validator function used by manual order creation
(or extract and call it) so both code paths share validation rules before
assigning state.fiatAmount.
- Around line 171-177: The delete confirmation handler for callbacks starting
with 'tpl_list_confirm_delete_' currently calls OrderTemplate.deleteOne({ _id:
id }) which lets a tampered id delete another user's template; change the filter
to include the current user (use creator_id: state.user._id) when calling
OrderTemplate.deleteOne/remove and capture the result, only send the success
reply (templatesMessages.templateDeletedMessage) and advance the wizard when
result.deletedCount === 1 (otherwise reply with a not-found/permission error);
keep the existing ctx.answerCbQuery() and wizard reset logic but gate it on the
delete result to ensure only the owner can delete.
In `@locales/en.yaml`:
- Line 706: Update the localization string for the key image_processing_error in
locales/en.yaml to correct the grammar: change "wait few minutes" to "wait a few
minutes" so the full message reads "We had an error processing the image, please
wait a few minutes and try again"; locate and edit the image_processing_error
entry accordingly.
In `@locales/ko.yaml`:
- Line 714: The Korean localization string for the key template_card contains
the English word "for"; update the value so it is fully Korean by replacing
"for" with the appropriate Korean phrase (e.g., "에 대해") and ensure
spacing/punctuation remains correct; modify the string for template_card to
something like "${action} ${amountStr} ${fiatAmount} ${fiatCode}에 대해 결제:
${paymentMethod}.${rateStr}" so only Korean remains.
In `@locales/ru.yaml`:
- Line 713: The template_summary localization string uses Handlebars-style
placeholders {{...}} which won't be interpolated by `@grammyjs/i18n`; update the
template_summary value in each locale (keys: template_summary in en, es, fa, ko,
pt, ru, uk, de, fr, it) to use `${type}`, `${fiatCode}`, `${fiatAmount}`,
`${priceMode}`, `${margin}` and `${paymentMethod}` (same placeholder names
already used in template_card) so the i18n engine will substitute values
correctly.
---
Nitpick comments:
In `@models/order_template.ts`:
- Around line 16-33: The schema currently manages timestamps manually with
created_at/updated_at fields and a pre('save') hook on orderTemplateSchema;
replace this with Mongoose's built-in timestamps by passing { timestamps: {
createdAt: 'created_at', updatedAt: 'updated_at' } } to the Schema constructor,
remove the explicit created_at/updated_at schema definitions and the
orderTemplateSchema.pre('save') hook, and ensure the rest of the schema
(creator_id, type, fiat_amount, etc.) remains unchanged so Mongoose will
automatically maintain those timestamp fields across saves and update queries.
🪄 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: 4b7ec0da-7101-44fb-9a52-fbb094ee6103
📒 Files selected for processing (18)
bot/middleware/stage.tsbot/modules/templates/commands.tsbot/modules/templates/index.tsbot/modules/templates/messages.tsbot/modules/templates/scenes.tsbot/start.tslocales/de.yamllocales/en.yamllocales/es.yamllocales/fa.yamllocales/fr.yamllocales/it.yamllocales/ko.yamllocales/pt.yamllocales/ru.yamllocales/uk.yamlmodels/index.tsmodels/order_template.ts
| export const publishFromTemplate = async (ctx: MainContext, template: any) => { | ||
| try { | ||
| const user = ctx.user || (await User.findOne({ tg_id: ctx.from?.id })); | ||
| if (!user) return; | ||
| if (await isMaxPending(user)) { | ||
| return await tooManyPendingOrdersMessage(ctx, user, ctx.i18n); | ||
| } | ||
|
|
||
| const order = await ordersActions.createOrder( | ||
| ctx.i18n, | ||
| ctx as any as HasTelegram, | ||
| user, | ||
| { | ||
| type: template.type, | ||
| amount: template.amount || 0, | ||
| fiatAmount: template.fiat_amount, | ||
| fiatCode: template.fiat_code, | ||
| paymentMethod: template.payment_method, | ||
| status: 'PENDING', | ||
| priceMargin: template.price_margin, | ||
| community_id: user.default_community_id, | ||
| }, | ||
| ); | ||
|
|
||
| if (order) { | ||
| const publishFn = | ||
| template.type === 'buy' | ||
| ? publishBuyOrderMessage | ||
| : publishSellOrderMessage; | ||
| await publishFn(ctx as any, user, order, ctx.i18n, true); |
There was a problem hiding this comment.
Re-check ownership inside publishFromTemplate.
This helper publishes whichever template object it receives. Line 130 in bot/modules/templates/scenes.ts currently loads the template by _id only, so a crafted callback can publish another user's private template. Resolve the template here with { _id, creator_id: user._id } instead of trusting the caller, and have the scene pass the template ID rather than a hydrated document.
🔒 Suggested direction
-export const publishFromTemplate = async (ctx: MainContext, template: any) => {
+export const publishFromTemplate = async (
+ ctx: MainContext,
+ templateId: string,
+) => {
try {
const user = ctx.user || (await User.findOne({ tg_id: ctx.from?.id }));
if (!user) return;
+ const template = await OrderTemplate.findOne({
+ _id: templateId,
+ creator_id: user._id,
+ });
+ if (!template) return;
if (await isMaxPending(user)) {
return await tooManyPendingOrdersMessage(ctx, user, ctx.i18n);
}📝 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.
| export const publishFromTemplate = async (ctx: MainContext, template: any) => { | |
| try { | |
| const user = ctx.user || (await User.findOne({ tg_id: ctx.from?.id })); | |
| if (!user) return; | |
| if (await isMaxPending(user)) { | |
| return await tooManyPendingOrdersMessage(ctx, user, ctx.i18n); | |
| } | |
| const order = await ordersActions.createOrder( | |
| ctx.i18n, | |
| ctx as any as HasTelegram, | |
| user, | |
| { | |
| type: template.type, | |
| amount: template.amount || 0, | |
| fiatAmount: template.fiat_amount, | |
| fiatCode: template.fiat_code, | |
| paymentMethod: template.payment_method, | |
| status: 'PENDING', | |
| priceMargin: template.price_margin, | |
| community_id: user.default_community_id, | |
| }, | |
| ); | |
| if (order) { | |
| const publishFn = | |
| template.type === 'buy' | |
| ? publishBuyOrderMessage | |
| : publishSellOrderMessage; | |
| await publishFn(ctx as any, user, order, ctx.i18n, true); | |
| export const publishFromTemplate = async ( | |
| ctx: MainContext, | |
| templateId: string, | |
| ) => { | |
| try { | |
| const user = ctx.user || (await User.findOne({ tg_id: ctx.from?.id })); | |
| if (!user) return; | |
| const template = await OrderTemplate.findOne({ | |
| _id: templateId, | |
| creator_id: user._id, | |
| }); | |
| if (!template) return; | |
| if (await isMaxPending(user)) { | |
| return await tooManyPendingOrdersMessage(ctx, user, ctx.i18n); | |
| } | |
| const order = await ordersActions.createOrder( | |
| ctx.i18n, | |
| ctx as any as HasTelegram, | |
| user, | |
| { | |
| type: template.type, | |
| amount: template.amount || 0, | |
| fiatAmount: template.fiat_amount, | |
| fiatCode: template.fiat_code, | |
| paymentMethod: template.payment_method, | |
| status: 'PENDING', | |
| priceMargin: template.price_margin, | |
| community_id: user.default_community_id, | |
| }, | |
| ); | |
| if (order) { | |
| const publishFn = | |
| template.type === 'buy' | |
| ? publishBuyOrderMessage | |
| : publishSellOrderMessage; | |
| await publishFn(ctx as any, user, order, ctx.i18n, true); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bot/modules/templates/commands.ts` around lines 38 - 67, publishFromTemplate
currently trusts the passed-in template object and can publish another user's
private template; instead resolve the template inside publishFromTemplate by
loading it with both _id and creator_id = user._id (e.g. replace use of the
incoming template object with a DB lookup using { _id: template._id, creator_id:
user._id }) before using template.type, ordersActions.createOrder, or calling
publishBuyOrderMessage/publishSellOrderMessage; update the scene to pass only
the template ID (not a hydrated document) so publishFromTemplate enforces
ownership.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
locales/en.yaml (1)
706-706:⚠️ Potential issue | 🟡 MinorDuplicate: grammar issue in
image_processing_erroris still present.Line 706 still uses “wait few minutes” instead of “wait a few minutes.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@locales/en.yaml` at line 706, The localization string image_processing_error in locales/en.yaml contains a grammar mistake; update its value to "We had an error processing the image, please wait a few minutes and try again" (insert "a" before "few minutes") so the message reads correctly; locate the image_processing_error key and replace the existing text accordingly.
🧹 Nitpick comments (1)
locales/fa.yaml (1)
258-258: Keep placeholder casing consistent in/buyhelp text.At Line 258,
Fiatis capitalized (<_Fiat code_>) while nearby command docs use lowercase (<_fiat code_>). Consistent casing improves readability.Suggested tweak
- /buy <_sats amount_> <_fiat amount_> <_Fiat code_> <_payment method_> [premium/discount] - یک سفارش خرید ایجاد میکند + /buy <_sats amount_> <_fiat amount_> <_fiat code_> <_payment method_> [premium/discount] - یک سفارش خرید ایجاد میکند🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@locales/fa.yaml` at line 258, Update the /buy help text in the locales/fa.yaml entry so the placeholder casing is consistent: change the placeholder "<_Fiat code_>" to "<_fiat code_>" in the string that begins with "/buy <_sats amount_> <_fiat amount_> <_Fiat code_> <_payment method_> [premium/discount] - یک سفارش خرید ایجاد میکند" so all fiat placeholders use lowercase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@locales/en.yaml`:
- Line 576: Fix the grammar in the locale key dispute_too_soon by updating the
string to include the article “a” (i.e., change "wait few minutes" to "wait a
few minutes") so the message reads: You can't start a dispute too soon, be
patient and wait a few minutes for your counterparty to reply; update the value
for dispute_too_soon accordingly.
---
Duplicate comments:
In `@locales/en.yaml`:
- Line 706: The localization string image_processing_error in locales/en.yaml
contains a grammar mistake; update its value to "We had an error processing the
image, please wait a few minutes and try again" (insert "a" before "few
minutes") so the message reads correctly; locate the image_processing_error key
and replace the existing text accordingly.
---
Nitpick comments:
In `@locales/fa.yaml`:
- Line 258: Update the /buy help text in the locales/fa.yaml entry so the
placeholder casing is consistent: change the placeholder "<_Fiat code_>" to
"<_fiat code_>" in the string that begins with "/buy <_sats amount_> <_fiat
amount_> <_Fiat code_> <_payment method_> [premium/discount] - یک سفارش خرید
ایجاد میکند" so all fiat placeholders use lowercase.
🪄 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: ba3f7ed0-26ad-456d-a682-016875920c6d
📒 Files selected for processing (10)
locales/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 (1)
- locales/ru.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- locales/ko.yaml
- locales/it.yaml
…y tokens, non-finite values, zeros, and invalid bounds
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
bot/modules/templates/scenes.ts (2)
171-177:⚠️ Potential issue | 🔴 CriticalRestrict confirmed deletes to the template owner.
This callback still deletes by
_idonly, so a tampered confirm payload can remove another user’s private template. Includecreator_id: state.user._idin the filter, and only send the success reply/reset whendeletedCount === 1.🔒 Suggested fix
- await OrderTemplate.deleteOne({ _id: id }); - await ctx.reply(templatesMessages.templateDeletedMessage(ctx.i18n)); - ctx.wizard.cursor = 0; - return (ctx.wizard as any).steps[0](ctx); + const deleted = await OrderTemplate.deleteOne({ + _id: id, + creator_id: state.user._id, + }); + if (deleted.deletedCount !== 1) return; + await ctx.reply(templatesMessages.templateDeletedMessage(ctx.i18n)); + ctx.wizard.cursor = 0; + return (ctx.wizard as any).steps[0](ctx);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bot/modules/templates/scenes.ts` around lines 171 - 177, The delete callback currently calls OrderTemplate.deleteOne({_id: id}) allowing tampered payloads to delete others' templates; change the deletion filter to include the owner by using OrderTemplate.deleteOne({ _id: id, creator_id: state.user._id }) (use the same state.user._id available in the scene), then inspect the result.deletedCount and only send templatesMessages.templateDeletedMessage(ctx.i18n) and reset ctx.wizard.cursor/return (ctx.wizard as any).steps[0](ctx) when deletedCount === 1; if deletedCount !== 1, respond with an appropriate failure/permission message instead.
421-435:⚠️ Potential issue | 🟠 MajorReject invalid sats input before it can flip into market-price flow.
This branch still accepts non-finite values, and inputs like
0or0.5are floored to0, which silently routes the wizard through the “market price” path. Require a positive finite integer before assigningstate.amount.💸 Suggested fix
- const input = Number(ctx.message.text); + const input = Number(ctx.message.text.trim()); await ctx.deleteMessage().catch(() => {}); - if (isNaN(input)) { + if (!Number.isFinite(input)) { state.error = ctx.i18n.t('not_number'); await state.updateUI?.(); return; } if (input < 0) { state.error = ctx.i18n.t('not_negative'); await state.updateUI?.(); return; } - state.amount = Math.floor(input); + if (!Number.isInteger(input)) { + state.error = ctx.i18n.t('not_number'); + await state.updateUI?.(); + return; + } + if (input === 0) { + state.error = ctx.i18n.t('not_zero'); + await state.updateUI?.(); + return; + } + state.amount = input;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bot/modules/templates/scenes.ts` around lines 421 - 435, The branch that parses numeric input from ctx.message.text currently accepts non-finite values and zeros which get floored to 0 and trigger the wrong "market-price" flow; update the validation in this block (the code that converts ctx.message.text to Number, checks isNaN, non-negative, and sets state.amount) to require a positive finite integer: after parsing, ensure Number.isFinite(input) and input >= 1 (or Math.floor(input) >= 1) before setting state.amount, otherwise set state.error via ctx.i18n.t('not_number' or a new 'must_be_positive_integer') and call state.updateUI?.() and return so invalid inputs are rejected.
🤖 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/templates/scenes.ts`:
- Around line 533-557: Trim and validate the user-provided payment method before
persisting: take state.method, replace it with a trimmed value, then run the
same payment-method validator used for manual order creation (e.g.
validatePaymentMethod or the project’s payment method validation function) and
handle invalid input by sending an error/validation message and aborting
template creation; only after the trimmed value passes validation should you
populate templateData.payment_method and call new OrderTemplate(...) and
template.save(). Ensure you still delete the prompt (state.promptId) and call
state.updateUI as before when aborting or succeeding.
- Around line 60-95: The handler currently loads all OrderTemplate records and
sends one message per template (using OrderTemplate.find and
templatesMessages.singleTemplateData), causing an N-message dump; change it to
render a single paginated page: fetch only a page of templates via a limited
query (use .limit/.skip or a pagination helper) based on a page index in state
(e.g., state.tplPage), build the message list for that page only, and send a
single combined message or a small set for that page with navigation buttons
(Next/Prev) using templatesMessages.newTemplateButtonData (or a new paginator
message) so only page-sized results are emitted and state.listMessageIds still
tracks the messages to delete.
- Around line 127-147: The current publish handler uses
OrderTemplate.findById(id) allowing a forged callback to target another user's
template; change the lookup to scope by both _id and creator_id (e.g. use
OrderTemplate.findOne({ _id: id, creator_id: state.user._id })) so only
templates owned by the current user are returned, check that the returned
template exists before calling templatesCommands.publishFromTemplate(ctx,
template), and only call ctx.scene.leave() after a successful match/operation
(do not leave the scene when no template is found).
---
Duplicate comments:
In `@bot/modules/templates/scenes.ts`:
- Around line 171-177: The delete callback currently calls
OrderTemplate.deleteOne({_id: id}) allowing tampered payloads to delete others'
templates; change the deletion filter to include the owner by using
OrderTemplate.deleteOne({ _id: id, creator_id: state.user._id }) (use the same
state.user._id available in the scene), then inspect the result.deletedCount and
only send templatesMessages.templateDeletedMessage(ctx.i18n) and reset
ctx.wizard.cursor/return (ctx.wizard as any).steps[0](ctx) when deletedCount ===
1; if deletedCount !== 1, respond with an appropriate failure/permission message
instead.
- Around line 421-435: The branch that parses numeric input from
ctx.message.text currently accepts non-finite values and zeros which get floored
to 0 and trigger the wrong "market-price" flow; update the validation in this
block (the code that converts ctx.message.text to Number, checks isNaN,
non-negative, and sets state.amount) to require a positive finite integer: after
parsing, ensure Number.isFinite(input) and input >= 1 (or Math.floor(input) >=
1) before setting state.amount, otherwise set state.error via
ctx.i18n.t('not_number' or a new 'must_be_positive_integer') and call
state.updateUI?.() and return so invalid inputs are rejected.
🪄 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: 75b322a5-9f6a-4343-8ae6-9b99921c6ade
📒 Files selected for processing (1)
bot/modules/templates/scenes.ts
Add reusable order templates
/templatescommandSummary
Allow each user to store multiple reusable order templates and publish them quickly via the
/templatescommand, avoiding re-entering repetitive fields.Goal
Enable one-step publishing of repetitive orders (button or command) to improve speed and UX.
Implemented behavior
Commands
/templates command
Publish flow / UI
Code changes
bot/modules/templates/*commands.ts:/templatescommand, interaction handling, pagination.scenes.ts: guided flows for create/edit/publish; UI cleanup and message tracking fixes.index.ts,messages.ts: helpers and localized texts.ko.yamland others).Notes
Summary by CodeRabbit
New Features
Localization
Integration