Conversation
Contributor
📝 WalkthroughWalkthroughAdds onboarding guards and redirect effects, replaces template array lookups with a templateService, consolidates wallet connection UI into WalletConnectionButtons, introduces logout analytics and storage key changes, refactors wallet auto-switching, and applies guarded exports to several protected pages. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as User/Browser
participant App as _app.tsx<br/>OnboardingRedirectEffect
participant Hook as useIsOnboarded
participant Guard as Guard HOC
participant Router as Next.js Router
participant Page as Protected Page
Browser->>App: load app / navigate
App->>Hook: evaluate onboarding state (user + wallet)
Hook-->>App: { isLoading, canVisit }
alt not onboarded
App->>Router: replace → onboarding (UrlService.onboarding)
Router->>Browser: onboarding page
else onboarded
App->>Page: allow render
Page->>Browser: show content
end
Browser->>Page: direct navigation to protected route
Page->>Guard: Guard checks useIsOnboarded
Guard->>Hook: get canVisit
alt not onboarded
Guard->>Router: redirect → onboarding
else
Guard->>Page: render
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Comment |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (17.24%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2639 +/- ##
==========================================
- Coverage 51.41% 50.56% -0.85%
==========================================
Files 1050 1019 -31
Lines 29553 28739 -814
Branches 6656 6552 -104
==========================================
- Hits 15194 14532 -662
+ Misses 14147 13997 -150
+ Partials 212 210 -2
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/deploy-web/src/components/auth/AuthPage/AuthPage.tsx (1)
93-114:⚠️ Potential issue | 🟡 MinorTighten
fromSignupdetection to avoid bypass.
searchParams.has("fromSignup")returns true even for?fromSignup=false. If onboarding should only be bypassed when the value is explicitly"true", useget(...) === "true"to match the rest of the flow.🔧 Suggested fix
- const isFromSignup = searchParams.has("fromSignup"); + const isFromSignup = searchParams.get("fromSignup") === "true";apps/deploy-web/src/pages/login/index.tsx (1)
21-84:⚠️ Potential issue | 🟡 MinorParse
fromSignupexplicitly instead of presence checks.
searchParams.has("fromSignup")andresolvedUrl.includes("fromSignup")treat any value as true (and the string check can false‑positive). Prefer reading the query value and checking for"true"in both client and server.🔧 Suggested fix
- fromSignup: searchParams.has("fromSignup"), + fromSignup: searchParams.get("fromSignup") === "true",- fromSignup: ctx.resolvedUrl.includes("fromSignup"), + fromSignup: ctx.query.fromSignup === "true",- query: z.object({ + query: z.object({ tab: z.enum(["login", "signup", "forgot-password"]).default("login"), returnTo: z.union([z.string(), z.array(z.string())]).optional(), - from: z.union([z.string(), z.array(z.string())]).optional() + from: z.union([z.string(), z.array(z.string())]).optional(), + fromSignup: z.union([z.literal("true"), z.literal("false")]).optional() })
🤖 Fix all issues with AI agents
In
`@apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx`:
- Around line 176-179: Replace the fragile string-concatenation redirect in
OnboardingContainer.tsx that does router.push(urlService.newSignup({ fromSignup:
"true" }) + "&fromSignup=true") with building the destination URL via URL or
URLSearchParams so the fromSignup query is set idempotently; call
urlService.newSignup() to get the base, create a URL object (or URLSearchParams)
from it, set or append the fromSignup=true parameter (overwriting if present),
and pass the resulting url.toString() into router.push to avoid duplicate params
or separator issues.
🧹 Nitpick comments (2)
apps/deploy-web/src/components/onboarding/OnboardingGuard/OnboardingGuard.tsx (2)
33-37: Children rendered during loading states may cause content flash.While
isUserLoadingorisWalletLoadingis true, the component renderschildreninstead of a loading indicator. This can cause a brief flash of protected content before the redirect occurs.♻️ Proposed fix to show loading during initial data fetch
if (isRedirecting) { return <Loading text="Redirecting to onboarding..." />; } + if (!isExcluded && (isUserLoading || isWalletLoading)) { + return <Loading />; + } + return <>{children}</>;
27-30: Consider handling potential redirect failures.
router.replacereturns a Promise that could reject. If navigation fails (e.g., due to route guards or network issues), the component remains stuck showing the loading state indefinitely sinceisRedirectingis never reset.♻️ Proposed fix with error handling
if (user?.userId && !hasManagedWallet && !isWalletConnected) { setIsRedirecting(true); - router.replace(UrlService.onboarding({ returnTo: router.asPath })); + router.replace(UrlService.onboarding({ returnTo: router.asPath })).catch(() => { + setIsRedirecting(false); + }); }
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx
Show resolved
Hide resolved
stalniy
reviewed
Feb 1, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/deploy-web/src/components/shared/PaymentMethodForm/PaymentMethodForm.tsx`:
- Around line 8-10: The onReady prop in PaymentMethodFormProps is too narrow;
update its type to match Stripe's PaymentElement handler so consumers receive
the StripePaymentElement param (e.g., derive it from PaymentElement's props or
use the same signature as Stripe's onReady handler). Locate
PaymentMethodFormProps and replace onReady?: () => void with a type derived from
PaymentElement (or React.ComponentProps<typeof PaymentElement>['onReady']) so
callers can access the StripePaymentElement argument.
🧹 Nitpick comments (1)
apps/deploy-web/src/components/onboarding/OnboardingPage.tsx (1)
16-19: Consider extracting the delay constant.The
1500timeout value is a magic number. For improved readability and maintainability, consider extracting it to a named constant.🔧 Suggested refactor
+const LOGOUT_BUTTON_DELAY_MS = 1500; + export const OnboardingPage: FC = () => { const { analyticsService, authService } = useServices(); const [isLoginVisible, setIsLoginVisible] = useState(false); useEffect(() => { - const timer = setTimeout(() => setIsLoginVisible(true), 1500); + const timer = setTimeout(() => setIsLoginVisible(true), LOGOUT_BUTTON_DELAY_MS); return () => clearTimeout(timer); }, []);
apps/deploy-web/src/components/shared/PaymentMethodForm/PaymentMethodForm.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/components/home/NoDeploymentsState.tsx`:
- Around line 35-36: Update the copy in the NoDeploymentsState component:
replace the hyphenated verb "sign-in" with the two-word verb "sign in" in the
JSX that renders when isSignedInWithTrial && !user (the paragraph currently
using "If you are expecting to see some, you may need to sign-in or connect a
wallet"); keep the rest of the sentence unchanged and preserve the className and
conditional rendering using the existing isSignedInWithTrial and user symbols.
In `@apps/deploy-web/src/components/onboarding/OnboardingPage.tsx`:
- Around line 15-20: The analytics event in handleLogout may be dropped because
authService.logout() runs immediately; make handleLogout async and await
analyticsService.track("onboarding_logout", { category: "onboarding" }) (or call
a provided analyticsService.flush() / promise-returning method) before calling
authService.logout(); also handle/retry or catch errors from the track promise
so logout still proceeds or logs failures appropriately while ensuring the event
has had a chance to send.
🧹 Nitpick comments (1)
apps/deploy-web/src/components/onboarding/OnboardingPage.tsx (1)
30-36: Add explicittype="button"for safety.This prevents accidental form submissions if the component is ever rendered inside a
<form>.🔧 Suggested tweak
- <button + <button + type="button" className={cn(buttonVariants({ variant: "ghost", size: "sm" }), "inline-flex items-center gap-1.5 text-xs text-muted-foreground")} onClick={handleLogout} >
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/components/home/NoDeploymentsState.tsx (1)
22-36:⚠️ Potential issue | 🟡 MinorAvoid showing the sign‑in prompt while the user is still loading.
If
useris still loading, this message can flash for already-signed-in users. Consider gating withisLoadingto prevent a brief misleading prompt.Suggested change
- const { user } = useCustomUser(); + const { user, isLoading } = useCustomUser(); ... - {isSignedInWithTrial && !user && ( + {isSignedInWithTrial && !isLoading && !user && ( <p className="mb-4 text-center text-sm text-muted-foreground">If you are expecting to see some, you may need to sign in or connect a wallet</p> )}
🧹 Nitpick comments (1)
apps/deploy-web/src/components/onboarding/steps/WelcomeStep/TrialStatusBar.tsx (1)
45-46: Handle pluralization for "days remaining".The string renders as "1 days remaining" which is grammatically incorrect. Since you're already using
react-intl, consider usingFormattedMessagewith ICU plural syntax to handle both singular and plural forms:<FormattedMessage defaultMessage="{count, plural, one {# day remaining} other {# days remaining}}" values={{ count: daysRemaining }} />
stalniy
previously approved these changes
Feb 3, 2026
Contributor
|
I think that we also need to remove |
…anagedWallet hook
…eck wallet type conditionally
…entElement interface
baktun14
force-pushed
the
fix/force-user-onboarding
branch
from
76a1c2b to
4b62fcb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/deploy-web/src/pages/login/index.tsx (1)
56-62:⚠️ Potential issue | 🟠 MajorAvoid false positives by parsing the query instead of string includes.
resolvedUrl.includes("fromSignup")can match unrelated substrings (e.g.,returnTo=/path?fromSignup=1) and incorrectly allow bypass of the onboarding guard. Prefer checking the parsed query param.🔧 Suggested fix (handler logic)
- const destination = getAuthRedirectDestination({ - currentLocation: ctx.resolvedUrl, - tab: ctx.query.tab, - fromSignup: ctx.resolvedUrl.includes("fromSignup"), + const destination = getAuthRedirectDestination({ + currentLocation: ctx.resolvedUrl, + tab: ctx.query.tab, + fromSignup: ctx.query.fromSignup !== undefined, services: { urlService: ctx.services.urlService, urlReturnToStack: ctx.services.urlReturnToStack } });🔧 Suggested fix (schema)
schema: z.object({ query: z.object({ tab: z.enum(["login", "signup", "forgot-password"]).default("login"), returnTo: z.union([z.string(), z.array(z.string())]).optional(), - from: z.union([z.string(), z.array(z.string())]).optional() + from: z.union([z.string(), z.array(z.string())]).optional(), + fromSignup: z.union([z.string(), z.array(z.string())]).optional() }) }),
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/components/home/NoDeploymentsState.tsx`:
- Around line 22-23: The persisted flag walletStore.isSignedInWithTrial (used
via isSignedInWithTrial) can remain true after logout causing stale hint; either
clear that atom on sign-out (update the logout/signOut handler to set
walletStore.isSignedInWithTrial to false) or stop relying on the persisted flag
in NoDeploymentsState and derive the hint from current session
(useCustomUser().user or session status) so the message only shows when a live
user matches the trial condition; update references in NoDeploymentsState and
the logout flow accordingly.
In `@apps/deploy-web/src/hooks/useManagedWallet.ts`:
- Around line 21-30: The one-time auto-switch guard is being flipped too early
in the useEffect (hasAutoSwitched.current) before confirming a switch; move the
assignment so it only becomes true when you actually change the type. In the
useEffect that depends on queried, selectedWalletType, setSelectedWalletType,
check if !hasAutoSwitched.current && queried then if selectedWalletType ===
"custodial" call setSelectedWalletType("managed") and immediately after set
hasAutoSwitched.current = true; leave hasAutoSwitched alone if no switch occurs
so the logic still triggers later when selectedWalletType hydrates.
…nally set wallet type
This reverts commit f831f1d.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes