🌐 server: add spanish translations push notifications#878
🌐 server: add spanish translations push notifications#878
Conversation
🦋 Changeset detectedLatest commit: 2a4b978 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the server's push notification system by integrating Spanish translations for a wide array of user-facing messages. The update ensures that Spanish-speaking users receive localized and culturally relevant notifications for key events, improving accessibility and overall user experience without altering the core functionality of the notifications. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR introduces i18n support for push notifications across multiple server-side hooks by replacing hardcoded English strings with i18n translations. Changes include adding the i18next dependency, creating a new server-side i18n module with English and Spanish translation files, updating six hook files to use localized notifications, and updating corresponding tests to expect translated payloads. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
✅ All tests passed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (3)
server/hooks/panda.ts (1)
757-766:⚠️ Potential issue | 🟡 MinorUse explicit locales for the translated purchase amounts.
These branches still call
toLocaleString(undefined, ...), so the amount format comes from the server/runtime locale instead of the notification language. That can render the new Spanish copy with non-Spanish currency formatting. Pass"en"and"es"explicitly here.♻️ Proposed fix
contents: { - en: `${(payload.body.spend.localAmount / 100).toLocaleString(undefined, { + en: `${(payload.body.spend.localAmount / 100).toLocaleString("en", { style: "currency", currency: payload.body.spend.localCurrency, })} at ${payload.body.spend.merchantName.trim()}. Paid ${{ 0: "with USDC", 1: "with credit" }[card.mode] ?? `in ${card.mode} installments`}`, - es: `${(payload.body.spend.localAmount / 100).toLocaleString(undefined, { + es: `${(payload.body.spend.localAmount / 100).toLocaleString("es", { style: "currency", currency: payload.body.spend.localCurrency, })} en ${payload.body.spend.merchantName.trim()}. Pagado ${{ 0: "con USDC", 1: "con crédito" }[card.mode] ?? `en ${card.mode} cuotas`}`, },server/api/card.ts (1)
385-386:⚠️ Potential issue | 🟡 MinorAdd inline
cspell:ignorefor the new Spanish literals.These strings are already tripping spell-check. Keep the file green by suppressing the one-off Spanish words on the same lines instead of expanding the shared dictionary.
📝 Proposed fix
- headings: { en: "Card mode", es: "Modo de tarjeta" }, - contents: { en: "Credit mode is active", es: "El modo crédito está activo" }, + headings: { en: "Card mode", es: "Modo de tarjeta" }, // cspell:ignore tarjeta + contents: { en: "Credit mode is active", es: "El modo crédito está activo" }, // cspell:ignore crédito está activoAs per coding guidelines
Place cspell:ignore annotations on the same line as the unknown word; only add words to cspell.json when the term is broadly used project-wide; keep one-off occurrences (variable names, company names, URLs, hashes) as inline cspell:ignore.server/hooks/activity.ts (1)
129-132:⚠️ Potential issue | 🟡 MinorSuppress the new spell-check hits inline.
These Spanish literals are already producing spell-check warnings. Add same-line
cspell:ignoreannotations here instead of broadening the shared dictionary.📝 Proposed fix
- headings: { en: "Funds received", es: "Fondos recibidos" }, + headings: { en: "Funds received", es: "Fondos recibidos" }, // cspell:ignore fondos recibidos contents: { en: `${value ? `${value} ` : ""}${assetSymbol} received${marketsByAsset.has(underlying) ? " and instantly started earning yield" : ""}`, - es: `${value ? `${value} ` : ""}${assetSymbol} recibidos${marketsByAsset.has(underlying) ? " y empezaron a generar rendimiento" : ""}`, + es: `${value ? `${value} ` : ""}${assetSymbol} recibidos${marketsByAsset.has(underlying) ? " y empezaron a generar rendimiento" : ""}`, // cspell:ignore recibidos empezaron generar rendimiento },As per coding guidelines
Place cspell:ignore annotations on the same line as the unknown word; only add words to cspell.json when the term is broadly used project-wide; keep one-off occurrences (variable names, company names, URLs, hashes) as inline cspell:ignore.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 965fa720-71b2-4403-8a6f-015286bc896c
📒 Files selected for processing (7)
.changeset/tame-candles-itch.mdserver/api/card.tsserver/hooks/activity.tsserver/hooks/block.tsserver/hooks/manteca.tsserver/hooks/panda.tsserver/test/hooks/block.test.ts
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
cspell.json (1)
215-244: 🧹 Nitpick | 🔵 TrivialNarrow this cspell override to the files that actually contain Spanish copy.
Allowlisting these terms for all
server/**/*.tsweakens spellcheck across the whole backend for strings that, per this PR, appear to live in a small set of notification/test files. Prefer inlinecspell:ignoreon the affected lines or a file-scoped override for those specific files unless these words are already reused broadly acrossserver/**. Based on learnings,Place cspell:ignore annotations on the same line as the unknown word; only add words to cspell.json when the term is broadly used project-wide; keep one-off occurrences (variable names, company names, URLs, hashes) as inline cspell:ignore.#!/bin/bash set -euo pipefail terms='activada|activado|activo|cambiado|completado|compra|crédito|cuenta|cuotas|depositados|empezaron|enviados|está|fondos|fueron|generar|pagado|procesado|rampa|recibidos|reembolsados|reembolso|rendimiento|retiro|sido|tarjeta' # Verify how broadly these terms are used inside server TypeScript files. # If matches are limited to a few translation-bearing files, prefer inline ignores # or a narrower override instead of a blanket `server/**/*.ts` override. rg -n --type ts -g 'server/**/*.ts' "\\b(${terms})\\b" -C1
aguxez
force-pushed
the
translations
branch
3 times, most recently
from
9270b44 to
9056fc2
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
aguxez
force-pushed
the
translations
branch
3 times, most recently
from
6beaa6e to
89ea61f
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
aguxez
force-pushed
the
translations
branch
3 times, most recently
from
a2f1cca to
c46af07
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
aguxez
force-pushed
the
translations
branch
3 times, most recently
from
e9312ef to
1a7f1b4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a7f1b49e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| typeof value === "string" | ||
| ? { minimumFractionDigits: 0, maximumFractionDigits: value.split(".")[1]?.length ?? 0 } | ||
| : { maximumSignificantDigits: 6 }; | ||
| return { en: n.toLocaleString("en-US", options), es: n.toLocaleString("es-AR", options) }; |
There was a problem hiding this comment.
en/es pushes
The mobile app collapses locales to bare language codes (src/app/_layout.tsx uses only languageCode), but this helper now hardcodes en-US and es-AR for every translated amount; server/hooks/panda.ts repeats the same choice for card purchases. That means es-419/es-ES recipients will now get Argentina-specific separators/symbol placement (for example, 1234.56 becomes 1.234,56), and non-US English users get US conventions, in every translated deposit/withdraw/refund push. Because OneSignal only receives generic en/es variants, this change is region-wrong for all users outside those two locales.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a4b978ccc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }); | ||
|
|
||
| export function formatAmount(value: number | string) { | ||
| const n = typeof value === "string" ? Number(value) : value; |
There was a problem hiding this comment.
formatAmount() coerces every string input through Number(value) before localization, which silently changes exact decimal strings used by webhook and onchain payloads. For example, long decimal strings (like values from formatUnits(...)) can lose precision during coercion, so translated push notifications may show a different amount than the source event. This is a regression from the previous behavior that interpolated the original string directly.
Useful? React with 👍 / 👎.
2 participants
closes #855
Summary by CodeRabbit
New Features
Chores