Translated gift subscription Open Graph preview#27675
Conversation
WalkthroughgetCadenceLabel in the gift-preview controller was changed to return localized cadence text via the i18n Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ghost/core/test/unit/server/web/gift-preview/controller.test.js (1)
9-12: ⚡ Quick winAdd one non-English regression case.
This setup fix makes the controller pick up the initialized translator, but the suite still only asserts English fallback strings. Since this PR is specifically about non-English OG output, please add one test that switches to a non-English locale and checks the translated
og:title,og:description, or noscript CTA so this regression stays covered.🤖 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 `@ghost/core/test/unit/server/web/gift-preview/controller.test.js` around lines 9 - 12, Add a regression test in the existing gift-preview controller unit suite that switches the i18n locale to a non-English language (e.g., call the i18n instance's locale/setLocale to 'es' or 'de' before requiring or invoking the controller) and asserts that the rendered output contains the translated string(s) instead of the English fallback; target the same outputs already checked (for example check the controller's rendered og:title, og:description or the noscript CTA) so the new test verifies the controller uses the initialized translator (refer to the existing i18n.init() setup and the controller under test in this file).
🤖 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.
Inline comments:
In `@ghost/core/core/server/web/gift-preview/controller.js`:
- Around line 6-11: The getCadenceLabel function currently uses a manual
singular/plural ternary which breaks languages with multiple plural forms;
update both branches to use i18next count-based pluralization (e.g., call t with
keys like '{count} year' and '{count} month' and pass {count: duration}) so
i18next can pick the correct plural form for the locale; update the two places
where t is invoked in getCadenceLabel (the 'year' branch and the default/month
branch) to use the '{count} ...' key and count: duration argument.
---
Nitpick comments:
In `@ghost/core/test/unit/server/web/gift-preview/controller.test.js`:
- Around line 9-12: Add a regression test in the existing gift-preview
controller unit suite that switches the i18n locale to a non-English language
(e.g., call the i18n instance's locale/setLocale to 'es' or 'de' before
requiring or invoking the controller) and asserts that the rendered output
contains the translated string(s) instead of the English fallback; target the
same outputs already checked (for example check the controller's rendered
og:title, og:description or the noscript CTA) so the new test verifies the
controller uses the initialized translator (refer to the existing i18n.init()
setup and the controller under test in this file).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7bf31bbd-1041-4992-b9f2-4712bb98daf8
📒 Files selected for processing (65)
ghost/core/core/server/web/gift-preview/controller.jsghost/core/test/unit/server/web/gift-preview/controller.test.jsghost/i18n/locales/af/ghost.jsonghost/i18n/locales/ar/ghost.jsonghost/i18n/locales/bg/ghost.jsonghost/i18n/locales/bn/ghost.jsonghost/i18n/locales/bs/ghost.jsonghost/i18n/locales/ca/ghost.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/ghost.jsonghost/i18n/locales/da/ghost.jsonghost/i18n/locales/de-CH/ghost.jsonghost/i18n/locales/de/ghost.jsonghost/i18n/locales/el/ghost.jsonghost/i18n/locales/en/ghost.jsonghost/i18n/locales/eo/ghost.jsonghost/i18n/locales/es/ghost.jsonghost/i18n/locales/et/ghost.jsonghost/i18n/locales/eu/ghost.jsonghost/i18n/locales/fa/ghost.jsonghost/i18n/locales/fi/ghost.jsonghost/i18n/locales/fr/ghost.jsonghost/i18n/locales/gd/ghost.jsonghost/i18n/locales/he/ghost.jsonghost/i18n/locales/hi/ghost.jsonghost/i18n/locales/hr/ghost.jsonghost/i18n/locales/hu/ghost.jsonghost/i18n/locales/id/ghost.jsonghost/i18n/locales/is/ghost.jsonghost/i18n/locales/it/ghost.jsonghost/i18n/locales/ja/ghost.jsonghost/i18n/locales/ko/ghost.jsonghost/i18n/locales/kz/ghost.jsonghost/i18n/locales/lt/ghost.jsonghost/i18n/locales/lv/ghost.jsonghost/i18n/locales/mk/ghost.jsonghost/i18n/locales/mn/ghost.jsonghost/i18n/locales/ms/ghost.jsonghost/i18n/locales/nb/ghost.jsonghost/i18n/locales/ne/ghost.jsonghost/i18n/locales/nl/ghost.jsonghost/i18n/locales/nn/ghost.jsonghost/i18n/locales/pa/ghost.jsonghost/i18n/locales/pl/ghost.jsonghost/i18n/locales/pt-BR/ghost.jsonghost/i18n/locales/pt/ghost.jsonghost/i18n/locales/ro/ghost.jsonghost/i18n/locales/ru/ghost.jsonghost/i18n/locales/si/ghost.jsonghost/i18n/locales/sk/ghost.jsonghost/i18n/locales/sl/ghost.jsonghost/i18n/locales/sq/ghost.jsonghost/i18n/locales/sr-Cyrl/ghost.jsonghost/i18n/locales/sr/ghost.jsonghost/i18n/locales/sv/ghost.jsonghost/i18n/locales/sw/ghost.jsonghost/i18n/locales/ta/ghost.jsonghost/i18n/locales/th/ghost.jsonghost/i18n/locales/tr/ghost.jsonghost/i18n/locales/uk/ghost.jsonghost/i18n/locales/ur/ghost.jsonghost/i18n/locales/uz/ghost.jsonghost/i18n/locales/vi/ghost.jsonghost/i18n/locales/zh-Hant/ghost.jsonghost/i18n/locales/zh/ghost.json
ref https://linear.app/ghost/issue/BER-3529 - the social media preview rendered at `/gift/:token` (title, description, noscript fallback) was hardcoded English, leaving non-English sites with mismatched OG metadata when gift links are shared - wrapped user-facing strings with `t()` and added 5 new keys to the ghost namespace - duration label uses the established `1 X` / `{n} Xs` two-key split (matching `portal.json`) rather than appending `s` to a bare unit, since plural rules and word order vary by locale - initialised i18n in the controller test before requiring the controller so the destructured `t` import resolves to the live `i18next` instance
c1cfe04 to
465fe00
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@ghost/i18n/locales/context.json`:
- Around line 427-430: Add context entries for the Slovak-specific plural keys
({count} month_few, {count} month_many, {count} year_few, {count} year_many)
into the context.json alongside the existing {count} month_one/{count}
month_other/{count} year_one/{count} year_other entries; use the same gift
subscription Open Graph preview descriptions as the corresponding month/year
entries (explaining singular/plural usage, example text like '1 month'/'3
months' or '1 year'/'2 years', and that translators may add language-specific
plural forms) so translators have full guidance for the Slovak plural variants.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7077a431-61a3-438a-84ab-49317cb7871d
📒 Files selected for processing (65)
ghost/core/core/server/web/gift-preview/controller.jsghost/core/test/unit/server/web/gift-preview/controller.test.jsghost/i18n/locales/af/ghost.jsonghost/i18n/locales/ar/ghost.jsonghost/i18n/locales/bg/ghost.jsonghost/i18n/locales/bn/ghost.jsonghost/i18n/locales/bs/ghost.jsonghost/i18n/locales/ca/ghost.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/ghost.jsonghost/i18n/locales/da/ghost.jsonghost/i18n/locales/de-CH/ghost.jsonghost/i18n/locales/de/ghost.jsonghost/i18n/locales/el/ghost.jsonghost/i18n/locales/en/ghost.jsonghost/i18n/locales/eo/ghost.jsonghost/i18n/locales/es/ghost.jsonghost/i18n/locales/et/ghost.jsonghost/i18n/locales/eu/ghost.jsonghost/i18n/locales/fa/ghost.jsonghost/i18n/locales/fi/ghost.jsonghost/i18n/locales/fr/ghost.jsonghost/i18n/locales/gd/ghost.jsonghost/i18n/locales/he/ghost.jsonghost/i18n/locales/hi/ghost.jsonghost/i18n/locales/hr/ghost.jsonghost/i18n/locales/hu/ghost.jsonghost/i18n/locales/id/ghost.jsonghost/i18n/locales/is/ghost.jsonghost/i18n/locales/it/ghost.jsonghost/i18n/locales/ja/ghost.jsonghost/i18n/locales/ko/ghost.jsonghost/i18n/locales/kz/ghost.jsonghost/i18n/locales/lt/ghost.jsonghost/i18n/locales/lv/ghost.jsonghost/i18n/locales/mk/ghost.jsonghost/i18n/locales/mn/ghost.jsonghost/i18n/locales/ms/ghost.jsonghost/i18n/locales/nb/ghost.jsonghost/i18n/locales/ne/ghost.jsonghost/i18n/locales/nl/ghost.jsonghost/i18n/locales/nn/ghost.jsonghost/i18n/locales/pa/ghost.jsonghost/i18n/locales/pl/ghost.jsonghost/i18n/locales/pt-BR/ghost.jsonghost/i18n/locales/pt/ghost.jsonghost/i18n/locales/ro/ghost.jsonghost/i18n/locales/ru/ghost.jsonghost/i18n/locales/si/ghost.jsonghost/i18n/locales/sk/ghost.jsonghost/i18n/locales/sl/ghost.jsonghost/i18n/locales/sq/ghost.jsonghost/i18n/locales/sr-Cyrl/ghost.jsonghost/i18n/locales/sr/ghost.jsonghost/i18n/locales/sv/ghost.jsonghost/i18n/locales/sw/ghost.jsonghost/i18n/locales/ta/ghost.jsonghost/i18n/locales/th/ghost.jsonghost/i18n/locales/tr/ghost.jsonghost/i18n/locales/uk/ghost.jsonghost/i18n/locales/ur/ghost.jsonghost/i18n/locales/uz/ghost.jsonghost/i18n/locales/vi/ghost.jsonghost/i18n/locales/zh-Hant/ghost.jsonghost/i18n/locales/zh/ghost.json
✅ Files skipped from review due to trivial changes (24)
- ghost/core/test/unit/server/web/gift-preview/controller.test.js
- ghost/i18n/locales/is/ghost.json
- ghost/i18n/locales/ko/ghost.json
- ghost/i18n/locales/bs/ghost.json
- ghost/i18n/locales/sr-Cyrl/ghost.json
- ghost/i18n/locales/mn/ghost.json
- ghost/i18n/locales/th/ghost.json
- ghost/i18n/locales/nn/ghost.json
- ghost/i18n/locales/ru/ghost.json
- ghost/i18n/locales/hi/ghost.json
- ghost/i18n/locales/zh-Hant/ghost.json
- ghost/i18n/locales/fi/ghost.json
- ghost/i18n/locales/af/ghost.json
- ghost/i18n/locales/sl/ghost.json
- ghost/i18n/locales/uk/ghost.json
- ghost/i18n/locales/nb/ghost.json
- ghost/i18n/locales/he/ghost.json
- ghost/i18n/locales/fa/ghost.json
- ghost/i18n/locales/sr/ghost.json
- ghost/i18n/locales/si/ghost.json
- ghost/i18n/locales/pa/ghost.json
- ghost/i18n/locales/ca/ghost.json
- ghost/i18n/locales/cs/ghost.json
- ghost/i18n/locales/sq/ghost.json
🚧 Files skipped from review as they are similar to previous changes (13)
- ghost/i18n/locales/el/ghost.json
- ghost/i18n/locales/fr/ghost.json
- ghost/i18n/locales/hu/ghost.json
- ghost/i18n/locales/pt-BR/ghost.json
- ghost/i18n/locales/gd/ghost.json
- ghost/i18n/locales/it/ghost.json
- ghost/i18n/locales/ro/ghost.json
- ghost/i18n/locales/eu/ghost.json
- ghost/i18n/locales/uz/ghost.json
- ghost/i18n/locales/ne/ghost.json
- ghost/i18n/locales/vi/ghost.json
- ghost/i18n/locales/pt/ghost.json
- ghost/i18n/locales/bn/ghost.json
| "{count} year_two": "", | ||
| "{count} year_few": "", | ||
| "{count} year_many": "", | ||
| "{count} year_other": "", |
There was a problem hiding this comment.
I'm not sure we need these? 🤔 Wouldn't it be sufficient to have:
"{count} month": "",
"{count} months": "",
"{count} year": "",
"{count} years": "",
There was a problem hiding this comment.
Its what our i18n tool generates - this might explain it better
| } | ||
|
|
||
| return t('{count} month', {count: duration}); | ||
| } |
There was a problem hiding this comment.
nit: works today, but would need adapting if we introduce 3 months, 6 months, etc.
There was a problem hiding this comment.
Update: actually I'm not sure anymore by looking at the generated translations lol. Does it cover plural forms already?
There was a problem hiding this comment.
Yep, thats what the year_many is for
ref https://linear.app/ghost/issue/BER-3529 - the social media preview rendered at `/gift/:token` (title, description, noscript fallback) was hardcoded English, leaving non-English sites with mismatched OG metadata when gift links are shared - wrapped user-facing strings with `t()` and added 5 new keys to the ghost namespace - duration label uses the established `1 X` / `{n} Xs` two-key split (matching `portal.json`) rather than appending `s` to a bare unit, since plural rules and word order vary by locale - initialised i18n in the controller test before requiring the controller so the destructured `t` import resolves to the live `i18next` instance
ref https://linear.app/ghost/issue/BER-3529
/gift/:token(title, description, noscript fallback) was hardcoded English, leaving non-English sites with mismatched OG metadata when gift links are sharedt()and added 5 new keys to the ghost namespace1 X/{n} Xstwo-key split (matchingportal.json) rather than appendingsto a bare unit, since plural rules and word order vary by localetimport resolves to the livei18nextinstance