Implemented gift subscription checkout UI#27060
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughUpdated the portal package version to 2.67.4. Replaced the static gift page with a stateful GiftPage that manages recipient email and validation, selected billing interval, derives an active interval via a new shared helper, loads and filters paid products, renders product cards with interval pricing and discounts, and stubs purchase handling (disabled when cookies are disabled). Popup modal now forces the gift page to render full-size. Tests and helper tests updated to reflect the new active-interval logic and revised gift-page copy. Possibly related PRs
🚥 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)
Comment |
mike182uk
force-pushed
the
BER-3484-add-gift-checkout
branch
from
4697155 to
59c27fb
Compare
![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: 4
🧹 Nitpick comments (1)
apps/portal/test/portal-links.test.js (1)
347-365: Expand this smoke test beyond the subtitle.The new gift page is stateful, so this still passes if the email field or purchase cards stop rendering. Adding a quick assertion for the purchaser email input and at least one
Purchase giftbutton would give the flow basic coverage.🤖 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 347 - 365, The test for '#/portal/gift' currently only asserts the gift subtitle; extend it to assert the presence of the purchaser email input and at least one "Purchase gift" action so the page state is exercised; after locating popupFrame via utils.findByTitle (and using within(popupFrame.contentDocument)), add assertions that a purchaser email input exists (e.g., queryByLabelText / purchaser email / or by placeholder/email role) and that a button with text like /purchase gift/i (or a role="button" with that name) is present and in the document to ensure the form and purchase call-to-action render correctly.
🤖 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-page.js`:
- Around line 144-162: getAvailableProducts is currently excluding products that
only have a monthly or yearly price, causing products to be empty for
single-cadence sites; update getAvailableProducts (in
apps/portal/src/utils/helpers.js) to return products that have a price for at
least one cadence instead of requiring both monthlyPrice and yearlyPrice, and
make its filtering respect the site's portal_plans/portal_default_plan (used in
gift-page.js) so monthly-only or yearly-only products are included; ensure the
call in gift-page.js (where products = getAvailableProducts({site}).filter(p =>
p.type === 'paid')) receives products that match the site's allowed cadences so
the subsequent hasGiftSubscriptions check can work correctly.
- Around line 205-217: The handlePurchase handler currently only validates email
and returns; update handlePurchase to accept the selected product context (e.g.,
priceId and optional interval) from the card that invokes it, run
ValidateInputForm({fields: emailField}) and setEmailError as before, then call
the existing checkout flow (or implement an
initiateGiftCheckout/createCheckoutSession call) with the validated email and
the passed priceId/interval so the checkout knows which product to purchase;
also update the card renderers to pass a bound callback like () =>
handlePurchase(priceId, interval, e) (or supply priceId when wiring the onClick)
so each card provides its product context.
- Around line 12-13: The i18n-disable comment in
apps/portal/src/components/pages/gift-page.js must be removed and all
user-facing literal strings in the GiftPage component replaced with calls to the
i18n translator t('portal.<key>') (or use the existing useTranslation hook) —
update each label/button/error in functions/render methods in GiftPage to use
t() with meaningful keys, then add those keys and translations into the
portal.json namespace for each target locale under
ghost/i18n/locales/{locale}/portal.json; ensure no remaining /* eslint-disable
i18next/no-literal-string */ remains in this file so the linter enforces future
i18n.
- Around line 104-127: The global highestDiscount value is computed across all
products and can overstate savings for the plan the user actually selects;
change the logic to compute and display the discount for the product(s)
currently shown/selected instead of Math.max(...discounts). Locate where
discounts and highestDiscount are computed and replace with a per-product
discount using calculateDiscount(product.monthlyPrice?.amount,
product.yearlyPrice?.amount) for the active product (e.g., the product at the
current selection index or the single featured product), or alternatively remove
the global badge and render per-plan save labels next to each plan; update any
UI bits that reference highestDiscount (the Yearly toggle span) to use that
per-product discount value and the selectedInterval/selection state (e.g.,
selectedInterval, setSelectedInterval).
---
Nitpick comments:
In `@apps/portal/test/portal-links.test.js`:
- Around line 347-365: The test for '#/portal/gift' currently only asserts the
gift subtitle; extend it to assert the presence of the purchaser email input and
at least one "Purchase gift" action so the page state is exercised; after
locating popupFrame via utils.findByTitle (and using
within(popupFrame.contentDocument)), add assertions that a purchaser email input
exists (e.g., queryByLabelText / purchaser email / or by placeholder/email role)
and that a button with text like /purchase gift/i (or a role="button" with that
name) is present and in the document to ensure the form and purchase
call-to-action render correctly.
🪄 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: c403ffd8-312e-4159-9d89-0af23d84b8f3
📒 Files selected for processing (4)
apps/portal/package.jsonapps/portal/src/components/pages/gift-page.jsapps/portal/src/components/popup-modal.jsapps/portal/test/portal-links.test.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/portal/src/components/pages/gift-page.js (2)
146-158: Consider memoizingactiveIntervalderivation.The
activeIntervalcalculation runs on every render. While functionally correct, it could be extracted or memoized for clarity:Optional: Use a helper function
const getDefaultInterval = (portalPlans, portalDefaultPlan) => { if (portalDefaultPlan === 'monthly' && portalPlans?.includes('monthly')) { return 'month'; } if (portalPlans?.includes('yearly')) { return 'year'; } if (portalPlans?.includes('monthly')) { return 'month'; } return 'year'; }; // Then use: const activeInterval = selectedInterval || getDefaultInterval(portalPlans, portalDefaultPlan);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/components/pages/gift-page.js` around lines 146 - 158, The inline derivation of activeInterval (using selectedInterval, portalDefaultPlan, and portalPlans) runs on every render; extract and memoize it by creating a helper such as getDefaultInterval(portalPlans, portalDefaultPlan) and then set const activeInterval = selectedInterval || getDefaultInterval(portalPlans, portalDefaultPlan) (or wrap that in React.useMemo if preferred) so the logic is centralized, clearer, and avoids recalculating on each render; update references to activeInterval in the component to use this new value.
104-105: Discount calculation also assumes both prices exist.Similar to
GiftProductCardPrice, this code would produce incorrect results ifgetAvailableProductsis updated to include single-interval products.calculateDiscountwould receiveundefinedfor the missing price.Consider guarding the calculation:
Defensive discount calculation
- const discounts = products.map(p => calculateDiscount(p.monthlyPrice?.amount, p.yearlyPrice?.amount)); + const discounts = products + .filter(p => p.monthlyPrice && p.yearlyPrice) + .map(p => calculateDiscount(p.monthlyPrice.amount, p.yearlyPrice.amount)); const highestDiscount = Math.max(...discounts);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/components/pages/gift-page.js` around lines 104 - 105, The discount mapping assumes both prices exist; update the discount computation that builds discounts and highestDiscount so it only calls calculateDiscount for products with both monthlyPrice?.amount and yearlyPrice?.amount (or alternatively ensure calculateDiscount handles undefined safely). Specifically, in the block that defines discounts and highestDiscount, filter products (from getAvailableProducts) to those having monthlyPrice?.amount and yearlyPrice?.amount before mapping to calculateDiscount, and compute Math.max over that filtered list (fallback to 0 if none), so calculateDiscount is never invoked with undefined; mirror the same defensive check used in GiftProductCardPrice.
🤖 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-page.js`:
- Around line 36-42: GiftProductCardPrice currently returns null if either
monthlyPrice or yearlyPrice is missing, so when getAvailableProducts is changed
to allow single-interval products this component will render nothing; update
GiftProductCardPrice to tolerate missing intervals by: determine an activePrice
based on selectedInterval but fallback to whichever of monthlyPrice or
yearlyPrice exists (or the only one present), only return null if neither price
exists, and ensure display logic uses activePrice and a label derived from
selectedInterval or the price's interval; reference the GiftProductCardPrice
function, selectedInterval, monthlyPrice, yearlyPrice and getAvailableProducts
when making the change.
---
Nitpick comments:
In `@apps/portal/src/components/pages/gift-page.js`:
- Around line 146-158: The inline derivation of activeInterval (using
selectedInterval, portalDefaultPlan, and portalPlans) runs on every render;
extract and memoize it by creating a helper such as
getDefaultInterval(portalPlans, portalDefaultPlan) and then set const
activeInterval = selectedInterval || getDefaultInterval(portalPlans,
portalDefaultPlan) (or wrap that in React.useMemo if preferred) so the logic is
centralized, clearer, and avoids recalculating on each render; update references
to activeInterval in the component to use this new value.
- Around line 104-105: The discount mapping assumes both prices exist; update
the discount computation that builds discounts and highestDiscount so it only
calls calculateDiscount for products with both monthlyPrice?.amount and
yearlyPrice?.amount (or alternatively ensure calculateDiscount handles undefined
safely). Specifically, in the block that defines discounts and highestDiscount,
filter products (from getAvailableProducts) to those having monthlyPrice?.amount
and yearlyPrice?.amount before mapping to calculateDiscount, and compute
Math.max over that filtered list (fallback to 0 if none), so calculateDiscount
is never invoked with undefined; mirror the same defensive check used in
GiftProductCardPrice.
🪄 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: fb2eff60-472f-4bb5-a1ce-707c8ed22ef7
📒 Files selected for processing (4)
apps/portal/package.jsonapps/portal/src/components/pages/gift-page.jsapps/portal/src/components/popup-modal.jsapps/portal/test/portal-links.test.js
✅ Files skipped from review due to trivial changes (2)
- apps/portal/package.json
- apps/portal/test/portal-links.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/portal/src/components/popup-modal.js
mike182uk
force-pushed
the
BER-3484-add-gift-checkout
branch
from
59c27fb to
1606b8f
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/portal/src/components/pages/gift-page.js (1)
15-33: Consider adding explicit return for completeness.The function can implicitly return
undefinedwhenportalPlanscontains neither'monthly'nor'yearly'. While this case is protected by the early return at line 170-195 (sincegetAvailableProductsreturns an empty array when neither interval is configured), an explicit return would make the function's contract clearer.📝 Optional: Add explicit undefined return
if (portalPlans.includes('monthly')) { return 'month'; } + return undefined; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/src/components/pages/gift-page.js` around lines 15 - 33, getActiveInterval may implicitly return undefined when portalPlans contains neither 'monthly' nor 'yearly'; update the function (getActiveInterval) to include an explicit final return (e.g., return undefined or a clear default) so the function's contract is explicit and callers don't rely on implicit behavior — locate getActiveInterval in gift-page.js and add the explicit return at the end after the existing checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/portal/src/components/pages/gift-page.js`:
- Around line 15-33: getActiveInterval may implicitly return undefined when
portalPlans contains neither 'monthly' nor 'yearly'; update the function
(getActiveInterval) to include an explicit final return (e.g., return undefined
or a clear default) so the function's contract is explicit and callers don't
rely on implicit behavior — locate getActiveInterval in gift-page.js and add the
explicit return at the end after the existing checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e72e986f-ce17-453d-b502-78e9c9680847
📒 Files selected for processing (4)
apps/portal/package.jsonapps/portal/src/components/pages/gift-page.jsapps/portal/src/components/popup-modal.jsapps/portal/test/portal-links.test.js
✅ Files skipped from review due to trivial changes (2)
- apps/portal/test/portal-links.test.js
- apps/portal/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/portal/src/components/popup-modal.js
mike182uk
force-pushed
the
BER-3484-add-gift-checkout
branch
from
1606b8f to
99a152c
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/portal/test/utils/helpers.test.js (1)
843-875: Good test coverage for the new helper.The tests cover the key scenarios: selection priority, default plan fallback, and handling unavailable intervals.
Consider adding an edge case for an empty
portalPlansarray to document the expectedundefinedreturn:test('returns undefined when no plans are available', () => { expect(getActiveInterval({portalPlans: [], portalDefaultPlan: null, selectedInterval: null})).toBeUndefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/test/utils/helpers.test.js` around lines 843 - 875, Add a test case to the getActiveInterval test suite to cover the edge case where portalPlans is an empty array: call getActiveInterval({portalPlans: [], portalDefaultPlan: null, selectedInterval: null}) and assert it returns undefined (use toBeUndefined()); place the new test alongside the existing tests in the describe('getActiveInterval', ...) block so the behavior for no available plans is documented and validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/portal/test/utils/helpers.test.js`:
- Around line 843-875: Add a test case to the getActiveInterval test suite to
cover the edge case where portalPlans is an empty array: call
getActiveInterval({portalPlans: [], portalDefaultPlan: null, selectedInterval:
null}) and assert it returns undefined (use toBeUndefined()); place the new test
alongside the existing tests in the describe('getActiveInterval', ...) block so
the behavior for no available plans is documented and validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3854f12-6cb8-4227-a4b4-2be3e6ffda41
📒 Files selected for processing (7)
apps/portal/package.jsonapps/portal/src/components/common/products-section.jsapps/portal/src/components/pages/gift-page.jsapps/portal/src/components/popup-modal.jsapps/portal/src/utils/helpers.jsapps/portal/test/portal-links.test.jsapps/portal/test/utils/helpers.test.js
✅ Files skipped from review due to trivial changes (2)
- apps/portal/package.json
- apps/portal/test/portal-links.test.js
ref https://linear.app/ghost/issue/BER-3484 Implemented the gift page content behind the `giftSubscriptions` labs flag: site header, purchaser email input, monthly/yearly toggle, paid tier cards with pricing and benefits, and a "Purchase gift" button
mike182uk
force-pushed
the
BER-3484-add-gift-checkout
branch
from
99a152c to
0d664e7
Compare
|
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
1 participant
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-3484
Implemented the gift page content behind the
giftSubscriptionslabs flag: site header, purchaser email input, monthly/yearly toggle, paid tier cards with pricing and benefits, and aPurchase giftbutton