Fixed retention offers not rendering for members on archived tiers#26672
Fixed retention offers not rendering for members on archived tiers#26672
Conversation
closes https://linear.app/ghost/issue/BER-3406 - previously, we expected the /tiers endpoint to always have tier-related data to render retention offer popups - however, archived tiers are not returned as part of the /tiers endpoint - with this fix, we directly get necessary data from the member subscription object
Changelog for v2.65.0 -> 2.65.1: - 199cb0cafe
WalkthroughThe changes introduce a subscription-centric refactoring to the retention offer flow in the account plan page component. The RetentionOfferSection component signature is updated to accept a subscription prop, and price/label computations are derived from this subscription instead of member data. PlansContainer is modified to retrieve the member's subscription and pass it to RetentionOfferSection. Package version is bumped to 2.65.1. Test coverage is expanded to include archived tier scenarios during the cancellation flow. 🚥 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 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Free Tier Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/portal/test/unit/components/pages/account-plan-page.test.js (1)
180-180: Prefer avoiding deep post-creation fixture mutation.Line 180 reaches into fixture internals (
subscription.price.product.product_id), which is brittle if fixture shape changes. If feasible, set this viagetSubscriptionDatainputs instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/portal/test/unit/components/pages/account-plan-page.test.js` at line 180, The test mutates fixture internals via subscription.price.product.product_id which is brittle; instead update the fixture creator getSubscriptionData to accept a product_id (or tier id) argument and set product.price.product.product_id at creation time, then update this test to call getSubscriptionData(...) with the desired product_id rather than assigning subscription.price.product.product_id after the fact; reference the subscription variable and the getSubscriptionData factory to locate changes.
🤖 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/account-plan-page.js`:
- Around line 459-466: The retention UI is using getMemberSubscription({member})
which may return a different subscription than the one being cancelled; pass the
targetSubscriptionId from AccountPlanPage.render into PlansContainer (add
targetSubscriptionId to the deconstructed this.state props passed into
PlansContainer) and in PlansContainer (where RetentionOfferSection is rendered)
select the subscription by matching targetSubscriptionId (e.g., find
member.subscriptions by id or call getMemberSubscription with the target id) and
pass that specific subscription into RetentionOfferSection instead of the
default getMemberSubscription({member}); ensure references to pendingOffer and
confirmationType remain unchanged.
---
Nitpick comments:
In `@apps/portal/test/unit/components/pages/account-plan-page.test.js`:
- Line 180: The test mutates fixture internals via
subscription.price.product.product_id which is brittle; instead update the
fixture creator getSubscriptionData to accept a product_id (or tier id) argument
and set product.price.product.product_id at creation time, then update this test
to call getSubscriptionData(...) with the desired product_id rather than
assigning subscription.price.product.product_id after the fact; reference the
subscription variable and the getSubscriptionData factory to locate changes.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/portal/package.jsonapps/portal/src/components/pages/account-plan-page.jsapps/portal/test/unit/components/pages/account-plan-page.test.js
| if (confirmationType === 'offerRetention' && pendingOffer) { | ||
| const offerProduct = pendingOffer.tier | ||
| ? getProductFromId({site, productId: pendingOffer.tier.id}) | ||
| : getMemberActiveProduct({member, site}); | ||
| const offerPrice = pendingOffer.cadence === 'month' ? offerProduct?.monthlyPrice : offerProduct?.yearlyPrice; | ||
| const subscription = getMemberSubscription({member}); | ||
|
|
||
| // Skip retention offer if product or price is invalid | ||
| if (offerProduct && offerPrice) { | ||
| if (subscription) { | ||
| return ( | ||
| <RetentionOfferSection | ||
| subscription={subscription} | ||
| offer={pendingOffer} |
There was a problem hiding this comment.
Use the targeted subscription in retention rendering (not the default member subscription).
At Line 460, getMemberSubscription({member}) can select a different subscription than the cancellation target. That can show one plan/price in the UI while Line 687 applies the offer to targetSubscriptionId, creating a mismatch.
💡 Proposed fix
const PlansContainer = ({
plans, selectedPlan, confirmationPlan, confirmationType, showConfirmation = false,
- pendingOffer, onPlanSelect, onPlanCheckout, onConfirm, onCancelSubscription,
+ pendingOffer, targetSubscriptionId, onPlanSelect, onPlanCheckout, onConfirm, onCancelSubscription,
onAcceptRetentionOffer, onDeclineRetentionOffer
}) => {
const {member} = useContext(AppContext);
@@
// Retention offer flow - shown before cancellation confirmation
if (confirmationType === 'offerRetention' && pendingOffer) {
- const subscription = getMemberSubscription({member});
+ const subscription = targetSubscriptionId
+ ? getSubscriptionFromId({subscriptionId: targetSubscriptionId, member})
+ : getMemberSubscription({member});
if (subscription) {
return (
<RetentionOfferSection
subscription={subscription}
offer={pendingOffer}
onAcceptOffer={onAcceptRetentionOffer}
onDeclineOffer={onDeclineRetentionOffer}
/>
);
}
}Also pass targetSubscriptionId into PlansContainer from AccountPlanPage.render():
const {selectedPlan, showConfirmation, confirmationPlan, confirmationType, pendingOffer, targetSubscriptionId} = this.state;
// ...
<PlansContainer
{...{plans, selectedPlan, showConfirmation, confirmationPlan, confirmationType, pendingOffer, targetSubscriptionId}}
// ...
/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/portal/src/components/pages/account-plan-page.js` around lines 459 -
466, The retention UI is using getMemberSubscription({member}) which may return
a different subscription than the one being cancelled; pass the
targetSubscriptionId from AccountPlanPage.render into PlansContainer (add
targetSubscriptionId to the deconstructed this.state props passed into
PlansContainer) and in PlansContainer (where RetentionOfferSection is rendered)
select the subscription by matching targetSubscriptionId (e.g., find
member.subscriptions by id or call getMemberSubscription with the target id) and
pass that specific subscription into RetentionOfferSection instead of the
default getMemberSubscription({member}); ensure references to pendingOffer and
confirmationType remain unchanged.
There was a problem hiding this comment.
While the API design is 1 member <> multiple subscriptions, there shouldn't be more than 1 active subscription for a given member in practice
There was a problem hiding this comment.
@sagzy, thanks for the clarification! That makes sense — since a member can only ever have one active subscription in practice, getMemberSubscription({member}) will always resolve to the same subscription as targetSubscriptionId, so there's no real risk of a mismatch here. The concern is addressed.
✏️ Learnings added
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 26672
File: apps/portal/src/components/pages/account-plan-page.js:459-466
Timestamp: 2026-03-04T10:16:23.797Z
Learning: In the Ghost Portal codebase (`apps/portal/src/components/pages/account-plan-page.js`), while the Ghost API design technically supports a 1-member-to-many-subscriptions relationship, in practice a member will never have more than one active subscription at a time. Therefore, `getMemberSubscription({member})` can safely be assumed to return the member's only active subscription without risk of selecting the wrong one.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
closes https://linear.app/ghost/issue/BER-3406
/tiersendpoint