Uh oh!
There was an error while loading. Please reload this page.
fix(ui): Factor in isDefault into isSwitchable - #8077
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 93454b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@clerk/agent-toolkit@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/dev-cli@clerk/expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
📝 WalkthroughWalkthroughThis pull request adds a changeset entry for 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/SubscriptionDetails/index.tsx`:
- Around line 378-380: Add unit tests for the new isSwitchable logic in
SubscriptionDetails (locate usage/definition around the isSwitchable/plan switch
guard in SubscriptionDetails/index.tsx) to cover three cases: a subscription
with plan.isDefault === true (should not be switchable), a subscription with
planPeriod === 'annual' but plan.fee falsy (should not be switchable), and a
valid non-default, non-past_due, switchable subscription (should be switchable);
place tests alongside existing SubscriptionDetails tests, asserting the
component or exported isSwitchable helper returns the expected boolean for each
scenario and include these new tests before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7037e44f-54f1-47e5-8093-da7b3623b21a
📒 Files selected for processing (2)
.changeset/witty-knives-ask.mdpackages/ui/src/components/SubscriptionDetails/index.tsx
| (subscription.planPeriod === 'annual' && Boolean(subscription.plan.fee))) && | ||
| subscription.status !== 'past_due' && | ||
| !subscription.plan.isDefault; |
There was a problem hiding this comment.
Add regression tests for the new isSwitchable guards before merge.
Line 378-Line 380 changes switch eligibility (annual fee and !isDefault) but no tests were added/updated in this PR. Please add coverage for: default plan (not switchable), annual without fee (not switchable), and valid non-default switchable cases.
As per coding guidelines, "If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/SubscriptionDetails/index.tsx` around lines 378 -
380, Add unit tests for the new isSwitchable logic in SubscriptionDetails
(locate usage/definition around the isSwitchable/plan switch guard in
SubscriptionDetails/index.tsx) to cover three cases: a subscription with
plan.isDefault === true (should not be switchable), a subscription with
planPeriod === 'annual' but plan.fee falsy (should not be switchable), and a
valid non-default, non-past_due, switchable subscription (should be switchable);
place tests alongside existing SubscriptionDetails tests, asserting the
component or exported isSwitchable helper returns the expected boolean for each
scenario and include these new tests before merging.
| ((subscription.planPeriod === 'month' && Boolean(subscription.plan.annualMonthlyFee)) || | ||
| subscription.planPeriod === 'annual') && | ||
| subscription.status !== 'past_due'; | ||
| (subscription.planPeriod === 'annual' && Boolean(subscription.plan.fee))) && |
There was a problem hiding this comment.
We're storing the amount for an annual plan inside of the fee?
There was a problem hiding this comment.
This is for switching between periods. That line reads "if the current subscription is an annual subscription and the plan has a monthly fee, allow switching from annual to monthly".
Uh oh!
There was an error while loading. Please reload this page.
Description
This PR fixes an issue where the
Switch to annualbutton was being rendered for default plans.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit