Conversation
|
Caution Review failedPull request was closed or merged during review WalkthroughReplaces the guest/member paywall with a multi-plan model (tiers: haiku, sonnet, opus) and updates checkout flows to pass a Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
js/app/packages/service-clients/service-auth/client.ts (1)
406-421: 🛠️ Refactor suggestion | 🟠 MajorReuse the generated checkout-tier type.
CreateCheckoutSessionRequestalready narrowstiertoStripeProductTier; widening it tostringhere drops compile-time validation at the network boundary. Please type this arg from the generated request model so invalid tiers cannot compile through to/user/stripe/checkout.As per coding guidelines,
js/app/**/*.{ts,tsx}: Use type-driven design with types guiding function composition. DO NOT USEany🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/service-clients/service-auth/client.ts` around lines 406 - 421, The createCheckoutSession function's args currently type tier as string which widens and bypasses the generated CreateCheckoutSessionRequest/StripeProductTier validation; change the args shape to use the generated request type (or at least type the tier as StripeProductTier from the generated model) so the function signature aligns with CreateCheckoutSessionRequest and preserves compile-time checking before calling fetchWithAuth(`/user/stripe/checkout`); update the parameter type reference in createCheckoutSession and any destructuring/JSON body usage to match CreateCheckoutSessionRequest/StripeProductTier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/auth/Signup.tsx`:
- Around line 97-105: The clickable Google sign-up element in Signup.tsx is a
<div> using onClick (startGoogleLogin) and lacks keyboard support; replace it
with a semantic <button> (or add role="button", tabindex="0" and keyDown handler
invoking startGoogleLogin) so keyboard users can activate it, preserve existing
classes and the IconGoogle markup, and ensure focus/hover styles remain
consistent and accessible.
- Around line 119-125: The terms and privacy anchor hrefs in Signup.tsx are
using absolute paths ("/terms", "/privacy") while other auth links use the
ROUTER_BASE_CONCAT prefix; update the two anchors in the Signup component to use
`${ROUTER_BASE_CONCAT}terms` and `${ROUTER_BASE_CONCAT}privacy` (or the
equivalent string concatenation used in this file) so they resolve under the app
base path, and apply the same change in LoginOptions and Onboarding components
if those files also use bare "/terms" or "/privacy".
In
`@js/app/packages/app/component/interactive-onboarding/lessons/choose-plan.tsx`:
- Around line 53-69: The component's handleCheckout directly calls
stripeServiceClient.createCheckoutSession and manages network/loading/error
state locally; move that network call into the queries package by adding a
Tanstack query mutation (e.g., createCheckoutSessionMutation in queries) that
calls stripeServiceClient.createCheckoutSession, then import and use that
mutation in this component instead of calling the service client directly.
Replace the local pending/error handling (setLoading, loading()) with the
mutation's status (mutation.isLoading, mutation.isError) and perform side
effects in the mutation callbacks: onSuccess should
analytics.track('subscription_start', { type: tier }) and redirect via
window.location.href to the returned URL, and onError should
toast.failure('Failed to start checkout...') (and clear any local state if still
used). Ensure no direct references to stripeServiceClient remain in this file
and the new mutation is the single caller of the client.
In `@js/app/packages/app/component/paywall/PaywallComponent.tsx`:
- Around line 48-55: handleCheckout currently calls
stripeServiceClient.createCheckoutSession directly (bypassing the data layer);
extract that call into a TanStack Query mutation in the queries package (create
a new mutation—e.g., createCheckoutSessionMutation) which invokes
stripeServiceClient.createCheckoutSession with the same arguments, then replace
the direct call in PaywallComponent.handleCheckout to call the mutation's
mutateAsync (or mutate) and derive button/loading/error state from the
mutation's status; preserve the existing props.cb invocation and ensure you pass
the same parameters (props.customType or props.errorKey and tier) through the
mutation API so the component no longer calls any service-clients directly.
In `@js/app/packages/app/component/Root.tsx`:
- Around line 255-257: The route object currently uses an unnecessary inline
component factory; replace the component field that is set to "() => <Signup />"
with the direct component reference "Signup" in the route entry so the route's
component property points to the Signup component itself (locate the route
object where path: '/signup' and component is currently an arrow returning
<Signup />).
In `@js/app/packages/service-clients/service-stripe/client.ts`:
- Around line 36-49: Update the createCheckoutSession API to accept a single
options object instead of positional parameters: change the signature of
createCheckoutSession to (opts: { type?: string; discount?: string; tier?:
AuthTierType }) and map opts.type/discount/tier into the
authServiceClient.createCheckoutSession payload (preserve gaClientId via
getGaClientId()). Update all callers to pass createCheckoutSession({ tier }) or
createCheckoutSession({ type, discount, tier }) rather than
createCheckoutSession('', undefined, tier). Ensure the parameter type uses the
specific narrowed tier type from the auth client (no any) and that optional
fields default to null where the auth call expects null (e.g., discount ?? null,
gaClientId ?? null).
---
Outside diff comments:
In `@js/app/packages/service-clients/service-auth/client.ts`:
- Around line 406-421: The createCheckoutSession function's args currently type
tier as string which widens and bypasses the generated
CreateCheckoutSessionRequest/StripeProductTier validation; change the args shape
to use the generated request type (or at least type the tier as
StripeProductTier from the generated model) so the function signature aligns
with CreateCheckoutSessionRequest and preserves compile-time checking before
calling fetchWithAuth(`/user/stripe/checkout`); update the parameter type
reference in createCheckoutSession and any destructuring/JSON body usage to
match CreateCheckoutSessionRequest/StripeProductTier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 308569ce-ec0e-4cfd-ab21-8d2a5e2cc7bd
📒 Files selected for processing (9)
js/app/packages/app/component/Root.tsxjs/app/packages/app/component/auth/EmailForm.tsxjs/app/packages/app/component/auth/LoginOptions.tsxjs/app/packages/app/component/auth/Signup.tsxjs/app/packages/app/component/auth/VerifyForm.tsxjs/app/packages/app/component/interactive-onboarding/lessons/choose-plan.tsxjs/app/packages/app/component/paywall/PaywallComponent.tsxjs/app/packages/service-clients/service-auth/client.tsjs/app/packages/service-clients/service-stripe/client.ts
No description provided.