Added static UI for gift subscription redemption#27188
Merged
Conversation
Contributor
|
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:
WalkthroughAdds end-to-end gift redemption to the portal: new GiftRedemptionPage component and styles, hash routing for Possibly related issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b19c3f6e12
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
ghost/i18n/locales/context.json (1)
7-7: Add context notes for the new strings to reduce translator ambiguity.
"Your email"and"Your name"can require tone/register choices across locales; a short description incontext.jsonhelps maintain consistency.Also applies to: 368-368, 373-373, 412-413
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/i18n/locales/context.json` at line 7, The locale context file is missing translator notes for ambiguous UI strings; add descriptive entries in context.json for the keys "1 year", "Your email", and "Your name" (and the other mentioned occurrences) explaining intended usage, tone/register and where they appear (e.g., "label for email input field" or "duration label meaning one full year") so translators can choose correct wording; update the JSON to include these context notes keyed by the exact string keys found in the diff (e.g., "1 year", "Your email", "Your name") and ensure the added notes follow the existing context.json structure and formatting.apps/portal/src/utils/api.js (1)
170-230: Mocked API response is appropriate for static UI development.The temporary mock aligns with the PR objective of enabling design work while backend integration is pending. The commented-out code effectively documents the intended implementation.
Consider creating a tracking issue for the TODO on Line 208 to ensure the real API integration isn't forgotten when backend work completes.
Do you want me to open an issue to track replacing the mocked
fetchRedemptionDatawith the actual API integration?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/utils/api.js` around lines 170 - 230, The mocked response in api.gift.fetchRedemptionData is fine for now but you should create a tracking issue to replace the mock and restore the real API integration (the TODO inside fetchRedemptionData referencing endpointFor/makeRequest and HumanReadableError); open an issue that references api.gift.fetchRedemptionData and the TODO comment so we don't lose the original implementation plan and include acceptance criteria: remove mock, re-enable the commented request flow, handle res.ok, and throw HumanReadableError.fromApiResponse when appropriate.apps/portal/src/utils/helpers.js (1)
801-816: Use optional chaining for cleaner null check.Static analysis flagged the condition at line 803. Using optional chaining improves readability.
Suggested fix
export const createNotification = ({type, status, autoHide, duration = 2600, closeable, state, message}) => { let count = 0; - if (state && state.notification) { + if (state?.notification) { count = (state.notification.count || 0) + 1; } return { type, status, autoHide, closeable, duration, message, count }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/utils/helpers.js` around lines 801 - 816, The null-check in createNotification is verbose; replace the conditional that checks state and state.notification with optional chaining and nullish coalescing so you compute count using state?.notification?.count (e.g., set count = (state?.notification?.count ?? 0) + 1 when a notification exists) to make the code cleaner and avoid manual && checks while preserving the current behavior in createNotification.apps/portal/test/portal-links.test.js (1)
503-516: TODO comment for feature flag cleanup.The comment at line 503 indicates this test should be removed when the
giftSubscriptionslabs flag is removed for GA. Consider creating a tracking issue to ensure this cleanup happens.Do you want me to open a new issue to track the removal of the labs flag check and associated test once
giftSubscriptionsreaches GA?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/test/portal-links.test.js` around lines 503 - 516, The TODO notes that the test 'does not open when giftSubscriptions labs flag is disabled' should be removed once the giftSubscriptions labs flag is removed for GA; create a tracking issue (or add a TODO with an issue link) to remove this test and any conditional code that checks the giftSubscriptions labs flag, and update the test name/fixtures if behavior changes; reference the test named "does not open when giftSubscriptions labs flag is disabled", the labs fixture usage (FixtureSite.singleTier.basic, labs: {}), and the hash route '#/portal/gift/redeem/gift-token-123' so the cleanup task knows exactly which test and feature-flag code to remove.ghost/i18n/locales/vi/portal.json (1)
15-19: Missing Vietnamese translations for new gift redemption keys.The new keys (
{years} years,1 year,Your email,Your name) have empty values and will fall back to English. This is acceptable for initial implementation, but translations should be added before GA release.Also applies to: 249-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/i18n/locales/vi/portal.json` around lines 15 - 19, The listed locale entries have empty values and will fall back to English; update the vi/portal.json translations by providing Vietnamese strings for the keys "{years} years", "1 year", "Your email", and "Your name" (and the same missing translations referenced at lines 249-253) so they no longer are blank; locate those exact JSON keys in the file and replace the empty values with appropriate Vietnamese translations.apps/portal/src/components/pages/gift-redemption-page.js (2)
213-226: Verify the effect behavior whengiftbecomes falsy after initial render.If
gifttransitions from truthy to falsy (e.g., due to a context update), this effect will trigger the error notification and close the popup. This may or may not be intended behavior. If the gift data should remain stable once loaded, consider adding an early-exit condition or tracking whether the notification was already shown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/components/pages/gift-redemption-page.js` around lines 213 - 226, The useEffect watching [doAction, gift] will fire when gift becomes falsy after initial render and unconditionally show the error notification and close the popup; modify the effect in gift-redemption-page.js (the useEffect that references gift, doAction and getGiftRedemptionErrorMessage) to avoid re-triggering on transient or post-load falsy changes—either add a guard that only runs when the component first mounts (e.g., track a local ref/flag like notificationShown) or check a stability condition (e.g., only show the error if gift was never truthy or if an explicit loadFailed state is set) so doAction('openNotification', ...) and doAction('closePopup') are only called once or under the intended error condition.
201-201: Address the TODO: Hardcoded English strings need translation.Several strings in this component are hardcoded in English instead of using the
t()function:
- Line 275:
'Gift membership'- Line 276:
'You\'ve been gifted a membership'- Line 302:
'Redeem gift membership'This is flagged by static analysis. Consider wrapping these in
t()calls once copy is finalized, or track this in a follow-up issue.Do you want me to open an issue to track this task?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/components/pages/gift-redemption-page.js` at line 201, The component contains hardcoded English strings that must be localized: in apps/portal/src/components/pages/gift-redemption-page.js replace the literals 'Gift membership', 'You've been gifted a membership', and 'Redeem gift membership' with calls to the i18n translator (e.g., t('...')) inside the GiftRedemptionPage component, using the same translation namespace/keys pattern as other strings in this file; if copy isn't finalized, add stable key names and a TODO or open a follow-up issue to finalize copy and translations so static analysis no longer flags these strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/portal/src/components/pages/gift-redemption-page.js`:
- Around line 278-284: The JSX is dereferencing gift.tier (gift.tier.name and
gift.tier.benefits) without a null guard; update the rendering in the component
around the gh-gift-redemption-plan block to first check for gift.tier (e.g., if
(!gift?.tier) return null or render a fallback) and only call
getGiftCadenceLabel(gift) and access gift.tier.name/benefits when tier is
present; ensure any conditional that currently reads gift.tier.benefits uses
optional chaining or the same guard to avoid runtime errors.
In `@apps/portal/src/utils/gift-redemption-notification.js`:
- Around line 6-15: The subtitle currently uses raw error.message in
getGiftRedemptionErrorMessage which bypasses i18n; instead map known error
identifiers/messages to translation keys and pass those keys through t() (e.g.,
create a mapping inside getGiftRedemptionErrorMessage from backend error codes
or known messages to locale keys like GIFT_ALREADY_REDEEMED, GIFT_EXPIRED), use
t(mappedKey) for the subtitle when a mapping exists, and fall back to
t(INVALID_GIFT_LINK_MESSAGE) when no mapping is found; also add the new keys to
the locale files and update callers/backend to surface stable error codes if
possible.
In `@ghost/i18n/locales/ar/portal.json`:
- Line 15: Fill in Arabic translations for the newly added, currently-empty
redemption label keys in ghost/i18n/locales/ar/portal.json — specifically
provide a proper Arabic value for the "{years} years" key shown and for the
other three empty redemption-related keys elsewhere in that file (the other
empty entries referenced at the same locale). Locate the empty string values in
portal.json (e.g., the "{years} years" entry and the two additional empty
entries mentioned) and replace each "" with the correct Arabic localized phrase
that matches the English source for gift redemption labels.
In `@ghost/i18n/locales/fi/portal.json`:
- Line 15: Fill in the missing Finnish translations for the new gift-redemption
keys by replacing the empty string values for the locale keys (e.g. "{years}
years" and the three other empty gift-redemption-related keys in the fi
portal.json) with correct Finnish phrases; ensure you use proper Finnish
pluralization/formatting for the placeholder {years} (e.g. " {years} vuotta" or
appropriate plural form) and provide accurate translations for the remaining
three keys so the Finnish UI is fully localized.
In `@ghost/i18n/locales/id/portal.json`:
- Line 15: Populate the empty Indonesian translations for the new
gift-redemption locale keys: replace the empty values for "{years} years", "1
year", "Your email", and "Your name" with proper Indonesian strings (e.g.,
"{years} tahun", "1 tahun", "Email Anda", "Nama Anda") so the redemption UI
shows localized copy; ensure you update each occurrence of these keys where they
appear in the portal.json locale entries (including the other reported spots)
and keep interpolation placeholders like {years} intact.
In `@ghost/i18n/locales/it/portal.json`:
- Line 15: Fill the empty Italian translations for the four new gift-redemption
keys in the locale JSON (including the key "{years} years" and the three other
empty keys referenced in the comment) by replacing the empty string values with
proper Italian phrases, preserving any interpolation placeholders (e.g.,
{years}, {months}) exactly as in the keys; ensure pluralization/grammar is
appropriate (e.g., "{years} years" -> "{years} anni") and update all four keys
consistently.
In `@ghost/i18n/locales/mk/portal.json`:
- Line 15: Add Macedonian translations for the four empty gift-redemption keys
in the mk portal.json: set values for the keys "{years} years", "1 year", "Your
email", and "Your name" so the UI is localized; update each key’s value to the
appropriate Macedonian string (for example "{years} години", "1 година", "Вашиот
е-пошта" or "Ваш е-пошта" as preferred, and "Вашето име"), ensuring you replace
the empty "" entries with the translated text for each key.
In `@ghost/i18n/locales/pt/portal.json`:
- Line 15: Several Portuguese translations are missing in the portal locale:
provide Portuguese strings for the keys "{years} years", "1 year", "Your email",
and "Your name" in the pt portal JSON so the redemption UI is fully localized;
update the entries for those exact keys (and the other mentioned empty keys at
the same keys on lines referenced) with appropriate pt translations (e.g.,
"{years} anos", "1 ano", "Seu e-mail", "Seu nome") ensuring proper plural
formatting and escaping as needed in the JSON file for keys "{years} years" and
"1 year".
In `@ghost/i18n/locales/sq/portal.json`:
- Line 15: The sq portal.json has four new redemption keys left empty (e.g., the
"{years} years" entry) causing untranslated strings; open
ghost/i18n/locales/sq/portal.json and provide Albanian translations for all four
newly added keys (the entries at the comment’s referenced locations), replacing
the empty strings with correct Albanian text while preserving the exact JSON
keys and quoting/escaping; ensure the translations match
pluralization/placeholders (e.g., keep "{years}" intact) and validate the file
is valid JSON after edits.
In `@ghost/i18n/locales/th/portal.json`:
- Line 15: The Thai locale JSON currently has empty values for the new
redemption-related keys (including the key "{years} years") which leaves the UI
untranslated; open ghost/i18n/locales/th/portal.json, locate the four newly
added keys (one shown as "{years} years" and the three other redemption-related
keys added in the same change) and replace each empty string with the correct
Thai translation strings ensuring proper interpolation syntax is preserved
(e.g., keep "{years}" intact inside the translated string) so the portal UI
displays localized text.
---
Nitpick comments:
In `@apps/portal/src/components/pages/gift-redemption-page.js`:
- Around line 213-226: The useEffect watching [doAction, gift] will fire when
gift becomes falsy after initial render and unconditionally show the error
notification and close the popup; modify the effect in gift-redemption-page.js
(the useEffect that references gift, doAction and getGiftRedemptionErrorMessage)
to avoid re-triggering on transient or post-load falsy changes—either add a
guard that only runs when the component first mounts (e.g., track a local
ref/flag like notificationShown) or check a stability condition (e.g., only show
the error if gift was never truthy or if an explicit loadFailed state is set) so
doAction('openNotification', ...) and doAction('closePopup') are only called
once or under the intended error condition.
- Line 201: The component contains hardcoded English strings that must be
localized: in apps/portal/src/components/pages/gift-redemption-page.js replace
the literals 'Gift membership', 'You've been gifted a membership', and 'Redeem
gift membership' with calls to the i18n translator (e.g., t('...')) inside the
GiftRedemptionPage component, using the same translation namespace/keys pattern
as other strings in this file; if copy isn't finalized, add stable key names and
a TODO or open a follow-up issue to finalize copy and translations so static
analysis no longer flags these strings.
In `@apps/portal/src/utils/api.js`:
- Around line 170-230: The mocked response in api.gift.fetchRedemptionData is
fine for now but you should create a tracking issue to replace the mock and
restore the real API integration (the TODO inside fetchRedemptionData
referencing endpointFor/makeRequest and HumanReadableError); open an issue that
references api.gift.fetchRedemptionData and the TODO comment so we don't lose
the original implementation plan and include acceptance criteria: remove mock,
re-enable the commented request flow, handle res.ok, and throw
HumanReadableError.fromApiResponse when appropriate.
In `@apps/portal/src/utils/helpers.js`:
- Around line 801-816: The null-check in createNotification is verbose; replace
the conditional that checks state and state.notification with optional chaining
and nullish coalescing so you compute count using state?.notification?.count
(e.g., set count = (state?.notification?.count ?? 0) + 1 when a notification
exists) to make the code cleaner and avoid manual && checks while preserving the
current behavior in createNotification.
In `@apps/portal/test/portal-links.test.js`:
- Around line 503-516: The TODO notes that the test 'does not open when
giftSubscriptions labs flag is disabled' should be removed once the
giftSubscriptions labs flag is removed for GA; create a tracking issue (or add a
TODO with an issue link) to remove this test and any conditional code that
checks the giftSubscriptions labs flag, and update the test name/fixtures if
behavior changes; reference the test named "does not open when giftSubscriptions
labs flag is disabled", the labs fixture usage (FixtureSite.singleTier.basic,
labs: {}), and the hash route '#/portal/gift/redeem/gift-token-123' so the
cleanup task knows exactly which test and feature-flag code to remove.
In `@ghost/i18n/locales/context.json`:
- Line 7: The locale context file is missing translator notes for ambiguous UI
strings; add descriptive entries in context.json for the keys "1 year", "Your
email", and "Your name" (and the other mentioned occurrences) explaining
intended usage, tone/register and where they appear (e.g., "label for email
input field" or "duration label meaning one full year") so translators can
choose correct wording; update the JSON to include these context notes keyed by
the exact string keys found in the diff (e.g., "1 year", "Your email", "Your
name") and ensure the added notes follow the existing context.json structure and
formatting.
In `@ghost/i18n/locales/vi/portal.json`:
- Around line 15-19: The listed locale entries have empty values and will fall
back to English; update the vi/portal.json translations by providing Vietnamese
strings for the keys "{years} years", "1 year", "Your email", and "Your name"
(and the same missing translations referenced at lines 249-253) so they no
longer are blank; locate those exact JSON keys in the file and replace the empty
values with appropriate Vietnamese translations.
🪄 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: 25b60fdd-9d1a-42f2-bc2c-62e1e932c909
📒 Files selected for processing (75)
apps/portal/package.jsonapps/portal/src/actions.jsapps/portal/src/app.jsapps/portal/src/components/frame.styles.jsapps/portal/src/components/notification.jsapps/portal/src/components/pages/gift-redemption-page.jsapps/portal/src/components/pages/gift-success-page.jsapps/portal/src/pages.jsapps/portal/src/utils/api.jsapps/portal/src/utils/gift-redemption-notification.jsapps/portal/src/utils/helpers.jsapps/portal/test/portal-links.test.jsghost/i18n/locales/af/portal.jsonghost/i18n/locales/ar/portal.jsonghost/i18n/locales/bg/portal.jsonghost/i18n/locales/bn/portal.jsonghost/i18n/locales/bs/portal.jsonghost/i18n/locales/ca/portal.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/portal.jsonghost/i18n/locales/da/portal.jsonghost/i18n/locales/de-CH/portal.jsonghost/i18n/locales/de/portal.jsonghost/i18n/locales/el/portal.jsonghost/i18n/locales/en/portal.jsonghost/i18n/locales/eo/portal.jsonghost/i18n/locales/es/portal.jsonghost/i18n/locales/et/portal.jsonghost/i18n/locales/eu/portal.jsonghost/i18n/locales/fa/portal.jsonghost/i18n/locales/fi/portal.jsonghost/i18n/locales/fr/portal.jsonghost/i18n/locales/gd/portal.jsonghost/i18n/locales/he/portal.jsonghost/i18n/locales/hi/portal.jsonghost/i18n/locales/hr/portal.jsonghost/i18n/locales/hu/portal.jsonghost/i18n/locales/id/portal.jsonghost/i18n/locales/is/portal.jsonghost/i18n/locales/it/portal.jsonghost/i18n/locales/ja/portal.jsonghost/i18n/locales/ko/portal.jsonghost/i18n/locales/kz/portal.jsonghost/i18n/locales/lt/portal.jsonghost/i18n/locales/lv/portal.jsonghost/i18n/locales/mk/portal.jsonghost/i18n/locales/mn/portal.jsonghost/i18n/locales/ms/portal.jsonghost/i18n/locales/nb/portal.jsonghost/i18n/locales/ne/portal.jsonghost/i18n/locales/nl/portal.jsonghost/i18n/locales/nn/portal.jsonghost/i18n/locales/pa/portal.jsonghost/i18n/locales/pl/portal.jsonghost/i18n/locales/pt-BR/portal.jsonghost/i18n/locales/pt/portal.jsonghost/i18n/locales/ro/portal.jsonghost/i18n/locales/ru/portal.jsonghost/i18n/locales/si/portal.jsonghost/i18n/locales/sk/portal.jsonghost/i18n/locales/sl/portal.jsonghost/i18n/locales/sq/portal.jsonghost/i18n/locales/sr-Cyrl/portal.jsonghost/i18n/locales/sr/portal.jsonghost/i18n/locales/sv/portal.jsonghost/i18n/locales/sw/portal.jsonghost/i18n/locales/ta/portal.jsonghost/i18n/locales/th/portal.jsonghost/i18n/locales/tr/portal.jsonghost/i18n/locales/uk/portal.jsonghost/i18n/locales/ur/portal.jsonghost/i18n/locales/uz/portal.jsonghost/i18n/locales/vi/portal.jsonghost/i18n/locales/zh-Hant/portal.jsonghost/i18n/locales/zh/portal.json
cathysarisky
reviewed
Apr 6, 2026
| ? error.message | ||
| : t(INVALID_GIFT_LINK_MESSAGE); | ||
|
|
||
| return { |
Member
There was a problem hiding this comment.
Noted on the TODO -- this is clearly still a bit WIP.. but just a reminder to please make sure the actual string occurs inside t, or else it won't get picked up by i18n-parser. So...
t(GIFT_REDEMPTION_ERROR_TITLE) is not picked up for translation, while t('Some specific error string') is. If you need to use a variable, you can provide a commented-out enumeration as a last resort i.e.
// t('first error message')
... but it might be preferable to wrap lines 3-4 directly, then they'll already be translated when you use them on lines 10 and 13 (no t needed there).
Contributor
Author
There was a problem hiding this comment.
Thanks @cathysarisky! This is a great should, keeping a note for when we add the translation keys (follow-up PR)
I've removed the unnecessary t(CONSTANT) from this PR
sagzy
force-pushed
the
gift-subs-add-redemption-screen
branch
from
b19c3f6 to
47def9c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/portal/src/app.js`:
- Around line 634-639: The code calls decodeURIComponent directly when
extracting the gift token (see usage around giftRedemptionRegex and the call to
fetchGiftRedemptionData) which can throw on malformed sequences; add a safe
decoding helper (e.g., safeDecodeURIComponent) and use it wherever
decodeURIComponent is used for gift tokens (both the initialization path that
extracts token from path and the other location around lines 957–963 that
decodes tokens for the click handler), so that malformed tokens are caught and
handled (return null/undefined or a sanitized string) before calling
fetchGiftRedemptionData to avoid throwing during initialization or click
handling.
In `@apps/portal/src/components/notification.js`:
- Around line 244-257: The NotificationContent child is reused across
notifications so its internal state can leak; modify the parent render to give
<NotificationContent> a key based on the notificationCount (e.g.,
key={this.state.notificationCount} or key={notification.count || 0}) so React
remounts the component whenever showNotification updates notificationCount;
update both places where NotificationContent is rendered (the instance related
to showNotification and the other render at the later block around line 300) to
use that key.
🪄 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: 759a73d6-cc71-44cd-bcaf-7d2d4d4ba695
📒 Files selected for processing (75)
apps/portal/package.jsonapps/portal/src/actions.jsapps/portal/src/app.jsapps/portal/src/components/frame.styles.jsapps/portal/src/components/notification.jsapps/portal/src/components/pages/gift-redemption-page.jsapps/portal/src/components/pages/gift-success-page.jsapps/portal/src/pages.jsapps/portal/src/utils/api.jsapps/portal/src/utils/gift-redemption-notification.jsapps/portal/src/utils/helpers.jsapps/portal/test/portal-links.test.jsghost/i18n/locales/af/portal.jsonghost/i18n/locales/ar/portal.jsonghost/i18n/locales/bg/portal.jsonghost/i18n/locales/bn/portal.jsonghost/i18n/locales/bs/portal.jsonghost/i18n/locales/ca/portal.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/portal.jsonghost/i18n/locales/da/portal.jsonghost/i18n/locales/de-CH/portal.jsonghost/i18n/locales/de/portal.jsonghost/i18n/locales/el/portal.jsonghost/i18n/locales/en/portal.jsonghost/i18n/locales/eo/portal.jsonghost/i18n/locales/es/portal.jsonghost/i18n/locales/et/portal.jsonghost/i18n/locales/eu/portal.jsonghost/i18n/locales/fa/portal.jsonghost/i18n/locales/fi/portal.jsonghost/i18n/locales/fr/portal.jsonghost/i18n/locales/gd/portal.jsonghost/i18n/locales/he/portal.jsonghost/i18n/locales/hi/portal.jsonghost/i18n/locales/hr/portal.jsonghost/i18n/locales/hu/portal.jsonghost/i18n/locales/id/portal.jsonghost/i18n/locales/is/portal.jsonghost/i18n/locales/it/portal.jsonghost/i18n/locales/ja/portal.jsonghost/i18n/locales/ko/portal.jsonghost/i18n/locales/kz/portal.jsonghost/i18n/locales/lt/portal.jsonghost/i18n/locales/lv/portal.jsonghost/i18n/locales/mk/portal.jsonghost/i18n/locales/mn/portal.jsonghost/i18n/locales/ms/portal.jsonghost/i18n/locales/nb/portal.jsonghost/i18n/locales/ne/portal.jsonghost/i18n/locales/nl/portal.jsonghost/i18n/locales/nn/portal.jsonghost/i18n/locales/pa/portal.jsonghost/i18n/locales/pl/portal.jsonghost/i18n/locales/pt-BR/portal.jsonghost/i18n/locales/pt/portal.jsonghost/i18n/locales/ro/portal.jsonghost/i18n/locales/ru/portal.jsonghost/i18n/locales/si/portal.jsonghost/i18n/locales/sk/portal.jsonghost/i18n/locales/sl/portal.jsonghost/i18n/locales/sq/portal.jsonghost/i18n/locales/sr-Cyrl/portal.jsonghost/i18n/locales/sr/portal.jsonghost/i18n/locales/sv/portal.jsonghost/i18n/locales/sw/portal.jsonghost/i18n/locales/ta/portal.jsonghost/i18n/locales/th/portal.jsonghost/i18n/locales/tr/portal.jsonghost/i18n/locales/uk/portal.jsonghost/i18n/locales/ur/portal.jsonghost/i18n/locales/uz/portal.jsonghost/i18n/locales/vi/portal.jsonghost/i18n/locales/zh-Hant/portal.jsonghost/i18n/locales/zh/portal.json
✅ Files skipped from review due to trivial changes (46)
- apps/portal/src/pages.js
- apps/portal/package.json
- ghost/i18n/locales/ne/portal.json
- apps/portal/src/components/frame.styles.js
- ghost/i18n/locales/pt-BR/portal.json
- ghost/i18n/locales/bg/portal.json
- ghost/i18n/locales/fa/portal.json
- ghost/i18n/locales/gd/portal.json
- ghost/i18n/locales/fr/portal.json
- ghost/i18n/locales/nb/portal.json
- apps/portal/src/components/pages/gift-success-page.js
- ghost/i18n/locales/et/portal.json
- ghost/i18n/locales/id/portal.json
- ghost/i18n/locales/mn/portal.json
- ghost/i18n/locales/sr-Cyrl/portal.json
- ghost/i18n/locales/th/portal.json
- ghost/i18n/locales/bs/portal.json
- ghost/i18n/locales/el/portal.json
- ghost/i18n/locales/sq/portal.json
- ghost/i18n/locales/pt/portal.json
- ghost/i18n/locales/de-CH/portal.json
- ghost/i18n/locales/sr/portal.json
- ghost/i18n/locales/da/portal.json
- ghost/i18n/locales/en/portal.json
- ghost/i18n/locales/ru/portal.json
- ghost/i18n/locales/it/portal.json
- ghost/i18n/locales/ca/portal.json
- ghost/i18n/locales/es/portal.json
- ghost/i18n/locales/zh/portal.json
- ghost/i18n/locales/uz/portal.json
- ghost/i18n/locales/lt/portal.json
- ghost/i18n/locales/uk/portal.json
- ghost/i18n/locales/cs/portal.json
- ghost/i18n/locales/eo/portal.json
- apps/portal/src/utils/api.js
- ghost/i18n/locales/he/portal.json
- ghost/i18n/locales/mk/portal.json
- apps/portal/src/utils/gift-redemption-notification.js
- ghost/i18n/locales/ja/portal.json
- ghost/i18n/locales/ur/portal.json
- ghost/i18n/locales/tr/portal.json
- ghost/i18n/locales/eu/portal.json
- ghost/i18n/locales/pa/portal.json
- ghost/i18n/locales/hi/portal.json
- ghost/i18n/locales/pl/portal.json
- ghost/i18n/locales/lv/portal.json
🚧 Files skipped from review as they are similar to previous changes (20)
- ghost/i18n/locales/si/portal.json
- ghost/i18n/locales/bn/portal.json
- ghost/i18n/locales/ar/portal.json
- ghost/i18n/locales/fi/portal.json
- ghost/i18n/locales/sv/portal.json
- ghost/i18n/locales/kz/portal.json
- ghost/i18n/locales/de/portal.json
- ghost/i18n/locales/sk/portal.json
- ghost/i18n/locales/hr/portal.json
- apps/portal/src/actions.js
- ghost/i18n/locales/is/portal.json
- ghost/i18n/locales/ro/portal.json
- ghost/i18n/locales/sw/portal.json
- ghost/i18n/locales/vi/portal.json
- ghost/i18n/locales/ta/portal.json
- apps/portal/src/utils/helpers.js
- ghost/i18n/locales/ms/portal.json
- ghost/i18n/locales/hu/portal.json
- ghost/i18n/locales/nl/portal.json
- apps/portal/src/components/pages/gift-redemption-page.js
sagzy
force-pushed
the
gift-subs-add-redemption-screen
branch
from
47def9c to
64123cf
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/portal/src/app.js`:
- Around line 177-184: The code awaits fetchGiftRedemptionData (and similarly
fetchRedemptionData) and unconditionally calls this.setState(), which lets
out‑of‑order async completions overwrite newer navigations; fix by tracking a
current redemption identifier on the instance (e.g., this.currentRedemptionToken
or this._redemptionRequestId), set/increment it before calling
fetchGiftRedemptionData({..., token: pageData.token}), capture the
identifier/token in a local variable, and before calling
this.setState(giftLinkData) verify the captured id/token still matches the
current one on the instance – if it doesn’t, drop the stale result and do not
call setState. Ensure the same guard is applied to the other redemption path
(fetchRedemptionData) as well.
- Around line 220-223: fetchData() may throw and the catch block calls
getGiftRedemptionErrorMessage() before the page locale is set, so error text can
be created in the wrong language; to fix, ensure
i18n.changeLanguage(i18nLanguage) is executed before calling fetchData() or else
move creation of the gift error message out of the fetchData() catch handler so
it is produced after locale initialization (reference fetchData(),
getGiftRedemptionErrorMessage(), i18n.changeLanguage and i18nLanguage).
In `@apps/portal/src/utils/helpers.js`:
- Around line 801-805: The current createNotification function resets count to 0
when state.notification is cleared, causing new notifications to reuse previous
notification.count and be treated as already shown; update createNotification
(and related logic around closeNotification and Notification.componentDidUpdate)
to ensure notification.count remains unique across dismissals by persisting a
counter outside state.notification (e.g., module-level or in a dedicated state
key) or by generating a fresh unique id when state.notification is missing;
ensure createNotification sets this external counter (or generates a new unique
id) and assigns it to notification.count so follow-up toasts always render.
🪄 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: 50093d50-1116-495c-86a1-5c1ee6e068aa
📒 Files selected for processing (75)
apps/portal/package.jsonapps/portal/src/actions.jsapps/portal/src/app.jsapps/portal/src/components/frame.styles.jsapps/portal/src/components/notification.jsapps/portal/src/components/pages/gift-redemption-page.jsapps/portal/src/components/pages/gift-success-page.jsapps/portal/src/pages.jsapps/portal/src/utils/api.jsapps/portal/src/utils/gift-redemption-notification.jsapps/portal/src/utils/helpers.jsapps/portal/test/portal-links.test.jsghost/i18n/locales/af/portal.jsonghost/i18n/locales/ar/portal.jsonghost/i18n/locales/bg/portal.jsonghost/i18n/locales/bn/portal.jsonghost/i18n/locales/bs/portal.jsonghost/i18n/locales/ca/portal.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/portal.jsonghost/i18n/locales/da/portal.jsonghost/i18n/locales/de-CH/portal.jsonghost/i18n/locales/de/portal.jsonghost/i18n/locales/el/portal.jsonghost/i18n/locales/en/portal.jsonghost/i18n/locales/eo/portal.jsonghost/i18n/locales/es/portal.jsonghost/i18n/locales/et/portal.jsonghost/i18n/locales/eu/portal.jsonghost/i18n/locales/fa/portal.jsonghost/i18n/locales/fi/portal.jsonghost/i18n/locales/fr/portal.jsonghost/i18n/locales/gd/portal.jsonghost/i18n/locales/he/portal.jsonghost/i18n/locales/hi/portal.jsonghost/i18n/locales/hr/portal.jsonghost/i18n/locales/hu/portal.jsonghost/i18n/locales/id/portal.jsonghost/i18n/locales/is/portal.jsonghost/i18n/locales/it/portal.jsonghost/i18n/locales/ja/portal.jsonghost/i18n/locales/ko/portal.jsonghost/i18n/locales/kz/portal.jsonghost/i18n/locales/lt/portal.jsonghost/i18n/locales/lv/portal.jsonghost/i18n/locales/mk/portal.jsonghost/i18n/locales/mn/portal.jsonghost/i18n/locales/ms/portal.jsonghost/i18n/locales/nb/portal.jsonghost/i18n/locales/ne/portal.jsonghost/i18n/locales/nl/portal.jsonghost/i18n/locales/nn/portal.jsonghost/i18n/locales/pa/portal.jsonghost/i18n/locales/pl/portal.jsonghost/i18n/locales/pt-BR/portal.jsonghost/i18n/locales/pt/portal.jsonghost/i18n/locales/ro/portal.jsonghost/i18n/locales/ru/portal.jsonghost/i18n/locales/si/portal.jsonghost/i18n/locales/sk/portal.jsonghost/i18n/locales/sl/portal.jsonghost/i18n/locales/sq/portal.jsonghost/i18n/locales/sr-Cyrl/portal.jsonghost/i18n/locales/sr/portal.jsonghost/i18n/locales/sv/portal.jsonghost/i18n/locales/sw/portal.jsonghost/i18n/locales/ta/portal.jsonghost/i18n/locales/th/portal.jsonghost/i18n/locales/tr/portal.jsonghost/i18n/locales/uk/portal.jsonghost/i18n/locales/ur/portal.jsonghost/i18n/locales/uz/portal.jsonghost/i18n/locales/vi/portal.jsonghost/i18n/locales/zh-Hant/portal.jsonghost/i18n/locales/zh/portal.json
✅ Files skipped from review due to trivial changes (49)
- apps/portal/src/components/frame.styles.js
- ghost/i18n/locales/bg/portal.json
- apps/portal/package.json
- ghost/i18n/locales/fr/portal.json
- ghost/i18n/locales/nb/portal.json
- apps/portal/src/pages.js
- ghost/i18n/locales/et/portal.json
- ghost/i18n/locales/sv/portal.json
- ghost/i18n/locales/es/portal.json
- apps/portal/src/components/pages/gift-success-page.js
- ghost/i18n/locales/ar/portal.json
- ghost/i18n/locales/id/portal.json
- ghost/i18n/locales/sl/portal.json
- ghost/i18n/locales/el/portal.json
- ghost/i18n/locales/gd/portal.json
- ghost/i18n/locales/mk/portal.json
- ghost/i18n/locales/de-CH/portal.json
- ghost/i18n/locales/fa/portal.json
- ghost/i18n/locales/bs/portal.json
- ghost/i18n/locales/zh/portal.json
- ghost/i18n/locales/it/portal.json
- ghost/i18n/locales/da/portal.json
- ghost/i18n/locales/th/portal.json
- ghost/i18n/locales/sq/portal.json
- ghost/i18n/locales/ko/portal.json
- ghost/i18n/locales/ru/portal.json
- ghost/i18n/locales/bn/portal.json
- ghost/i18n/locales/pt/portal.json
- ghost/i18n/locales/sr/portal.json
- ghost/i18n/locales/sw/portal.json
- ghost/i18n/locales/lt/portal.json
- ghost/i18n/locales/ja/portal.json
- ghost/i18n/locales/ta/portal.json
- ghost/i18n/locales/is/portal.json
- ghost/i18n/locales/tr/portal.json
- ghost/i18n/locales/en/portal.json
- ghost/i18n/locales/he/portal.json
- ghost/i18n/locales/lv/portal.json
- ghost/i18n/locales/uz/portal.json
- ghost/i18n/locales/hu/portal.json
- ghost/i18n/locales/pl/portal.json
- ghost/i18n/locales/nl/portal.json
- ghost/i18n/locales/hi/portal.json
- ghost/i18n/locales/ur/portal.json
- ghost/i18n/locales/eu/portal.json
- apps/portal/src/components/pages/gift-redemption-page.js
- ghost/i18n/locales/zh-Hant/portal.json
- ghost/i18n/locales/vi/portal.json
- ghost/i18n/locales/ro/portal.json
🚧 Files skipped from review as they are similar to previous changes (14)
- ghost/i18n/locales/uk/portal.json
- ghost/i18n/locales/ne/portal.json
- ghost/i18n/locales/mn/portal.json
- ghost/i18n/locales/kz/portal.json
- ghost/i18n/locales/sk/portal.json
- ghost/i18n/locales/de/portal.json
- ghost/i18n/locales/eo/portal.json
- ghost/i18n/locales/nn/portal.json
- apps/portal/src/utils/gift-redemption-notification.js
- apps/portal/src/actions.js
- ghost/i18n/locales/ca/portal.json
- apps/portal/src/utils/api.js
- ghost/i18n/locales/ms/portal.json
- ghost/i18n/locales/pa/portal.json
sagzy
force-pushed
the
gift-subs-add-redemption-screen
branch
from
64123cf to
bafb364
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (5)
apps/portal/src/app.js (3)
217-222:⚠️ Potential issue | 🟠 MajorInitialize the locale before building gift-redemption errors.
fetchData()can create the gift-redemption notification message before the portal language is set, so first-load failures on localized sites will toast in the default language. Move the locale setup ahead offetchData(), or defergetGiftRedemptionErrorMessage()until render time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app.js` around lines 217 - 222, The portal initializes i18n after calling fetchData() inside initSetup(), which allows fetchData() (or code it triggers like getGiftRedemptionErrorMessage()) to build localized messages in the wrong (default) language; move the locale setup earlier by calling i18n.changeLanguage(...) (using this.props.siteI18nEnabled / this.props.locale or site.locale fallback) before invoking fetchData(), or alternatively ensure getGiftRedemptionErrorMessage() is not called inside fetchData() and is deferred until render time so localization happens after initSetup sets i18n.
634-639:⚠️ Potential issue | 🟠 MajorGuard
decodeURIComponent()when reading gift tokens.A malformed
%sequence in the hash ordata-portalattribute throwsURIErrorbefore the API call, which can fail portal initialization or break the custom trigger handler. Please use a safe decode helper and fall back to the raw token when decoding fails.#!/bin/bash set -euo pipefail echo "== Hash-route decoding ==" sed -n '632,640p' apps/portal/src/app.js echo echo "== data-portal decoding ==" sed -n '957,963p' apps/portal/src/app.js echo echo "== JavaScript behavior ==" node - <<'NODE' for (const token of ['gift-token-123', 'bad%ZZ', '%']) { try { console.log(token, '=>', decodeURIComponent(token)); } catch (err) { console.log(token, 'throws', err.name); } } NODEExpected result: the code paths show direct
decodeURIComponent()calls, and the Node snippet demonstrates that malformed tokens throwURIError.Also applies to: 957-963
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app.js` around lines 634 - 639, Guard decodeURIComponent calls when extracting gift tokens: wrap the decode of the matched token (the decodeURIComponent call used with giftRedemptionRegex and the one reading the data-portal attribute) in a small safe-decode helper that catches URIError and returns the original raw token on failure, then pass that safe-decoded value into fetchGiftRedemptionData; update both locations where decodeURIComponent(token) is used so they call the helper instead.
177-184:⚠️ Potential issue | 🟠 MajorIgnore stale gift-redemption fetches before applying state.
Both the custom-trigger path and the hashchange path await redemption data and then always call
setState()with the result. Two quick navigations can resolve out of order and reopen the portal with the wrong gift or an older error state. Please track the current token/request id and drop stale completions.Also applies to: 847-866
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/app.js` around lines 177 - 184, When handling gift redemption fetches (the branches that check page === 'giftRedemption' and use pageData?.token and the separate hashchange path around the other block), record the current token/request id before awaiting fetchGiftRedemptionData(token) and only call this.setState(giftLinkData) if the recorded token still matches the latest token stored on the instance (e.g., this._currentGiftToken or similar); discard the fetched result when they differ so stale responses don't overwrite newer navigations. Ensure both code locations (the custom-trigger path and the hashchange path that call fetchGiftRedemptionData and then this.setState) use the same token-check guard.apps/portal/src/components/notification.js (1)
311-315:⚠️ Potential issue | 🟠 MajorForce
NotificationContentto remount for each new toast.
NotificationContentkeeps its ownclassNameand timeout state, so reusing the same instance here lets the previous auto-hide/slideout leak into the next notification. Key it withnotificationCountso each toast starts clean.#!/bin/bash set -euo pipefail echo "== NotificationContent local state ==" sed -n '150,210p' apps/portal/src/components/notification.js echo echo "== Render path for NotificationContent ==" sed -n '303,317p' apps/portal/src/components/notification.jsExpected result:
NotificationContentowns local state/timeout logic, and the render path shows it being mounted without a changingkey.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/components/notification.js` around lines 311 - 315, The NotificationContent component retains local timeout and className state across toasts because the same instance is reused; update the render in the component that returns <NotificationContent ... /> (inside the block using this.state.type/status/message/autoHide/duration) to force a remount by adding a unique key prop derived from the notification counter (e.g., key={this.state.notificationCount} or the notificationCount variable) so each toast mounts fresh; ensure the key uses the changing notificationCount value managed where notifications are created/updated (refer to NotificationContent and notificationCount) and no other props need changing.apps/portal/src/utils/helpers.js (1)
801-805:⚠️ Potential issue | 🟠 MajorKeep
notification.countmonotonic across dismissals.When
closeNotificationclearsstate.notification, this falls back tocount = 0again.Notificationkeeps the previousnotificationCount, so the next state-backed toast can be treated as already shown and never render. Please generate the id independently ofstate.notificationinstead of deriving it from the current state object.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/utils/helpers.js` around lines 801 - 805, createNotification currently derives the new notification id from state.notification.count, which resets when closeNotification clears state and can cause duplicate/ignored toasts; change createNotification to generate the id independently (e.g., use a module-scoped incremental counter or timestamp) so ids are monotonic across dismissals and do not rely on state.notification; update createNotification to increment and use that independent counter for the id and leave closeNotification/state logic unchanged; reference createNotification and closeNotification/Notification behavior so you locate where to replace state-based counting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/portal/src/app.js`:
- Around line 217-222: The portal initializes i18n after calling fetchData()
inside initSetup(), which allows fetchData() (or code it triggers like
getGiftRedemptionErrorMessage()) to build localized messages in the wrong
(default) language; move the locale setup earlier by calling
i18n.changeLanguage(...) (using this.props.siteI18nEnabled / this.props.locale
or site.locale fallback) before invoking fetchData(), or alternatively ensure
getGiftRedemptionErrorMessage() is not called inside fetchData() and is deferred
until render time so localization happens after initSetup sets i18n.
- Around line 634-639: Guard decodeURIComponent calls when extracting gift
tokens: wrap the decode of the matched token (the decodeURIComponent call used
with giftRedemptionRegex and the one reading the data-portal attribute) in a
small safe-decode helper that catches URIError and returns the original raw
token on failure, then pass that safe-decoded value into
fetchGiftRedemptionData; update both locations where decodeURIComponent(token)
is used so they call the helper instead.
- Around line 177-184: When handling gift redemption fetches (the branches that
check page === 'giftRedemption' and use pageData?.token and the separate
hashchange path around the other block), record the current token/request id
before awaiting fetchGiftRedemptionData(token) and only call
this.setState(giftLinkData) if the recorded token still matches the latest token
stored on the instance (e.g., this._currentGiftToken or similar); discard the
fetched result when they differ so stale responses don't overwrite newer
navigations. Ensure both code locations (the custom-trigger path and the
hashchange path that call fetchGiftRedemptionData and then this.setState) use
the same token-check guard.
In `@apps/portal/src/components/notification.js`:
- Around line 311-315: The NotificationContent component retains local timeout
and className state across toasts because the same instance is reused; update
the render in the component that returns <NotificationContent ... /> (inside the
block using this.state.type/status/message/autoHide/duration) to force a remount
by adding a unique key prop derived from the notification counter (e.g.,
key={this.state.notificationCount} or the notificationCount variable) so each
toast mounts fresh; ensure the key uses the changing notificationCount value
managed where notifications are created/updated (refer to NotificationContent
and notificationCount) and no other props need changing.
In `@apps/portal/src/utils/helpers.js`:
- Around line 801-805: createNotification currently derives the new notification
id from state.notification.count, which resets when closeNotification clears
state and can cause duplicate/ignored toasts; change createNotification to
generate the id independently (e.g., use a module-scoped incremental counter or
timestamp) so ids are monotonic across dismissals and do not rely on
state.notification; update createNotification to increment and use that
independent counter for the id and leave closeNotification/state logic
unchanged; reference createNotification and closeNotification/Notification
behavior so you locate where to replace state-based counting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 44805b62-2382-469f-8e1e-3fbefa4232ed
📒 Files selected for processing (75)
apps/portal/package.jsonapps/portal/src/actions.jsapps/portal/src/app.jsapps/portal/src/components/frame.styles.jsapps/portal/src/components/notification.jsapps/portal/src/components/pages/gift-redemption-page.jsapps/portal/src/components/pages/gift-success-page.jsapps/portal/src/pages.jsapps/portal/src/utils/api.jsapps/portal/src/utils/gift-redemption-notification.jsapps/portal/src/utils/helpers.jsapps/portal/test/portal-links.test.jsghost/i18n/locales/af/portal.jsonghost/i18n/locales/ar/portal.jsonghost/i18n/locales/bg/portal.jsonghost/i18n/locales/bn/portal.jsonghost/i18n/locales/bs/portal.jsonghost/i18n/locales/ca/portal.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/portal.jsonghost/i18n/locales/da/portal.jsonghost/i18n/locales/de-CH/portal.jsonghost/i18n/locales/de/portal.jsonghost/i18n/locales/el/portal.jsonghost/i18n/locales/en/portal.jsonghost/i18n/locales/eo/portal.jsonghost/i18n/locales/es/portal.jsonghost/i18n/locales/et/portal.jsonghost/i18n/locales/eu/portal.jsonghost/i18n/locales/fa/portal.jsonghost/i18n/locales/fi/portal.jsonghost/i18n/locales/fr/portal.jsonghost/i18n/locales/gd/portal.jsonghost/i18n/locales/he/portal.jsonghost/i18n/locales/hi/portal.jsonghost/i18n/locales/hr/portal.jsonghost/i18n/locales/hu/portal.jsonghost/i18n/locales/id/portal.jsonghost/i18n/locales/is/portal.jsonghost/i18n/locales/it/portal.jsonghost/i18n/locales/ja/portal.jsonghost/i18n/locales/ko/portal.jsonghost/i18n/locales/kz/portal.jsonghost/i18n/locales/lt/portal.jsonghost/i18n/locales/lv/portal.jsonghost/i18n/locales/mk/portal.jsonghost/i18n/locales/mn/portal.jsonghost/i18n/locales/ms/portal.jsonghost/i18n/locales/nb/portal.jsonghost/i18n/locales/ne/portal.jsonghost/i18n/locales/nl/portal.jsonghost/i18n/locales/nn/portal.jsonghost/i18n/locales/pa/portal.jsonghost/i18n/locales/pl/portal.jsonghost/i18n/locales/pt-BR/portal.jsonghost/i18n/locales/pt/portal.jsonghost/i18n/locales/ro/portal.jsonghost/i18n/locales/ru/portal.jsonghost/i18n/locales/si/portal.jsonghost/i18n/locales/sk/portal.jsonghost/i18n/locales/sl/portal.jsonghost/i18n/locales/sq/portal.jsonghost/i18n/locales/sr-Cyrl/portal.jsonghost/i18n/locales/sr/portal.jsonghost/i18n/locales/sv/portal.jsonghost/i18n/locales/sw/portal.jsonghost/i18n/locales/ta/portal.jsonghost/i18n/locales/th/portal.jsonghost/i18n/locales/tr/portal.jsonghost/i18n/locales/uk/portal.jsonghost/i18n/locales/ur/portal.jsonghost/i18n/locales/uz/portal.jsonghost/i18n/locales/vi/portal.jsonghost/i18n/locales/zh-Hant/portal.jsonghost/i18n/locales/zh/portal.json
✅ Files skipped from review due to trivial changes (49)
- ghost/i18n/locales/uk/portal.json
- ghost/i18n/locales/bg/portal.json
- ghost/i18n/locales/bn/portal.json
- ghost/i18n/locales/mk/portal.json
- apps/portal/src/components/pages/gift-success-page.js
- ghost/i18n/locales/et/portal.json
- ghost/i18n/locales/es/portal.json
- ghost/i18n/locales/fi/portal.json
- ghost/i18n/locales/ar/portal.json
- apps/portal/src/pages.js
- ghost/i18n/locales/id/portal.json
- ghost/i18n/locales/pa/portal.json
- ghost/i18n/locales/el/portal.json
- ghost/i18n/locales/ko/portal.json
- ghost/i18n/locales/fr/portal.json
- ghost/i18n/locales/cs/portal.json
- ghost/i18n/locales/de-CH/portal.json
- ghost/i18n/locales/da/portal.json
- ghost/i18n/locales/sq/portal.json
- ghost/i18n/locales/mn/portal.json
- ghost/i18n/locales/th/portal.json
- ghost/i18n/locales/sl/portal.json
- ghost/i18n/locales/ne/portal.json
- ghost/i18n/locales/gd/portal.json
- ghost/i18n/locales/it/portal.json
- ghost/i18n/locales/zh/portal.json
- ghost/i18n/locales/si/portal.json
- ghost/i18n/locales/sk/portal.json
- ghost/i18n/locales/ca/portal.json
- apps/portal/src/components/frame.styles.js
- ghost/i18n/locales/hi/portal.json
- ghost/i18n/locales/eo/portal.json
- ghost/i18n/locales/is/portal.json
- ghost/i18n/locales/lt/portal.json
- apps/portal/src/utils/gift-redemption-notification.js
- ghost/i18n/locales/tr/portal.json
- ghost/i18n/locales/ru/portal.json
- ghost/i18n/locales/pl/portal.json
- ghost/i18n/locales/ro/portal.json
- ghost/i18n/locales/ms/portal.json
- ghost/i18n/locales/lv/portal.json
- ghost/i18n/locales/ja/portal.json
- ghost/i18n/locales/en/portal.json
- ghost/i18n/locales/hu/portal.json
- ghost/i18n/locales/ta/portal.json
- ghost/i18n/locales/sr/portal.json
- ghost/i18n/locales/eu/portal.json
- ghost/i18n/locales/sw/portal.json
- apps/portal/src/components/pages/gift-redemption-page.js
🚧 Files skipped from review as they are similar to previous changes (18)
- apps/portal/package.json
- ghost/i18n/locales/pt-BR/portal.json
- apps/portal/src/utils/api.js
- ghost/i18n/locales/bs/portal.json
- ghost/i18n/locales/nb/portal.json
- ghost/i18n/locales/fa/portal.json
- ghost/i18n/locales/de/portal.json
- ghost/i18n/locales/hr/portal.json
- ghost/i18n/locales/uz/portal.json
- ghost/i18n/locales/sr-Cyrl/portal.json
- ghost/i18n/locales/vi/portal.json
- ghost/i18n/locales/nl/portal.json
- ghost/i18n/locales/nn/portal.json
- ghost/i18n/locales/pt/portal.json
- ghost/i18n/locales/he/portal.json
- ghost/i18n/locales/ur/portal.json
- ghost/i18n/locales/af/portal.json
- ghost/i18n/locales/context.json
sagzy
force-pushed
the
gift-subs-add-redemption-screen
branch
from
bafb364 to
7067ebd
Compare
ref https://linear.app/ghost/issue/BER-3476 - added static UI for design team to continue with the UI, while engineers integrate with the backend - note: this uses a mocked api response, that is obviously temporary
sagzy
force-pushed
the
gift-subs-add-redemption-screen
branch
from
7067ebd to
9bc7266
Compare
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/portal/src/utils/gift-redemption-notification.js (1)
3-13:⚠️ Potential issue | 🟠 MajorLocalize notification title/subtitle instead of returning raw English/backend strings.
titleand fallback subtitle are hard-coded English, and Line 8 can surface raw backend messages directly. In non-English locales this will render mixed-language notifications.💡 Suggested direction
-const GIFT_REDEMPTION_ERROR_TITLE = 'Gift could not be redeemed'; -const INVALID_GIFT_LINK_MESSAGE = 'Gift link is not valid'; +const GIFT_REDEMPTION_ERROR_TITLE = 'Gift could not be redeemed'; +const INVALID_GIFT_LINK_MESSAGE = 'Gift link is not valid'; export function getGiftRedemptionErrorMessage(error) { - const subtitle = error?.message && error.message !== 'Failed to load gift data' - ? error.message - : INVALID_GIFT_LINK_MESSAGE; + let subtitle; + switch (error?.message) { + case 'This gift has already been redeemed.': + subtitle = t('This gift has already been redeemed.'); + break; + case 'This gift has expired.': + subtitle = t('This gift has expired.'); + break; + default: + subtitle = t(INVALID_GIFT_LINK_MESSAGE); + } return { - title: GIFT_REDEMPTION_ERROR_TITLE, + title: t(GIFT_REDEMPTION_ERROR_TITLE), subtitle }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/utils/gift-redemption-notification.js` around lines 3 - 13, The title and fallback subtitle in getGiftRedemptionErrorMessage (and constants GIFT_REDEMPTION_ERROR_TITLE, INVALID_GIFT_LINK_MESSAGE) are hard-coded English or pass-through backend text; replace them with localized strings by using the app's i18n/localization helper (e.g., import and call the translation function or message IDs) for the title and the INVALID_GIFT_LINK_MESSAGE fallback, and ensure any backend error.message is mapped or wrapped via localization (or sanitized and fed into a localized template) before returning the { title, subtitle } object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/portal/src/components/pages/gift-redemption-page.js`:
- Around line 202-203: The kicker, title and CTA strings inside the
GiftRedemptionPage component are hard-coded English; update GiftRedemptionPage
to wrap those literal strings with the i18n translator t() (e.g.,
t('giftRedemption.kicker'), t('giftRedemption.title'), t('giftRedemption.cta'))
and add matching translation keys, and do the same for the other hard-coded
strings in the same file noted around the secondary occurrences (the strings at
the later return blocks). Ensure you import/use the existing t function or use
the same translation hook used elsewhere in this file so all labels render
localized text.
- Around line 224-237: The two UI updates (doAction('openNotification') and
doAction('closePopup')) race and cause NotificationContent to slide out while
showPopup is still true; collapse them into one coordinated transition by
closing the popup first and only opening the notification after the close
completes. In the useEffect that currently calls doAction('openNotification')
and doAction('closePopup'), replace the back-to-back calls with a coordinated
sequence: call doAction('closePopup') and then open the notification in the
close completion (either by awaiting a promise returned from
doAction('closePopup') if available, or by posting the openNotification call to
the next tick via setTimeout(..., 0)). Target the useEffect, the doAction calls,
and NotificationContent behavior so the popup close finishes before triggering
openNotification.
In `@apps/portal/src/utils/api.js`:
- Around line 170-201: The current api.gift.fetchRedemptionData mock returns a
successful redemption for runtime use which makes live routes appear valid;
change fetchRedemptionData (the function on api.gift used by setupGhostApi and
consumed by fetchGiftRedemptionData/GiftRedemptionPage) to fail-closed by
default (throw an error or return a rejected Promise / 404-like response) unless
explicitly running in a test/dev fixture mode; move the happy-path mock into
test-only fixtures or gate it behind a clear flag (e.g., NODE_ENV === 'test' or
a dedicated mock flag) so the live portal does not treat tokens as valid.
---
Duplicate comments:
In `@apps/portal/src/utils/gift-redemption-notification.js`:
- Around line 3-13: The title and fallback subtitle in
getGiftRedemptionErrorMessage (and constants GIFT_REDEMPTION_ERROR_TITLE,
INVALID_GIFT_LINK_MESSAGE) are hard-coded English or pass-through backend text;
replace them with localized strings by using the app's i18n/localization helper
(e.g., import and call the translation function or message IDs) for the title
and the INVALID_GIFT_LINK_MESSAGE fallback, and ensure any backend error.message
is mapped or wrapped via localization (or sanitized and fed into a localized
template) before returning the { title, subtitle } object.
🪄 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: d153fdbd-70f6-4040-b223-b7600174d753
📒 Files selected for processing (78)
apps/portal/package.jsonapps/portal/src/actions.jsapps/portal/src/app.jsapps/portal/src/components/frame.styles.jsapps/portal/src/components/notification.jsapps/portal/src/components/pages/gift-redemption-page.jsapps/portal/src/components/pages/gift-success-page.jsapps/portal/src/pages.jsapps/portal/src/utils/api.jsapps/portal/src/utils/gift-redemption-notification.jsapps/portal/src/utils/helpers.jsapps/portal/test/actions.test.tsapps/portal/test/app.test.jsapps/portal/test/portal-links.test.jsapps/portal/test/unit/components/notification.test.jsghost/i18n/locales/af/portal.jsonghost/i18n/locales/ar/portal.jsonghost/i18n/locales/bg/portal.jsonghost/i18n/locales/bn/portal.jsonghost/i18n/locales/bs/portal.jsonghost/i18n/locales/ca/portal.jsonghost/i18n/locales/context.jsonghost/i18n/locales/cs/portal.jsonghost/i18n/locales/da/portal.jsonghost/i18n/locales/de-CH/portal.jsonghost/i18n/locales/de/portal.jsonghost/i18n/locales/el/portal.jsonghost/i18n/locales/en/portal.jsonghost/i18n/locales/eo/portal.jsonghost/i18n/locales/es/portal.jsonghost/i18n/locales/et/portal.jsonghost/i18n/locales/eu/portal.jsonghost/i18n/locales/fa/portal.jsonghost/i18n/locales/fi/portal.jsonghost/i18n/locales/fr/portal.jsonghost/i18n/locales/gd/portal.jsonghost/i18n/locales/he/portal.jsonghost/i18n/locales/hi/portal.jsonghost/i18n/locales/hr/portal.jsonghost/i18n/locales/hu/portal.jsonghost/i18n/locales/id/portal.jsonghost/i18n/locales/is/portal.jsonghost/i18n/locales/it/portal.jsonghost/i18n/locales/ja/portal.jsonghost/i18n/locales/ko/portal.jsonghost/i18n/locales/kz/portal.jsonghost/i18n/locales/lt/portal.jsonghost/i18n/locales/lv/portal.jsonghost/i18n/locales/mk/portal.jsonghost/i18n/locales/mn/portal.jsonghost/i18n/locales/ms/portal.jsonghost/i18n/locales/nb/portal.jsonghost/i18n/locales/ne/portal.jsonghost/i18n/locales/nl/portal.jsonghost/i18n/locales/nn/portal.jsonghost/i18n/locales/pa/portal.jsonghost/i18n/locales/pl/portal.jsonghost/i18n/locales/pt-BR/portal.jsonghost/i18n/locales/pt/portal.jsonghost/i18n/locales/ro/portal.jsonghost/i18n/locales/ru/portal.jsonghost/i18n/locales/si/portal.jsonghost/i18n/locales/sk/portal.jsonghost/i18n/locales/sl/portal.jsonghost/i18n/locales/sq/portal.jsonghost/i18n/locales/sr-Cyrl/portal.jsonghost/i18n/locales/sr/portal.jsonghost/i18n/locales/sv/portal.jsonghost/i18n/locales/sw/portal.jsonghost/i18n/locales/ta/portal.jsonghost/i18n/locales/th/portal.jsonghost/i18n/locales/tr/portal.jsonghost/i18n/locales/uk/portal.jsonghost/i18n/locales/ur/portal.jsonghost/i18n/locales/uz/portal.jsonghost/i18n/locales/vi/portal.jsonghost/i18n/locales/zh-Hant/portal.jsonghost/i18n/locales/zh/portal.json
✅ Files skipped from review due to trivial changes (53)
- apps/portal/package.json
- ghost/i18n/locales/sl/portal.json
- ghost/i18n/locales/mk/portal.json
- ghost/i18n/locales/et/portal.json
- ghost/i18n/locales/el/portal.json
- ghost/i18n/locales/mn/portal.json
- ghost/i18n/locales/nb/portal.json
- ghost/i18n/locales/uz/portal.json
- ghost/i18n/locales/fa/portal.json
- apps/portal/src/pages.js
- apps/portal/src/components/frame.styles.js
- ghost/i18n/locales/th/portal.json
- ghost/i18n/locales/bn/portal.json
- ghost/i18n/locales/ne/portal.json
- ghost/i18n/locales/sv/portal.json
- ghost/i18n/locales/sq/portal.json
- ghost/i18n/locales/af/portal.json
- apps/portal/src/components/pages/gift-success-page.js
- ghost/i18n/locales/gd/portal.json
- ghost/i18n/locales/es/portal.json
- ghost/i18n/locales/hi/portal.json
- ghost/i18n/locales/fr/portal.json
- ghost/i18n/locales/uk/portal.json
- ghost/i18n/locales/nn/portal.json
- ghost/i18n/locales/zh/portal.json
- ghost/i18n/locales/pa/portal.json
- ghost/i18n/locales/tr/portal.json
- ghost/i18n/locales/da/portal.json
- ghost/i18n/locales/lt/portal.json
- ghost/i18n/locales/is/portal.json
- ghost/i18n/locales/ru/portal.json
- ghost/i18n/locales/sk/portal.json
- ghost/i18n/locales/bs/portal.json
- ghost/i18n/locales/de-CH/portal.json
- ghost/i18n/locales/cs/portal.json
- ghost/i18n/locales/sw/portal.json
- ghost/i18n/locales/he/portal.json
- ghost/i18n/locales/nl/portal.json
- ghost/i18n/locales/eu/portal.json
- ghost/i18n/locales/en/portal.json
- ghost/i18n/locales/lv/portal.json
- apps/portal/test/app.test.js
- ghost/i18n/locales/eo/portal.json
- ghost/i18n/locales/ur/portal.json
- ghost/i18n/locales/hu/portal.json
- ghost/i18n/locales/vi/portal.json
- ghost/i18n/locales/pt/portal.json
- ghost/i18n/locales/hr/portal.json
- ghost/i18n/locales/kz/portal.json
- ghost/i18n/locales/ms/portal.json
- ghost/i18n/locales/pl/portal.json
- ghost/i18n/locales/ca/portal.json
- ghost/i18n/locales/ko/portal.json
🚧 Files skipped from review as they are similar to previous changes (11)
- ghost/i18n/locales/ar/portal.json
- ghost/i18n/locales/si/portal.json
- ghost/i18n/locales/de/portal.json
- ghost/i18n/locales/ja/portal.json
- ghost/i18n/locales/fi/portal.json
- ghost/i18n/locales/it/portal.json
- ghost/i18n/locales/sr-Cyrl/portal.json
- ghost/i18n/locales/pt-BR/portal.json
- ghost/i18n/locales/ro/portal.json
- apps/portal/src/actions.js
- apps/portal/src/utils/helpers.js
9larsons
added a commit
that referenced
this pull request
Apr 7, 2026
ref https://linear.app/ghost/issue/BER-3476 - added static UI for gift redemption for design team to continue with the UI, while engineers integrate with the backend. This uses mocked API responses that are temporary - the new page `#/portal/gift/redeem/<token>` is behind a feature flag: if flag is disabled, the gift redemption popup does not render # Conflicts: # apps/portal/package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref https://linear.app/ghost/issue/BER-3476
#/portal/gift/redeem/<token>is behind a feature flag: if flag is disabled, the gift redemption popup does not render