Added community name to checkorder command output #737
Conversation
…and restricted 'default' as a community name
WalkthroughThe PR converts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/modules/community/scenes.ts (1)
133-137:⚠️ Potential issue | 🟡 MinorCase-insensitive check and specific error message needed.
The check
name === 'default'is case-sensitive, butupdateNameCommunityWizard(line 519) and the model layer both use case-insensitive comparison. Also, when the name is "default", users see the generic prompt instead of the reserved-name message.🔧 Proposed fix for consistency
- if (!name || name === 'default') { + if (!name) { ctx.telegram.deleteMessage(ctx.chat!.id, ctx.message!.message_id); ctx.wizard.state.error = ctx.i18n.t('wizard_community_enter_name'); return await ctx.wizard.state.updateUI(); } + if (name.toLowerCase() === 'default') { + ctx.telegram.deleteMessage(ctx.chat!.id, ctx.message!.message_id); + ctx.wizard.state.error = ctx.i18n.t('community_name_default_reserved'); + return await ctx.wizard.state.updateUI(); + }
🤖 Fix all issues with AI agents
In `@bot/messages.ts`:
- Around line 1702-1707: The admin message currently uses the result of
getDetailedOrder and can end up rendering "undefined" if that call fails; before
rendering the admin template in bot/messages.ts (around the usage of
detailedOrder), add a guard that assigns a safe fallback when detailedOrder is
falsy (e.g., default object or human-readable string like "N/A" or an empty
order summary) so the template never receives undefined; update references to
detailedOrder (the variable returned by getDetailedOrder) to use this fallback
or explicitly check and format the admin message only when detailedOrder is
present.
In `@bot/modules/dispute/messages.ts`:
- Around line 100-105: The call to getDetailedOrder may return undefined so when
using detailedOrder in template generation you must guard against that and
provide a fallback; update the code around the detailedOrder assignment and
where it's interpolated (variable detailedOrder and the function
getDetailedOrder) to check if detailedOrder is undefined and replace it with a
safe default (e.g., an empty object or descriptive placeholder string) or bail
out early, then use that safe value when rendering the dispute message to avoid
the literal "undefined" appearing in output.
In `@util/index.ts`:
- Around line 406-413: The communityName value is assigned from user-controlled
community.name and must be sanitized before being used in MarkdownV2 output;
update the block that sets communityName (the Community.findOne(...) branch) to
run community.name through the existing sanitizeMD function (or import/implement
it if missing) so communityName = sanitizeMD(community.name) instead of raw
community.name, ensuring any downstream Markdown rendering uses the sanitized
communityName.
🧹 Nitpick comments (1)
locales/de.yaml (1)
211-212: Consider translating "Community" to German for consistency.Other field labels in this file use German (e.g., "Ersteller", "Käufer", "Verkäufer"). For consistency, consider using "Gemeinschaft: ${communityName}" instead of the English "Community:".
Suggested change
- Community: ${communityName} + Gemeinschaft: ${communityName}
| const detailedOrder = await getDetailedOrder( | ||
| i18n, | ||
| order, | ||
| buyerUser, | ||
| sellerUser, | ||
| ); |
There was a problem hiding this comment.
Guard against undefined detailedOrder in admin message.
If getDetailedOrder fails (now with a DB lookup), the template can render “undefined”. A simple fallback keeps the admin message clean.
🛠️ Suggested fix
- const detailedOrder = await getDetailedOrder(
- i18n,
- order,
- buyerUser,
- sellerUser,
- );
+ const detailedOrder =
+ (await getDetailedOrder(i18n, order, buyerUser, sellerUser)) ?? '';🤖 Prompt for AI Agents
In `@bot/messages.ts` around lines 1702 - 1707, The admin message currently uses
the result of getDetailedOrder and can end up rendering "undefined" if that call
fails; before rendering the admin template in bot/messages.ts (around the usage
of detailedOrder), add a guard that assigns a safe fallback when detailedOrder
is falsy (e.g., default object or human-readable string like "N/A" or an empty
order summary) so the template never receives undefined; update references to
detailedOrder (the variable returned by getDetailedOrder) to use this fallback
or explicitly check and format the admin message only when detailedOrder is
present.
| const detailedOrder = await getDetailedOrder( | ||
| ctx.i18n, | ||
| order, | ||
| buyer, | ||
| seller, | ||
| ); |
There was a problem hiding this comment.
Handle undefined detailedOrder before templating.
If getDetailedOrder returns undefined, the dispute message can include a literal “undefined”. A fallback avoids that.
🛠️ Suggested fix
- const detailedOrder = await getDetailedOrder(
- ctx.i18n,
- order,
- buyer,
- seller,
- );
+ const detailedOrder =
+ (await getDetailedOrder(ctx.i18n, order, buyer, seller)) ?? '';🤖 Prompt for AI Agents
In `@bot/modules/dispute/messages.ts` around lines 100 - 105, The call to
getDetailedOrder may return undefined so when using detailedOrder in template
generation you must guard against that and provide a fallback; update the code
around the detailedOrder assignment and where it's interpolated (variable
detailedOrder and the function getDetailedOrder) to check if detailedOrder is
undefined and replace it with a safe default (e.g., an empty object or
descriptive placeholder string) or bail out early, then use that safe value when
rendering the dispute message to avoid the literal "undefined" appearing in
output.
If the user does not have community selected then the text should be |
mostronator
left a comment
mostronator
left a comment
There was a problem hiding this comment.
Good feature — adding community name to order details and preventing "default" as a community name. However, there are issues to address:
1. Breaking change: getDetailedOrder is now async but not all callers are updated.
You changed getDetailedOrder to async (returns a Promise) and updated callers in messages.ts and dispute/messages.ts. But dispute/messages.ts in the disputeData function (line ~100 in current main) also calls getDetailedOrder and your diff shows it's NOT awaited there:
const detailedOrder = getDetailedOrder(ctx.i18n, order, buyer, seller);This will now be a Promise object instead of a string, causing the dispute data message to break silently. This needs await.
2. Case-insensitive check inconsistency:
- In
scenes.tscreate wizard:name === 'default'(case-sensitive) - In
scenes.tsupdate wizard:name.toLowerCase() === 'default'(case-insensitive) - In model validator:
value.toLowerCase() !== 'default'(case-insensitive)
The create wizard check should also be case-insensitive: name.toLowerCase() === 'default'
3. UX issue in update wizard: When the name is "default", you reply with an error message and leave the scene, but the community name was not changed. This is fine, but consider staying in the scene to let the user try again instead of forcing them to restart.
4. Missing newline at end of files: Several locale files are missing the final newline (shown by \ No newline at end of file). This is minor but should be fixed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@locales/es.yaml`:
- Around line 691-692: Update the Spanish translation for the key
community_name_no_community_reserved to correct the accent: change "operacion"
to "operación" so the string reads "No fue posible realizar la operación, 'No
community' es un nombre reservado." Keep the rest of the sentence unchanged and
ensure the updated value remains valid YAML quoting.
locales/es.yaml
Outdated
| check_solvers_last_warning: Tu comunidad ${communityName} no tiene ningún solucionador. Agregue al menos uno hoy para evitar que la comunidad quede inhabilitada. | ||
| community_name_no_community_reserved: No fue posible realizar la operacion, 'No community' es un nombre reservado. |
There was a problem hiding this comment.
Fix Spanish accent in reserved-name string.
Line 692 should read “operación”.
✏️ Suggested fix
-community_name_no_community_reserved: No fue posible realizar la operacion, 'No community' es un nombre reservado.
+community_name_no_community_reserved: No fue posible realizar la operación, 'No community' es un nombre reservado.📝 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.
| check_solvers_last_warning: Tu comunidad ${communityName} no tiene ningún solucionador. Agregue al menos uno hoy para evitar que la comunidad quede inhabilitada. | |
| community_name_no_community_reserved: No fue posible realizar la operacion, 'No community' es un nombre reservado. | |
| check_solvers_last_warning: Tu comunidad ${communityName} no tiene ningún solucionador. Agregue al menos uno hoy para evitar que la comunidad quede inhabilitada. | |
| community_name_no_community_reserved: No fue posible realizar la operación, 'No community' es un nombre reservado. |
🤖 Prompt for AI Agents
In `@locales/es.yaml` around lines 691 - 692, Update the Spanish translation for
the key community_name_no_community_reserved to correct the accent: change
"operacion" to "operación" so the string reads "No fue posible realizar la
operación, 'No community' es un nombre reservado." Keep the rest of the sentence
unchanged and ensure the updated value remains valid YAML quoting.
…output and logic simplification in community/scenes.ts as there's no need of checking for reserved values in the community name
3 participants
Modified the 'getDetailedOrder' function to include the community of the order. In the case the order has no community, then it displays 'default' as the community name.
Also, to enforce that restriction, no communities created can be named 'default'.
Summary by CodeRabbit