Improve i18n flexibility for showusername order sentences Issue712 - #770
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR refactors order title message generation by introducing a dedicated helper function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/ordersActions.ts (1)
244-250:⚠️ Potential issue | 🟠 MajorRemove unused
amountTextto unblock lint.
amountTextis assigned but never read (Line 244 / Line 247), and CI is failing ontypescript-eslint/no-unused-vars.🧹 Suggested fix
- let amountText = `${numberFormat(fiatCode, amount)} `; let tasaText = ''; if (priceFromAPI) { - amountText = ''; tasaText = i18n.t('rate') + `: ${process.env.FIAT_RATE_NAME} ${priceMarginText}\n`; } else {As per coding guidelines, "Follow ESLint Standard plus TypeScript rules and address warnings instead of disabling them".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bot/ordersActions.ts` around lines 244 - 250, The variable amountText is assigned but never used in the block guarded by priceFromAPI causing a lint error; remove the unused assignment and any redundant initialization of amountText and instead only set tasaText when priceFromAPI is truthy (keep the existing use of tasaText, priceFromAPI, numberFormat, fiatCode and amount elsewhere intact), or collapse the conditional so amountText is not declared at all in this scope—ensure references to amountText elsewhere are preserved or replaced if they relied on this value.
🤖 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/fr.yaml`:
- Line 441: The French translation key showusername_selling_sats uses the
incorrect verb form "vent"; update its value to use the singular conjugation
"vend" so the line reads showusername_selling_sats: "@${username} vend des sats"
to match the singular subject interpolation @${username}.
---
Outside diff comments:
In `@bot/ordersActions.ts`:
- Around line 244-250: The variable amountText is assigned but never used in the
block guarded by priceFromAPI causing a lint error; remove the unused assignment
and any redundant initialization of amountText and instead only set tasaText
when priceFromAPI is truthy (keep the existing use of tasaText, priceFromAPI,
numberFormat, fiatCode and amount elsewhere intact), or collapse the conditional
so amountText is not declared at all in this scope—ensure references to
amountText elsewhere are preserved or replaced if they relied on this value.
🪄 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: d052ce83-ced9-4825-b670-f78bd928e73d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
bot/ordersActions.tslocales/de.yamllocales/en.yamllocales/es.yamllocales/fa.yamllocales/fr.yamllocales/it.yamllocales/ko.yamllocales/pt.yamllocales/ru.yamllocales/uk.yamlpackage.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bot/ordersActions.ts (1)
263-296:⚠️ Potential issue | 🟡 MinorFix Prettier formatting before merging.
The CI pipeline reports formatting violations on these lines. As per coding guidelines, run Prettier before committing.
npm run format🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bot/ordersActions.ts` around lines 263 - 296, The file has Prettier formatting issues; run the formatter and fix whitespace/line breaks around the description assembly in the ordersActions.ts function (the multi-line description construction and related template strings) and ensure getOrderTitleMessage remains unchanged in behavior; run "npm run format" and commit the formatted changes so the CI formatting checks pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@bot/ordersActions.ts`:
- Around line 263-296: The file has Prettier formatting issues; run the
formatter and fix whitespace/line breaks around the description assembly in the
ordersActions.ts function (the multi-line description construction and related
template strings) and ensure getOrderTitleMessage remains unchanged in behavior;
run "npm run format" and commit the formatted changes so the CI formatting
checks pass.
|
utACK, pls fix CI |
Matobi98
changed the title
![mostronatorcoder[bot]](https://avatars.githubusercontent.com/in/2922838?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: PR #770
Estado general: Funcionalmente correcto.
🔴 Blocking
Typo en francés — → en .
🟡 Minor
- Keys huérfanas en todos los locale files.
- PR description no coincide con el diff real.
✅ Lo que está bien
- correcta y clara
- 4 casos bien cubiertos
- Nuevas keys de locale en todos los idiomas
- Enfoque de oración completa (resuelve Persian RTL)
- PR mergeable, CI verde
Veredicto: Request Changes — typo → y limpiar keys huérfanas.
'vent' (wind) → 'vend' (sells) in locales/fr.yaml.
|
French typo — Fixed: vent → vend in locales/fr.yaml. PR description — Updated to reflect the actual diff: the key names are showusername_selling_sats / Orphan keys — I checked and none of them were introduced by this PR. They are all pre-existing issues: disput_too_soon and |
4 participants
Improve i18n flexibility for showusername order sentences
Problem
When a user sets
/showusername yesand creates an order, the bot constructs a sentence using several isolated one-word strings from locale files:This approach assumes English word order, which breaks natural phrasing in many other languages. Translators are forced to work around a fixed sentence structure they have no control over.
A concrete example is Persian (Farsi), an RTL language. Telegram determines text direction based on the first character of each line. Since
@usernamealways comes first and@is an LTR character, the entire line is rendered LTR — making Persian text appear incorrectly.A common workaround is prepending a native word (e.g., "کاربر", meaning User) to trigger RTL rendering, but the current string structure makes this impossible.
Solution
Replace the isolated
sellingandbuyingstrings with complete, context-aware strings:showusername_selling_sats: '@${username} is selling sats' showusername_buying_sats: '@${username} is buying sats' <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Order descriptions now clearly indicate sats trading context with optional counterparty username display for enhanced transaction clarity. * **Localization** * Updated translation strings across all supported languages including English, German, Spanish, French, Italian, Korean, Portuguese, Russian, Ukrainian, and Persian. <!-- end of auto-generated comment: release notes by coderabbit.ai -->