-
Notifications
You must be signed in to change notification settings - Fork 36
feat: Remove option to change to yearly/annual plan #3958
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| }) | ||
|
|
||
| const isMonthlyPlan = plan?.billingRate === BillingRate.MONTHLY | ||
|
|
||
| const isPaidPlan = !!plan?.billingRate // If the plan has a billing rate, it's a paid plan | ||
|
|
||
| let newPlan = proPlanYear | ||
| if (isSentryUpgrade && !plan?.isSentryPlan) { | ||
| newPlan = isMonthlyPlan ? sentryPlanMonth : sentryPlanYear | ||
| // Ensure we default to monthly plan regardless of current billing cycle | ||
| let newPlan = proPlanMonth | ||
| if ((isSentryUpgrade && !plan?.isSentryPlan) || selectedPlan?.isSentryPlan) { | ||
| newPlan = sentryPlanMonth | ||
| } else if (plan?.isTeamPlan || selectedPlan?.isTeamPlan) { | ||
| newPlan = isMonthlyPlan ? teamPlanMonth : teamPlanYear | ||
| } else if (isPaidPlan) { | ||
| newPlan = plan | ||
| newPlan = teamPlanMonth | ||
| } | ||
|
|
||
| const seats = extractSeats({ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3958 +/- ##
==========================================
+ Coverage 98.61% 98.73% +0.11%
==========================================
Files 828 826 -2
Lines 15126 14970 -156
Branches 4348 4285 -63
==========================================
- Hits 14917 14780 -137
+ Misses 201 182 -19
Partials 8 8
... and 5 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #3958 +/- ##
==========================================
+ Coverage 98.61% 98.73% +0.11%
==========================================
Files 828 826 -2
Lines 15126 14970 -156
Branches 4348 4285 -63
==========================================
- Hits 14917 14780 -137
+ Misses 201 182 -19
Partials 8 8
... and 5 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Bundle ReportChanges will decrease total bundle size by 11.99kB (-0.1%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: gazebo-production-esmAssets Changed:
Files in
Files in
Files in
view changes for bundle: gazebo-production-systemAssets Changed:
Files in
Files in
Files in
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
src/pages/PlanPage/subRoutes/UpgradePlanPage/UpgradeForm/UpdateBlurb/UpdateBlurb.test.tsx
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #3958 +/- ##
==========================================
+ Coverage 98.61% 98.73% +0.11%
==========================================
Files 828 826 -2
Lines 15126 14970 -156
Branches 4348 4280 -68
==========================================
- Hits 14917 14780 -137
+ Misses 201 182 -19
Partials 8 8
... and 5 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #3958 +/- ##
==========================================
+ Coverage 96.54% 98.73% +2.18%
==========================================
Files 828 826 -2
Lines 15126 14970 -156
Branches 4348 4285 -63
==========================================
+ Hits 14604 14780 +176
+ Misses 467 182 -285
+ Partials 55 8 -47
... and 47 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incomplete refactoring leaves yearly plan references inconsistent
The PR changed the pricing display from teamPlanYear?.baseUnitPrice to teamPlanMonth?.baseUnitPrice and updated the billing text to say "billed monthly", but left teamPlanYear?.marketingName (line 22) and teamPlanYear?.benefits (line 42) unchanged. This creates an inconsistent state where the component shows monthly pricing but displays the yearly plan's name and benefits. If yearly plans become unavailable from the API, teamPlanYear would be undefined, causing the card to render without a title and with an empty benefits list.
src/pages/PlanPage/subRoutes/CancelPlanPage/subRoutes/TeamPlanSpecialOffer/TeamPlanCard/TeamPlanCard.tsx#L21-L22
Lines 21 to 22 in 45dfba9
| <div className="flex justify-between p-4"> | |
| <h2 className="font-semibold">{teamPlanYear?.marketingName} plan</h2> |
src/pages/PlanPage/subRoutes/CancelPlanPage/subRoutes/TeamPlanSpecialOffer/TeamPlanCard/TeamPlanCard.tsx#L41-L42
Lines 41 to 42 in 45dfba9
| <BenefitList | |
| benefits={teamPlanYear?.benefits} |
| }) | ||
| const monthlyProPlan = isSentryUpgrade ? sentryPlanMonth : proPlanMonth | ||
|
|
||
| let planOption = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was an existing bug where on default, the UI selected option didn't match up with what the form had as its value. planParams here on first load doesn't have anything set on default so it automatically has the UI show Pro is selected.
However, in the UpgradePlanPage component, we have this code
if (
(hasTeamPlans && planParam === TierNames.TEAM) ||
planData?.plan?.isTeamPlan
) {
defaultPaidYearlyPlan = teamPlanYear
(changed to use monthly instead of yearly in this PR) which sets the form default to Team if the current plan that the customer is on is a Team plan.
Consequently, the "Update" button at the bottom of the form will be correctly disabled even though it looks like it should be enabled. This line in UpdateButton.tsx const isSamePlan = newPlan?.value === currentPlanValue resolves to true. (newPlan comes from form values which start out with what getDefaultUpgradeForm resolves to.
With this change, the radio buttons will just look to selectedPlan just as getDefaultUpgradeForm does in UpgradeForm.tsx.
| /month | ||
| {nextBillingDate && ( | ||
| <Fragment> | ||
| ,<span className="font-semibold"> next billing date</span> is{' '} | ||
| {nextBillingDate} | ||
| </Fragment> | ||
| )} | ||
| </p> | ||
| <div className="flex flex-row gap-1"> | ||
| <Icon size="sm" name="lightBulb" variant="solid" /> | ||
| <p> | ||
| You{' '} | ||
| <span className="font-semibold"> | ||
| save {formatNumberToUSD(nonBundledCost)} | ||
| </span>{' '} | ||
| with the Sentry bundle | ||
| {seats > 5 && ( | ||
| <> | ||
| , save an{' '} | ||
| <span className="font-semibold"> | ||
| additional{' '} | ||
| {formatNumberToUSD( | ||
| (perMonthPrice - perYearPrice) * MONTHS_PER_YEAR | ||
| )} | ||
| </span>{' '} | ||
| a year with annual billing | ||
| {nextBillingDate && ( | ||
| <Fragment> | ||
| ,<span className="font-semibold"> next billing date</span> is{' '} | ||
| {nextBillingDate} | ||
| </Fragment> | ||
| )}{' '} | ||
| <button | ||
| className="cursor-pointer font-semibold text-ds-blue-darker hover:underline" | ||
| onClick={() => setFormValue('newPlan', sentryPlanYear)} | ||
| > | ||
| switch to annual | ||
| </button> | ||
| </> | ||
| )} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // figures out the default plan to use based on the selected plan, current plan, and isSentryUpgrade | ||
| export const determineDefaultPlan = ({ | ||
| selectedPlan, | ||
| currentPlan, | ||
| plans, | ||
| isSentryUpgrade, | ||
| }: { | ||
| selectedPlan?: IndividualPlan | null | ||
| currentPlan?: Plan | null | ||
| plans?: IndividualPlan[] | null | ||
| isSentryUpgrade: boolean | ||
| }): IndividualPlan | undefined => { | ||
| const { proPlanMonth } = findProPlans({ plans }) | ||
| const { sentryPlanMonth } = findSentryPlans({ plans }) | ||
| const { teamPlanMonth } = findTeamPlans({ plans }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to consolidate logic here for choosing default plan. Previously, we had some code here in the getDefaultValuesUpgradeForm() function and a slightly different logic in <UpgradePlanPage /> and its child <PlanTypeOptions /> which just showed Pro in the UI unless specified otherwise with the url param.
| register={register} | ||
| errors={errors} | ||
| /> | ||
| <SentryPlanController seats={seats} register={register} errors={errors} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we got rid of the setformvalue and register in many components here. How does the logic work now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The <BillingOption /> components don't need setFormValue anymore since the only option available now is monthly. The <...Controller /> parent components also don't need it anymore to pass down. The other input components will still function as normal though.
adrianviquez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went over this logic offline together and tested scenarios. Generally looked good, though Calvin wanted to revisit some specific behaviors. Will follow up after those are looked at
…pgrade weight for default
| (currentPlan?.billingRate === BillingRate.MONTHLY | ||
| ? currentPlan | ||
| : undefined) ?? | ||
| undefined | ||
|
|
||
| return plan | ||
| } | ||
|
|
||
| export const getDefaultValuesUpgradeForm = ({ | ||
| accountDetails, | ||
| plans, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The shouldDisplayTeamCard function will incorrectly hide the Team plan if the backend stops providing a yearly plan option, as it requires both monthly and yearly plans to be present.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The function shouldDisplayTeamCard requires both monthly and yearly team plans to be available to return true. However, this pull request is removing the option for users to start new yearly plans. If the backend API is updated to no longer provide a yearly team plan option for new customers, shouldDisplayTeamCard will incorrectly return false. This will cause the 'Team' plan card to be hidden on the upgrade page, preventing users from selecting it, even when a monthly team plan is available.
💡 Suggested Fix
Update the shouldDisplayTeamCard function to only check for the existence of a monthly team plan. Change the return condition from !isUndefined(teamPlanMonth) && !isUndefined(teamPlanYear) to !isUndefined(teamPlanMonth).
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/shared/utils/upgradeForm.ts#L199-L249
Potential issue: The function `shouldDisplayTeamCard` requires both monthly and yearly
team plans to be available to return `true`. However, this pull request is removing the
option for users to start new yearly plans. If the backend API is updated to no longer
provide a yearly team plan option for new customers, `shouldDisplayTeamCard` will
incorrectly return `false`. This will cause the 'Team' plan card to be hidden on the
upgrade page, preventing users from selecting it, even when a monthly team plan is
available.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7621343
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will address this in a separate PR as this isn't a problem at the moment.
Removing option to start a new yearly plan
Closes https://linear.app/getsentry/project/block-annual-plan-subscriptions-4e0b8ca9a437/overview
Note
Remove annual plan selection across plan pages, default everything to monthly pricing, update links/utilities accordingly, and align tests.
BillingOptionsfor Pro/Sentry/Team; show only monthly radio and pricing inProPlanController,SentryPlanController,TeamPlanController.PriceCalloutcomponents to compute/show monthly totals only and next billing date.UpdateBlurb: stop treating monthly→annual as upgrade; only note Annual→Monthly changes.ProPlanDetails,TeamPlanDetails,SentryPlanDetails: display monthly unit prices and “billed monthly”; include monthly plan benefits.TeamPlanCardand current planPlanUpgradeTeam: show monthly price/aux text and upgrade links.?plan=pro|team);ActionsBillingaccepts optionalbuttonOptionsto pass params through.determineDefaultPlanto choose default monthly plan (respects selected/current plan and Sentry eligibility); used inUpgradePlanPageandgetDefaultValuesUpgradeForm.BillingRate.ANNUALLYas deprecated inbilling.ts.$12,$6, Sentry “billed monthly”) and removal of annual radio/options; add coverage for link query params and new default plan logic.Written by Cursor Bugbot for commit e8a0632. This will update automatically on new commits. Configure here.