Fix/cancel counterparty locale - #874
Conversation
…arty-locale # Conflicts: # util/index.ts
WalkthroughThis PR fixes locale mismatches in cancellation notifications by deriving per-recipient i18n contexts in cancellation flows, updating the admin cancel message helper to accept an explicit i18n argument, simplifying locale selection, and adding regression tests. ChangesRecipient-specific i18n for cancellation messages
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
🧹 Nitpick comments (1)
util/index.ts (1)
388-395: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider caching the
I18ninstance to avoid repeated disk reads.
getUserI18nContextconstructsnew I18n({ directory: 'locales' })on every invocation. With this PR's per-recipient pattern, a single cancel flow can call it 3–4 times, each reading the same locale files. A module-level singleton keyed by locale (or a single sharedI18nwithcreateContextper language) would eliminate redundant I/O.♻️ Suggested refactor: reuse a single I18n instance
+// Module-level singleton — loaded once, reused across all calls. +const sharedI18n = new I18n({ + defaultLanguageOnMissing: true, + directory: 'locales', +}); + const getUserI18nContext = async (user: UserDocument) => { const language = user.lang || 'en'; - const i18n = new I18n({ - locale: language, - defaultLanguageOnMissing: true, - directory: 'locales', - }); - - return i18n.createContext(language); + return sharedI18n.createContext(language); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@util/index.ts` around lines 388 - 395, `getUserI18nContext` in util/index.ts is recreating `I18n` on every call, causing repeated locale-file reads. Refactor it to reuse a cached `I18n` instance, ideally as a module-level singleton shared across calls, and keep using `createContext(language)` for the requested locale. If locale-specific state is needed, key the cache by language but avoid reconstructing `new I18n({ directory: 'locales' })` repeatedly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@util/index.ts`:
- Around line 388-395: `getUserI18nContext` in util/index.ts is recreating
`I18n` on every call, causing repeated locale-file reads. Refactor it to reuse a
cached `I18n` instance, ideally as a module-level singleton shared across calls,
and keep using `createContext(language)` for the requested locale. If
locale-specific state is needed, key the cache by language but avoid
reconstructing `new I18n({ directory: 'locales' })` repeatedly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97a68042-e4c6-4ca7-b616-670425594e71
📒 Files selected for processing (6)
bot/commands.tsbot/messages.tsbot/start.tstests/bot/cancelLocale.spec.tstests/util/index.spec.tsutil/index.ts
Luquitasjeffrey
left a comment
There was a problem hiding this comment.
Cancelation message before the changes of this PR with the code of main
Cancelation message with the changes of this PR:
It happens when an admin cancels an order using the /cancelorder command.
tACK
| const language = user.lang || 'en'; | ||
| const i18n = new I18n({ | ||
| locale: language, | ||
| defaultLanguageOnMissing: true, | ||
| directory: 'locales', | ||
| }); | ||
|
|
||
| return i18n.createContext(user.lang || language || 'en'); | ||
| return i18n.createContext(language); | ||
| }; |
There was a problem hiding this comment.
This is a nice fix. user.language didn't existed so always falled back to user.lang or english.
There was a problem hiding this comment.
@Luquitasjeffrey huh? there was no user.language string here
What
Fixes cancellation (and admin-cancel) messages being delivered in the wrong language — the caller's, the buyer's, or the admin's — instead of each recipient's own configured language.
Closes #873.
Root cause
Two independent layers:
getUserI18nContextnever resolved the user's real language. It readuser.language, but the User model stores the language inlang(only Community haslanguage). The field was always undefined, so every per-user context silently fell back to the default locale — affecting every counterparty-facing message, not just cancellations.Call sites passing the wrong context to a recipient.
cancelAddInvoicebuilt a single context from the buyer and reused it for the seller, andsuccessCancelOrderByAdminMessagesent notices to both parties using the admin'sctx.i18n.Changes
util/index.ts—getUserI18nContextnow readsuser.lang(with an'en'fallback) and builds the context from the resolved language.bot/commands.ts—cancelAddInvoicenow resolvesi18nCtxSeller/i18nCtxBuyerseparately and passes the correct one to eachmessages.*call (mirroring the already-correctcancelShowHoldInvoice).bot/messages.ts—successCancelOrderByAdminMessagetakes an expliciti18nparameter instead of usingctx.i18n.bot/start.ts— the admin/cancelorderhandler resolves the buyer's and seller's contexts and passes each to its own message.Affected entry points (all verified)
All cancellation paths funnel through
cancelAddInvoice,cancelShowHoldInvoice, and the cooperativecancelOrder:/cancel(command and inline cancel buttons)/cancelall/cancelorder(admin / solver)Tests
tests/bot/cancelLocale.spec.ts(new) — exercises every buyer/seller × taker/creator combination of the cancel flow and asserts, generically, that each message's i18n context matches its recipient's language. Verified the guard actually catches the bug by temporarily reintroducing it.tests/util/index.spec.ts— adds unit tests lockinggetUserI18nContextto thelangfield (and the'en'fallback), covering the root cause directly.All new tests pass (
155 passing); lint andtscare clean. The 28 pre-existing failures onmain(DB/LND-dependent) are unchanged.Steps to reproduce (before this fix)
WAITING_BUYER_INVOICE)./cancelall(or/cancel).Summary by CodeRabbit
Bug Fixes
Tests
getUserI18nContextbuilds locale fromuser.langand properly falls back to English.Style