feat(billing): unified BillingCardComponent + annual toggle disabled state#4149
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR migrates the billing module's pricing UI and data to V4 design by removing the savings-on-toggle pattern (moving savings display to the card), simplifying the pricing toggle component, updating the static plan schema, and refactoring the pricing view to support both legacy and new plan data shapes while preserving Stripe price overrides. ChangesBilling Pricing V4 Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
… card edge cases Phase 0 critical-review BLOCK findings — iter 1: - [critical] Migrate devkit static-content plans to V4 schema (title/subtitle/highlight) to match the new BillingCardComponent contract. Drop legacy name/tagline/highlighted + featureSections + included:false features + equivalences. Cards now render with proper titles and elevated variant for the highlighted plan on the devkit dev server. - [high] BillingCardComponent: skip cta-click emit when cta.to is set. v-btn's :to binds router-link which navigates natively; emitting also triggered a duplicate $router.push in the parent's @cta-click handler with a divergent URL (free+guest plan dropped the redirect query param). - [medium] Drop dead _equivalences computation in resolvedPlanItems — the new BillingCardComponent does not consume equivalences. Trawl downstream already removed them too. - [low] Drop dead maxAnnualSavingsPct prop on BillingPricingToggleComponent — caption was removed in V4 (savings live on card price.chip). Also drop the prop binding in billing.pricing.view.vue. - Test updates: add cta.to → no-emit test; drop maxAnnualSavingsPct usage in toggle tests (prop removed); simplify i18n mock (savingsActive key unused). All tests green (1669/1669, 101 files). Lint clean.
Phase 0 critical-review BLOCK findings — iter 2:
- [high] Free+guest CTA `cta.to` was '/signup' (string), losing the redirect=/pricing
query param. Card skips emit when cta.to is set (intentional double-nav fix), so
the view's onCtaClick fallback that pushed /signup?redirect=/pricing was dead.
Fix: pass cta.to as { path: '/signup', query: { redirect: '/pricing' } } — v-btn :to
accepts the same shape as $router.push().
- [medium] Drop dead `meterMode` computed from the view (only consumed by the
removed _equivalences logic).
- [low] Update billing.pricing.view.unit.tests.js mock fixtures to V4 schema
(title/subtitle/highlight) — masked schema-compliance gaps.
- [low] Clarify BillingCardComponent ITEM SCHEMA doc: cta is a string in
static-content plans but expanded into an object by resolvedPlanItems; cta-click
emit guard now documented (disabled OR to=set).
Note: usePricing.maxAnnualSavingsPct stays — it's a public composable API still
tested by billing.usePricing.unit.tests.js. Downstream may consume it.
All tests green (1669/1669). Lint clean.
Phase 0 nit — BillingPricingToggleComponent has no density prop; the binding falls through as inert HTML attribute on the root div with no effect. Remove the binding (Vuetify density is not surfaced by this wrapper).
65a5e1d to
b20588a
Compare
… tests)
Replace this.$t('billing.pricingCard.saveAnnual') with Save ${pct}% template literal
to match master convention and avoid $t not a function failures in unit tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4149 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 31 31
Lines 1136 1136
Branches 328 328
=======================================
Hits 1131 1131
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
This PR updates the billing pricing UI to use a unified, item-driven BillingCardComponent for plans/packs and adds a disabled state to the annual/monthly pricing toggle, while migrating the devkit billing static-content plans to the V4 schema (title/subtitle/highlight) and removing legacy toggle savings caption behavior.
Changes:
- Replace the pricing view’s card rendering with
BillingCardComponentusing a fully-resolved item schema (including annual savings asprice.chip). - Add
disabledprop support toBillingPricingToggleComponentand remove legacy “savings caption” rendering/tests. - Migrate devkit
billing.static-content.jsplans to the V4 schema and update unit tests to match.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/modules/billing/views/billing.pricing.view.vue | Builds resolved card items (price display + annual savings chip) and wires toggle disabled state in both-tabs mode. |
| src/modules/billing/components/billing.pricingToggle.component.vue | Removes savings caption API; adds disabled prop passthrough + UI dimming. |
| src/modules/billing/components/billing.card.component.vue | Prevents cta-click emit when cta.to is set to avoid double navigation. |
| src/modules/billing/config/billing.static-content.js | Migrates devkit plan copy to V4 (title/subtitle/highlight/info/features). |
| src/modules/billing/tests/billing.pricingToggle.component.unit.tests.js | Updates assertions to reflect caption-free toggle + new disabled behavior. |
| src/modules/billing/tests/billing.pricing.view.unit.tests.js | Updates mocked plan shape to V4 fields used by the view/card contract. |
| src/modules/billing/tests/billing.card.component.unit.tests.js | Adds coverage for “no emit when cta.to is set” behavior. |
| // Preserve the `redirect` query param so the user lands back on the pricing | ||
| // page after signup. v-btn's :to accepts the same shape as $router.push(). | ||
| // The card skips its cta-click emit when cta.to is set (router-link owns | ||
| // the navigation), so the view's onCtaClick fallback is intentionally bypassed. | ||
| ctaTo = { path: '/signup', query: { redirect: '/pricing' } }; |
Addresses Copilot review on #4149. The pricing CTA already set ?redirect=/pricing on /signup (Phase 0 fix cc6c585), but signup.view silently dropped it — only signin.view:157 honored redirect. The redirect param was therefore dead code from the signup CTA's POV. Adds a pushAfterAuth() helper mirroring signin's pattern: - typeof redirect === 'string' && redirect.startsWith('/') → push redirect - otherwise → push config.sign.route - typeof guard prevents arrays (?redirect=/a&redirect=/b); startsWith('/') guard prevents open-redirect to external URLs. 5 call sites swapped to pushAfterAuth(): - watch.auth (orgs disabled branch) - created() (already logged in + has org) - validate() success, orgs enabled, no setup needed - validate() success, orgs disabled - proceedToApp() (end of org flow) 4 new unit tests (redirect honored / fallback / open-redirect guarded / proceedToApp redirect). Total signup tests 25 → 29.
|
@copilot — good catch. Fixed in latest commit: signup.view now honors |
Verified in commit |
Summary
BillingCardComponent(flat resolved-item schema, plans + packs), adddisabledprop toBillingPricingToggleComponent, migrate devkit static-content plans to V4 schema (title/subtitle/highlight), delete legacyBillingPricingCardComponent.Scope
billing(components, view, static-content, tests)none— billing module internals only.medium— schema migration of devkitplansstatic-content (downstream projects already use V4 schema; only the devkit dev server example is impacted).Validation
npm run lintnpm run test:unit(1669/1669 passing across 101 files)npm run build(CI will verify)Guardrails check
/update-stack)Optional: Infra/Stack alignment details
Before vs After (key changes only)
BillingPricingCardComponent(424 lines, internal price/CTA resolution, skeleton loader, tooltip on unavailable, dual feature/featureSections paths, equivalences mode)BillingCardComponent(165 lines, dumb item-driven, no internal logic)resolvedPlanItemsdisabledprop absent; savings caption rendered below switch (maxAnnualSavingsPctprop)disabledprop added (wrapper opacity + v-switch passthrough); no caption (savings on card chip)maxAnnualSavingsPctprop dropped — public composable API kept for downstreamBillingPricingCardComponentwith multi-prop pass-throughBillingCardComponentwith single:item+@cta-clickresolvedPlanItemscomputed builds the item per planplansname/tagline/highlighted+ legacyfeatures: [{ text, included }]+featureSectionstitle/subtitle/highlight+ V4features: [{ icon, color, text }]+infolinecta.to = '/signup'(string)cta.to = { path: '/signup', query: { redirect: '/pricing' } }BillingCardComponent.onCtaClickcta.disabledORcta.tois setpierreb-devkit/Vue@HEAD^)Notes for reviewers
plansstatic-content). Iter 2 BLOCK (1 high: signup redirect query loss whencta.tois set as string). Iter 3OK with nits. Final commit fixed inertdensityattr.name/tagline/highlightedschema in their ownbilling.static-content.jsare NOT touched (this PR only updates the devkit defaults).usePricing.maxAnnualSavingsPctis unused by the view but retained as a public composable API. A future cleanup could deprecate it if no downstream consumes it.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements