fix: billing usage page guard flicker#1826
Conversation
WalkthroughHooks and the Guard HOC were changed to return/loading-aware objects ({ canVisit, isLoading }). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as Page
participant G as Guard HOC
participant C as Composed UseCheck
participant H as Hooks (useUser / useWallet)
U->>P: Navigate to protected route
P->>G: Render wrapped component
G->>C: call useCheck()
C->>H: call useUser()/useWallet()
H-->>C: { canVisit, isLoading }
C-->>G: { canVisit, isLoading }
alt isLoading == true
G->>P: Render Loading
else canVisit == true
G->>P: Render Component
else
G->>P: Render Fallback (404)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/deploy-web/src/hooks/useUser.ts (1)
7-17: Return type is too strong; prefer Partial to match actual runtime shape
useUserpromises a fullCustomUserProfile, but can return{}when no user is present. This is type-unsafe and can mislead call sites into treating fields as always defined.Apply this diff to align the type contract with behavior:
-export const useUser = (): CustomUserProfile & { isLoading: boolean } => { +export const useUser = (): Partial<CustomUserProfile> & { isLoading: boolean } => {This keeps your current usage pattern (optional chaining) sound, and avoids leaking unsound “always-present” fields.
apps/deploy-web/src/hoc/guard/guard.hoc.tsx (1)
4-8: Export the UseCheck type for consumersExporting
UseCheckallows guard hooks to explicitly annotate their return type and get compile-time alignment withGuard/composeGuards.-type UseCheck = () => { +export type UseCheck = () => { canVisit: boolean; isLoading: boolean; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx(1 hunks)apps/deploy-web/src/hoc/guard/guard.hoc.tsx(2 hunks)apps/deploy-web/src/hooks/useUser.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/hooks/useUser.tsapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/hoc/guard/guard.hoc.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/hooks/useUser.tsapps/deploy-web/src/context/WalletProvider/WalletProvider.tsxapps/deploy-web/src/hoc/guard/guard.hoc.tsx
🧬 Code Graph Analysis (2)
apps/deploy-web/src/hooks/useUser.ts (3)
apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/src/hooks/useCustomUser.ts (1)
useCustomUser(16-26)apps/deploy-web/src/hooks/useStoredAnonymousUser.ts (1)
useStoredAnonymousUser(18-49)
apps/deploy-web/src/hoc/guard/guard.hoc.tsx (1)
apps/deploy-web/src/components/layout/Layout.tsx (1)
Loading(151-162)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (4)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (1)
360-363: LGTM: guard-ready hook shape is correct
useIsManagedWalletUsercleanly maps to the new guard contract and will prevent the 404 flicker by surfacingisLoading. A repository-wide search across.ts/.tsxfiles confirms there are no remaining boolean-style usages ofuseIsManagedWalletUserthat need adapting.apps/deploy-web/src/hooks/useUser.ts (1)
19-26: Normalize registered-user detection to consider bothidanduserIdTo prevent false negatives when one of these fields is populated and the other isn’t, update
useIsRegisteredUserto accept either:– export const useIsRegisteredUser = () => { – const { isLoading, userId } = useUser(); – – return { – isLoading, – canVisit: !!userId – }; – }; + export const useIsRegisteredUser = () => { + const { isLoading, userId, id } = useUser(); + return { + isLoading, + canVisit: Boolean(userId ?? id) + }; + };– This aligns with checks elsewhere (e.g.
user?.idinWalletProvider.tsx).
– Once the user shape is consolidated, you can remove one of these branches.apps/deploy-web/src/hoc/guard/guard.hoc.tsx (2)
9-16: LGTM: loading gate removes 404 flickerRendering
<Loading>duringisLoadingneatly fixes the transient 404 without violating hook rules.
31-44: ComposeGuards verification complete – no boolean-only guards found• Searched for all
composeGuards(…)usages and confirmed only the intended calls in
- apps/deploy-web/src/pages/usage/index.tsx
- apps/deploy-web/src/pages/billing/index.tsx
• Searched across all.ts/.tsxfiles for any guard hooks still returning a bareboolean– none foundComposition logic is sound, hooks correctly return
{ canVisit, isLoading }, and there are no remaining boolean-based guards to migrate.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/deploy-web/src/types/user.ts (1)
19-22: Consider keeping loading state out of the domain/User typeAdding isLoading onto CustomUserProfile blurs concerns (data vs. UI state) and can accidentally leak into analytics/persistence. Optional, but consider returning a wrapper result from hooks instead (e.g., { user?: CustomUserProfile; isLoading: boolean }) and keep CustomUserProfile purely user data.
Example type (outside this hunk) if you decide to refactor hooks later:
export type UseUserResult = { user?: CustomUserProfile; isLoading: boolean; };apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (1)
21-21: Follow repo test guideline: prefer queryBy over getBy in expectationsIn apps//.spec.tsx we use queryBy methods. Replace getByRole with queryByRole here.
Apply:
- expect(screen.getByRole("status")).toBeInTheDocument(); + expect(screen.queryByRole("status")).toBeInTheDocument();apps/deploy-web/src/hooks/useUser.ts (2)
7-16: useMemo here is optional and adds noiseThese computations are cheap and derived from stable hook outputs. You can simplify by inlining them without useMemo to reduce indirection and dependency maintenance.
Example:
- const user = useMemo(() => registeredUser ?? anonymousUser, [registeredUser, anonymousUser]); - const isLoading = useMemo(() => isLoadingRegisteredUser || isLoadingAnonymousUser, [isLoadingRegisteredUser, isLoadingAnonymousUser]); + const user = registeredUser ?? anonymousUser; + const isLoading = isLoadingRegisteredUser || isLoadingAnonymousUser;
8-12: Potential double invocation of useCustomUser via nested hook usageuseUser calls useCustomUser, and useStoredAnonymousUser also calls useCustomUser internally. This duplicates subscription to Auth0’s user state and can cause extra renders. Optional: thread registered user/loading into useStoredAnonymousUser, or have useStoredAnonymousUser accept a flag/param to avoid its internal useCustomUser call when already provided.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx(1 hunks)apps/deploy-web/src/hoc/guard/guard.hoc.tsx(2 hunks)apps/deploy-web/src/hooks/useUser.spec.tsx(1 hunks)apps/deploy-web/src/hooks/useUser.ts(1 hunks)apps/deploy-web/src/types/user.ts(1 hunks)apps/deploy-web/tests/seeders/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/deploy-web/src/hoc/guard/guard.hoc.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/tests/seeders/user.tsapps/deploy-web/src/types/user.tsapps/deploy-web/src/hooks/useUser.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/hooks/useUser.ts
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/tests/seeders/user.tsapps/deploy-web/src/types/user.tsapps/deploy-web/src/hooks/useUser.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsxapps/deploy-web/src/hooks/useUser.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/hooks/useUser.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/hooks/useUser.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/deploy-web/src/hooks/useUser.spec.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx
🧬 Code Graph Analysis (3)
apps/deploy-web/src/types/user.ts (1)
apps/deploy-web/src/components/user/UserProfile.tsx (1)
UserProfile(20-71)
apps/deploy-web/src/hooks/useUser.spec.tsx (1)
apps/deploy-web/tests/seeders/user.ts (1)
buildUser(7-26)
apps/deploy-web/src/hooks/useUser.ts (2)
apps/deploy-web/src/hooks/useCustomUser.ts (1)
useCustomUser(16-26)apps/deploy-web/src/hooks/useStoredAnonymousUser.ts (1)
useStoredAnonymousUser(18-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (7)
apps/deploy-web/tests/seeders/user.ts (1)
23-23: LGTM: sensible default for testsDefaulting isLoading to false in the user seeder is consistent with the updated typing and enables overriding per test.
apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx (1)
56-62: LGTM: updated tracking payloads match the new loading-aware shapeThe test now asserts the transient loading payload and the final anonymous user payload with isLoading. This aligns with the new contract and should prevent the flicker regressions from slipping.
apps/deploy-web/src/hooks/useUser.spec.tsx (4)
15-15: LGTM: test name reflects the new canVisit contractClearer intent with “can visit” wording.
20-23: LGTM: expectation updated to the new return shapeAsserting { isLoading: false, canVisit: true } matches the updated API.
26-26: LGTM: negative case coveredCovers the falsy userId path appropriately.
31-34: LGTM: expectation mirrors the object result for the negative pathMatches { isLoading: false, canVisit: false } as designed.
apps/deploy-web/src/hooks/useUser.ts (1)
20-25: No boolean-style usages of useIsRegisteredUser remain – all call sites are up to dateA repository-wide search for
useIsRegisteredUser(in *.ts/tsx only returned its use withinapps/deploy-web/src/hooks/useUser.spec.tsx(no production code treating the hook call as a bare boolean). There are no downstreamif (useIsRegisteredUser())orconst foo = useIsRegisteredUser()patterns in the app.Since no code outside tests was broken by the change, no further updates are required.
| export const useUser = (): CustomUserProfile => { | ||
| const { user: registeredUser } = useCustomUser(); | ||
| const { user: anonymousUser } = useStoredAnonymousUser(); | ||
| const user = useMemo(() => registeredUser || anonymousUser, [registeredUser, anonymousUser]); | ||
| const { user: registeredUser, isLoading: isLoadingRegisteredUser } = useCustomUser(); | ||
| const { user: anonymousUser, isLoading: isLoadingAnonymousUser } = useStoredAnonymousUser(); | ||
| const user = useMemo(() => registeredUser || anonymousUser || {}, [registeredUser, anonymousUser]); | ||
| const isLoading = useMemo(() => isLoadingRegisteredUser || isLoadingAnonymousUser, [isLoadingRegisteredUser, isLoadingAnonymousUser]); | ||
|
|
||
| return user; | ||
| return { | ||
| ...user, | ||
| isLoading | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tighten return typing and avoid {} as a user fallback
Returning {} as the user can be type-unsound against CustomUserProfile (depending on required fields of Auth0’s UserProfile) and spreads UI state into a domain object. Also, nullish coalescing communicates intent better than || here.
Minimal, local change to improve type-safety without altering callers:
-export const useUser = (): CustomUserProfile => {
+export const useUser = (): Partial<CustomUserProfile> & { isLoading: boolean } => {
const { user: registeredUser, isLoading: isLoadingRegisteredUser } = useCustomUser();
const { user: anonymousUser, isLoading: isLoadingAnonymousUser } = useStoredAnonymousUser();
- const user = useMemo(() => registeredUser || anonymousUser || {}, [registeredUser, anonymousUser]);
+ const user = useMemo(() => registeredUser ?? anonymousUser, [registeredUser, anonymousUser]);
const isLoading = useMemo(() => isLoadingRegisteredUser || isLoadingAnonymousUser, [isLoadingRegisteredUser, isLoadingAnonymousUser]);
return {
- ...user,
+ ...(user ?? {}),
isLoading
};
};Notes:
- Return type is now Partial with a guaranteed isLoading flag.
- Avoids returning {} as a “user” while keeping the existing spread shape for consumers.
- Uses ?? over || to avoid treating falsy-but-valid values as absent.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useUser = (): CustomUserProfile => { | |
| const { user: registeredUser } = useCustomUser(); | |
| const { user: anonymousUser } = useStoredAnonymousUser(); | |
| const user = useMemo(() => registeredUser || anonymousUser, [registeredUser, anonymousUser]); | |
| const { user: registeredUser, isLoading: isLoadingRegisteredUser } = useCustomUser(); | |
| const { user: anonymousUser, isLoading: isLoadingAnonymousUser } = useStoredAnonymousUser(); | |
| const user = useMemo(() => registeredUser || anonymousUser || {}, [registeredUser, anonymousUser]); | |
| const isLoading = useMemo(() => isLoadingRegisteredUser || isLoadingAnonymousUser, [isLoadingRegisteredUser, isLoadingAnonymousUser]); | |
| return user; | |
| return { | |
| ...user, | |
| isLoading | |
| }; | |
| export const useUser = (): Partial<CustomUserProfile> & { isLoading: boolean } => { | |
| const { user: registeredUser, isLoading: isLoadingRegisteredUser } = useCustomUser(); | |
| const { user: anonymousUser, isLoading: isLoadingAnonymousUser } = useStoredAnonymousUser(); | |
| const user = useMemo(() => registeredUser ?? anonymousUser, [registeredUser, anonymousUser]); | |
| const isLoading = useMemo(() => isLoadingRegisteredUser || isLoadingAnonymousUser, [isLoadingRegisteredUser, isLoadingAnonymousUser]); | |
| return { | |
| ...(user ?? {}), | |
| isLoading | |
| }; | |
| }; |
🤖 Prompt for AI Agents
In apps/deploy-web/src/hooks/useUser.ts around lines 7 to 16, the function
returns {} as a user and uses || which is type-unsafe and can mix UI state into
the domain object; change the function return type to Partial<CustomUserProfile>
& { isLoading: boolean }, replace the user fallback {} with undefined (i.e.
compute user = useMemo(() => registeredUser ?? anonymousUser, [...])), use ??
instead of || for nullish coalescing, and when returning only spread the
resolved user if present (e.g. ...(user ? user : {})) alongside a guaranteed
isLoading boolean so callers keep the same shape without unsafe empty-object
fallbacks.
There was a problem hiding this comment.
@domhhv not sure empty object for a user makes sense. if there is no user it should be undefined or null
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
I'd also separate isLoading from userProfile data. This would look and behave consistent with other places
There was a problem hiding this comment.
I made user and isLoading two separate properties returned from useUser()
| const { canVisit, isLoading } = useCheck(); | ||
|
|
||
| if (isLoading) { | ||
| return <Loading text="Please wait..." />; |
There was a problem hiding this comment.
question(non-blocking): Is there a way we can customize the text in different pages?
There was a problem hiding this comment.
I think we could pass text as an argument to the Guard() call, after optional FallbackComponent param or by combining the two into an options param
| expect(userTracker.track).toHaveBeenCalledWith({ | ||
| isLoading: true | ||
| }); | ||
| expect(userTracker.track).toHaveBeenCalledWith({ | ||
| ...anonymousUser, | ||
| isLoading: false | ||
| }); |
There was a problem hiding this comment.
issue(blocking): userTracker.track should not receive an object without .id because then it will behave in the way that user exists but without properties. it's either user or falsy value
There was a problem hiding this comment.
I included user as a property in the return value of useUser(), so now userTracker.track here receives either undefined or full user value
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 (5)
apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (2)
55-55: Replace Array with a typed ref handleAvoid any in TS/TSX per guidelines and type the LeaseRow ref so calls like lr.current?.getLeaseStatus() are type-safe.
Apply these diffs within the selected lines:
- const [leaseRefs, setLeaseRefs] = useState<Array<any>>([]); + const [leaseRefs, setLeaseRefs] = useState<React.RefObject<LeaseRowHandle>[]>([]);- .map((_, i) => elRefs[i] || createRef()) + .map((_, i) => elRefs[i] || createRef<LeaseRowHandle>())Add this handle type near the top of the file (or colocate with LeaseRow):
type LeaseRowHandle = { getLeaseStatus: () => void };Also applies to: 84-88
93-94: Avoid casting errors to any; add a narrow type guard and prefer status checksCasting to any breaks type safety and violates repo guidelines. Also, checking the HTTP status (404) is more robust than message text matching.
Apply this change to the predicate:
- const isDeploymentNotFound = deploymentError && (deploymentError as any).response?.data?.message?.includes("Deployment not found") && !isLoadingDeployment; + const isDeploymentNotFound = + isApiErrorWithMessage(deploymentError) && + (deploymentError.response?.status === 404 || + deploymentError.response?.data?.message?.includes("Deployment not found")) && + !isLoadingDeployment;Add this helper in the module (top-level):
type ApiError = { response?: { status?: number; data?: { message?: string } } }; function isApiErrorWithMessage(e: unknown): e is ApiError { return typeof e === "object" && e !== null && "response" in (e as Record<string, unknown>); }apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
169-175: Payment Method step should not be disabled when there are no payment methods.Disabling this step when
paymentMethods.length === 0blocks users from navigating to the step where they add a card, creating a dead-end in the flow. Advancement to “Welcome” is already guarded inhandleStepChange(Lines 83–87).Apply this diff:
{ id: "payment-method", title: "Payment Method", description: "Add payment info", component: null, isCompleted: completedSteps.has(OnboardingStepIndex.PAYMENT_METHOD), - isDisabled: paymentMethods.length === 0 + // Allow navigation to add a payment method; progression to "Welcome" is guarded elsewhere. + isDisabled: false },apps/deploy-web/src/queries/useApiKeysQuery.ts (2)
47-51: Guard cache updates when userId is absent to avoid poisoning the '' query key.If
user?.userIdis falsy, this writes togetApiKeysKey(''), which can cause collisions and stale cache scenarios.Apply this diff:
- queryClient.setQueryData(QueryKeys.getApiKeysKey(user?.userId ?? ""), (oldData: ApiKeyResponse[] | undefined) => { - if (!oldData) return [_response]; - return [...oldData, _response]; - }); + if (!user?.userId) return; + queryClient.setQueryData(QueryKeys.getApiKeysKey(user.userId), (oldData: ApiKeyResponse[] | undefined) => { + if (!oldData) return [_response]; + return [...oldData, _response]; + });
63-66: Same guarding needed for delete mutation cache update.Prevent updates to a cache key built from an empty user id.
Apply this diff:
- queryClient.setQueryData(QueryKeys.getApiKeysKey(user?.userId ?? ""), (oldData: ApiKeyResponse[] = []) => { - return oldData.filter(t => t.id !== id); - }); + if (!user?.userId) return; + queryClient.setQueryData(QueryKeys.getApiKeysKey(user.userId), (oldData: ApiKeyResponse[] = []) => { + return oldData.filter(t => t.id !== id); + });
♻️ Duplicate comments (1)
apps/deploy-web/src/hooks/useUser.ts (1)
7-19: Fix return typing and prefer nullish coalescing to reflect actual runtime shape
usercan beundefinedwhile data is loading; typing it as non-optionalCustomUserProfileis unsound and pushes the risk downstream. Also prefer??over||to avoid treating falsy-but-valid values as absent.-export const useUser = (): { - user: CustomUserProfile; - isLoading: boolean; -} => { +export const useUser = (): { + user?: CustomUserProfile; + isLoading: boolean; +} => { const { user: registeredUser, isLoading: isLoadingRegisteredUser } = useCustomUser(); const { user: anonymousUser, isLoading: isLoadingAnonymousUser } = useStoredAnonymousUser(); - const user = useMemo(() => registeredUser || anonymousUser, [registeredUser, anonymousUser]); + const user = useMemo(() => registeredUser ?? anonymousUser, [registeredUser, anonymousUser]); const isLoading = useMemo(() => isLoadingRegisteredUser || isLoadingAnonymousUser, [isLoadingRegisteredUser, isLoadingAnonymousUser]); return { user, isLoading }; };Note: Making
useroptional better matches consumer usage with optional chaining across the codebase, and eliminates reliance on an implicitly always-defined profile during initial load.
🧹 Nitpick comments (9)
apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (1)
265-266: Pass a safe default for editedManifest instead of a type castCasting null to string at compile time won’t prevent runtime null propagation. Provide a fallback string to match the component contract.
- editedManifest={editedManifest as string} + editedManifest={editedManifest ?? ""}apps/deploy-web/src/hooks/useHasCreditCardBanner.ts (1)
28-32: Banner visibility never turns off after conditions changeThe effect only sets visibility to true; it never hides the banner if, for example, a managed wallet is later created. Keep the state in sync with the derived predicate.
- useEffect(() => { - if (shouldShowBanner) { - setIsBannerVisible(true); - } - }, [shouldShowBanner]); + useEffect(() => { + setIsBannerVisible(shouldShowBanner); + }, [shouldShowBanner]);apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (2)
53-55: Avoid fragile enum size math when restoring the saved step.
Object.keys(Enum).length / 2relies on TS enum reverse-mapping details. Prefer explicit bounds for clarity and durability.Apply this diff:
- if (step >= 0 && step < Object.keys(OnboardingStepIndex).length / 2) { + const minStep = OnboardingStepIndex.FREE_TRIAL; + const maxStep = OnboardingStepIndex.WELCOME; + if (step >= minStep && step <= maxStep) { setCurrentStep(step); }
89-96: Deduplicate step name literals.
stepNamesis declared twice; extract a top-level constant to prevent drift.Example (apply outside these ranges as appropriate):
const STEP_NAMES = ["free_trial", "signup", "email_verification", "payment_method", "welcome"]; // ... analyticsService.track("onboarding_step_started", { category: "onboarding", step: STEP_NAMES[step], step_index: step }); // ... analyticsService.track("onboarding_step_completed", { category: "onboarding", step: STEP_NAMES[step], step_index: step });Also applies to: 104-111
apps/deploy-web/src/context/FlagProvider/FlagProvider.tsx (1)
17-19: Destructuring is correct; verify consistent user identifier property.You pass
user?.idinto Unleash context, whereas other parts of the app often useuser?.userId. Ensure the chosen field is stable and unique across registered and anonymous users to avoid segment/flag misattribution.Apply this diff to prefer a stable fallback:
- return <c.FlagProvider config={{ context: { userId: user?.id } }}>{children}</c.FlagProvider>; + return <c.FlagProvider config={{ context: { userId: user?.userId ?? user?.id } }}>{children}</c.FlagProvider>;Confirm which identifier Unleash targeting expects (registered id vs. anonymous id), and standardize across the codebase.
apps/deploy-web/src/pages/payment.tsx (2)
79-86: Avoid sending empty userId in payment confirmation.While this page is auth-protected, defensive checks prevent accidental API calls with
userId: ""during transient states.Apply this diff:
try { - await confirmPayment({ - userId: user?.id || "", + if (!user?.id) { + enqueueSnackbar(<Snackbar title="Please sign in to complete payment" iconVariant="error" />, { variant: "error" }); + return; + } + await confirmPayment({ + userId: user.id, paymentMethodId, amount: parseFloat(amount), currency: "usd" });
116-118: Same guard for coupon application.Avoid applying coupons with a blank user id.
Apply this diff:
- const response = await applyCoupon({ coupon, userId: user?.id || "" }); + if (!user?.id) { + enqueueSnackbar(<Snackbar title="Please sign in to apply a coupon" iconVariant="error" />, { variant: "error" }); + return; + } + const response = await applyCoupon({ coupon, userId: user.id });apps/deploy-web/src/hooks/useLoginRequiredEventHandler.tsx (1)
40-43: Ensure identifier consistency across hooks.This hook checks
user?.userId. Other modules sometimes useuser?.id. If anonymous users can exist, keep usinguserIdhere for “registered” checks, but consider a small utility (e.g.,getStableUserId(user)) to reduce drift.If desired, I can propose a tiny shared helper and update call sites in a follow-up PR.
apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.tsx (1)
8-15: Use isLoading to avoid transient 404s in this HOC as wellGiven useUser now exposes isLoading, returning the fallback before loading completes can still cause flicker in routes wrapped by this HOC. Gate on isLoading first.
- const { user } = useUser(); + const { user, isLoading } = useUser(); const isRegistered = !!user?.userId; - if (isRegistered) { + if (isLoading) { + return null; // or a dedicated <Loading/> component + } + if (isRegistered) { return <Component {...props} />; } return <FallbackComponent />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
apps/deploy-web/src/components/alerts/AccountEmailChannelCreator/AccountEmailChannelCreator.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentDetail.tsx(1 hunks)apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx(1 hunks)apps/deploy-web/src/components/layout/Sidebar.tsx(1 hunks)apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx(1 hunks)apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx(1 hunks)apps/deploy-web/src/context/FlagProvider/FlagProvider.tsx(1 hunks)apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx(2 hunks)apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.tsx(1 hunks)apps/deploy-web/src/hooks/useHasCreditCardBanner.ts(1 hunks)apps/deploy-web/src/hooks/useLoginRequiredEventHandler.tsx(1 hunks)apps/deploy-web/src/hooks/useManagedWallet.ts(1 hunks)apps/deploy-web/src/hooks/useUser.ts(1 hunks)apps/deploy-web/src/pages/payment.tsx(1 hunks)apps/deploy-web/src/queries/useApiKeysQuery.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.tsxapps/deploy-web/src/components/alerts/AccountEmailChannelCreator/AccountEmailChannelCreator.tsxapps/deploy-web/src/hooks/useLoginRequiredEventHandler.tsxapps/deploy-web/src/hooks/useHasCreditCardBanner.tsapps/deploy-web/src/components/deployments/DeploymentDetail.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/hooks/useManagedWallet.tsapps/deploy-web/src/context/FlagProvider/FlagProvider.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/queries/useApiKeysQuery.tsapps/deploy-web/src/pages/payment.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/layout/Sidebar.tsxapps/deploy-web/src/hooks/useUser.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.tsxapps/deploy-web/src/components/alerts/AccountEmailChannelCreator/AccountEmailChannelCreator.tsxapps/deploy-web/src/hooks/useLoginRequiredEventHandler.tsxapps/deploy-web/src/hooks/useHasCreditCardBanner.tsapps/deploy-web/src/components/deployments/DeploymentDetail.tsxapps/deploy-web/src/components/user/UserProviders/UserProviders.tsxapps/deploy-web/src/hooks/useManagedWallet.tsapps/deploy-web/src/context/FlagProvider/FlagProvider.tsxapps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsxapps/deploy-web/src/queries/useApiKeysQuery.tsapps/deploy-web/src/pages/payment.tsxapps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsxapps/deploy-web/src/components/layout/Sidebar.tsxapps/deploy-web/src/hooks/useUser.ts
🧬 Code graph analysis (11)
apps/deploy-web/src/hoc/registered-users-only/registered-users-only.hoc.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/components/alerts/AccountEmailChannelCreator/AccountEmailChannelCreator.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/hooks/useLoginRequiredEventHandler.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/hooks/useHasCreditCardBanner.ts (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/hooks/useManagedWallet.ts (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/pages/payment.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/components/layout/Sidebar.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
apps/deploy-web/src/hooks/useUser.ts (3)
apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/src/hooks/useCustomUser.ts (1)
useCustomUser(16-26)apps/deploy-web/src/hooks/useStoredAnonymousUser.ts (1)
useStoredAnonymousUser(18-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (14)
apps/deploy-web/src/components/deployments/DeploymentDetail.tsx (1)
59-61: ConfirmidvsuserIdsemantics & Guard HOC loading behaviorPer the
CustomUserProfileinsrc/types/user.ts, the two fields are indeed distinct:
user.id– the internal settings record IDuser.userId– the external Auth0 user ID (foreign key)All existing uses align with that split:
- gating alerts, sidebar routes, “registered user” checks, etc. use
user?.userId- flag‐provider, payment flows, deployment‐settings queries, etc. use
user?.idwhere the settings record ID is requiredTo avoid future confusion, you may optionally rename these as
settingsIdvs.auth0UserId(and update theFlagProviderto mapcontext.userIdfromuser.userIdif your feature-flag backend expects the Auth0 ID).On the guards side, the
GuardHOC andcomposeGuardscorrectly short-circuit on anyisLoadingflag—rendering the<Loading>spinner before ever falling back to the 404 component—so the 404-flicker issue is resolved.Everything else looks good to merge.
apps/deploy-web/src/hooks/useManagedWallet.ts (1)
16-21: Verifiedenabledflag; please confirmidvsuserIdsemantics
- ✅ Checked
useManagedWalletQueryinapps/deploy-web/src/queries/useManagedWalletQuery.ts– it hasenabled: !!userId, so it won’t fire whenuserIdis undefined.⚠️ Noticed mixed usage ofuser?.id(e.g. inuseManagedWalletQuery) anduser?.userId(e.g. inuseApiKeysQuery). Please confirm that both properties refer to the same identifier by design and update documentation or rename for consistency to prevent future drift.apps/deploy-web/src/components/deployments/DeploymentDetailTopBar.tsx (1)
58-60: Approved: invocation of useDeploymentSettingQuery is correct and already gatedThe hook’s
useQuerycall includesenabled: !!params.userId && !!params.dseq && !!wallet.isManaged, so requests won’t fire with an undefineduserId, and transient 404s on mount are already prevented. No further changes needed.apps/deploy-web/src/hooks/useHasCreditCardBanner.ts (1)
12-12: LGTM: correct destructuring from useUser()apps/deploy-web/src/components/alerts/AccountEmailChannelCreator/AccountEmailChannelCreator.tsx (1)
26-30: LGTM: uses nested user object and guards on presence of emailPassing email when available is correct, and the trigger stays inert otherwise.
apps/deploy-web/src/components/onboarding/OnboardingContainer/OnboardingContainer.tsx (1)
45-46: Destructuring useUser return shape looks correct.Aligns with the new
{ user, isLoading }API and avoids accidental access to the wrapper object.apps/deploy-web/src/queries/useApiKeysQuery.ts (2)
19-20: Destructuring{ user }fromuseUseris consistent with the new hook contract.Readability improves and avoids accidental access to the wrapper object.
Also applies to: 35-36, 56-57
26-27: Double-check enablement logic with Wallet flags.
enabled: !!user?.userId && !isTrialing && isManagedassumes API keys are only available for managed wallets and not during trial. Please confirm these invariants align with product rules; otherwise queries may never run for eligible users.If needed, I can scan call sites for
isManagedsemantics across the repo to verify consistency.apps/deploy-web/src/pages/payment.tsx (2)
32-33: Destructuring{ user }fromuseUseraligns with the new contract.All downstream accesses use
user?.id, which matches the back-end expectations here.
81-82: Confirm the canonical user identifier.This file uses
user.id, while other files (API keys, login handler) often referenceuser.userId. Please confirm the back-end API expectsidin both confirm payment and coupon endpoints, and standardize across the app for consistency.I can help run a quick repo scan to enumerate usages of
user.idvsuser.userIdto surface inconsistencies.Also applies to: 117-118
apps/deploy-web/src/hooks/useLoginRequiredEventHandler.tsx (1)
10-11: Destructuring{ user }matches the updated hook API.The login gate using
user?.userIdremains correct for registered-user checks.apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx (1)
31-44: LGTM: destructuring aligns with the new useUser shapeSwitching to
const { user } = useUser()is correct and keeps the identify call guarded byuser?.id. No issues spotted.apps/deploy-web/src/components/layout/Sidebar.tsx (1)
67-115: LGTM: updated destructuring and guards are correctUsing
const { user } = useUser()and guarding Alerts withuser?.userIdkeeps behavior intact and avoids null dereferences. Memo deps includeuser?.userIdandwallet.isManaged, which is appropriate.apps/deploy-web/src/hooks/useUser.ts (1)
22-29: Prevent crashes by guarding alluser.userIdaccesses and add return type to hookIn
apps/deploy-web/src/hooks/useUser.ts, update the hook signature and guard theuserobject:-export const useIsRegisteredUser = () => { +export const useIsRegisteredUser = (): { isLoading: boolean; canVisit: boolean } => { const { isLoading, user } = useUser(); return { - isLoading, - canVisit: !!user.userId + isLoading, + canVisit: !!user?.userId }; };Additionally, similar patterns in the front-end should be updated to avoid unguarded access:
• useManagedWallet hook
File:apps/deploy-web/src/hooks/useManagedWallet.ts(around line 54)
Changeif (user?.id && !user.userId) { … }to
if (user?.id && !user?.userId) { … }• UserProviders component
File:apps/deploy-web/src/components/user/UserProviders/UserProviders.tsx(around line 40)
Changeanonymous: !user.userId,to
anonymous: !user?.userId,• UserProviders tests
File:apps/deploy-web/src/components/user/UserProviders/UserProviders.spec.tsx(around line 47)
Update expected props to match the new optional chaining (!user?.userId).• User-tracker service
File:apps/deploy-web/src/services/user-tracker/user-tracker.service.ts(around line 15)
Changeanonymous: user ? !user.userId : undefined,to
anonymous: user ? !user?.userId : undefined,Please manually verify that no remaining
user.userIdoccurrences exist without a guard (i.e. in any.tsor.tsxfile) and ensure all are wrapped inuser? .userIdor preceded by a null check.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx (3)
13-44: Optional: avoid showing the “Add Funds” modal while user state is still loadingIf a user clicks immediately on mount,
usermay still be resolving and we’ll open the “Add Funds” modal unnecessarily. Consider short‑circuiting clicks during user load (prevent default, no modal) to better match the PR’s goal of removing flicker/false-negatives during loading.Here’s a minimal change that preserves current behavior but suppresses the modal during load:
- const { user } = useUser(); + const { user, isLoading: isUserLoading } = useUser(); @@ - return user?.userId && isManaged && !isTrialing ? handler : preventer; + if (isUserLoading) { + // Suppress modal while loading; just block the click. + return event => { + event.preventDefault(); + }; + } + return user?.userId && isManaged && !isTrialing ? handler : preventer; @@ - [isTrialing, isManaged, requireAction, user?.userId] + [isTrialing, isManaged, requireAction, user?.userId, isUserLoading]
20-21: Minor UX: also stop propagation on blocked clicksWhen we block the action, consider calling
event.stopPropagation()as well. This prevents parent handlers from reacting (e.g., list rows with row-level onClick), reducing surprises.- event.preventDefault(); + event.preventDefault(); + event.stopPropagation();
17-44: Deps: includerouterfor exhaustive-deps compliance (optional)If your eslint config enforces exhaustive deps for hooks, add
routerto the dependency array.next/navigation’s router is stable, so this won’t cause churn, but it eliminates potential lint noise.- [isTrialing, isManaged, requireAction, user?.userId] + [isTrialing, isManaged, requireAction, user?.userId, router]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx
🧬 Code graph analysis (1)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx (1)
apps/deploy-web/src/hooks/useUser.ts (1)
useUser(7-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (1)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx (1)
13-13: Destructure from updated useUser API — LGTMSwitching to
const { user } = useUser();aligns this hook with the new{ user, isLoading }shape and keeps its logic unchanged.
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 (3)
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsx (3)
5-29: Refactor test to comply with repo testing guidelines: use queryBy, add setup() at bottom, and removeany*
- Per retrieved learnings and coding guidelines for apps/.../*.spec.tsx, replace
getBy*withqueryBy*in expectations.- Introduce a
setupfunction at the bottom of the rootdescribeblock; don’t specify its return type, accept a single param with inline type, avoid shared state.- Replace
anywith a minimal inline type for thecustomFlagProviderprops.Based on these rules, here’s a cohesive patch:
@@ -import { render } from "@testing-library/react"; +import { render, screen } from "@testing-library/react"; @@ describe(UserAwareFlagProvider.name, () => { it("passes userId from useUser to the custom FlagProvider", () => { - const testUser = { id: "my-user-id" }; - const customFlagProvider = ({ config, children }: any) => ( - <div data-testid="flag-provider"> - {config.context.userId} - {children} - </div> - ); - const customUseUser = () => ({ - user: testUser, - isLoading: false - }); - - const { getByTestId } = render( - <UserAwareFlagProvider components={{ FlagProvider: customFlagProvider, useUser: customUseUser }}> - <div data-testid="child" /> - </UserAwareFlagProvider> - ); - - expect(getByTestId("flag-provider").textContent).toContain("my-user-id"); - expect(getByTestId("child")).toBeInTheDocument(); + // Arrange + Act via setup + setup({ userId: "my-user-id" }); + // Assert + expect(screen.queryByTestId("flag-provider")).toHaveTextContent("my-user-id"); + expect(screen.queryByTestId("child")).toBeInTheDocument(); }); + + // setup must be at the bottom of the root describe + const setup = ({ userId }: { userId: string }) => { + const testUser = { id: userId }; + type CustomFlagProviderProps = { + config: { context: { userId: string } }; + children?: React.ReactNode; + }; + const customFlagProvider = ({ config, children }: CustomFlagProviderProps) => ( + <div data-testid="flag-provider"> + {config.context.userId} + {children} + </div> + ); + const customUseUser = () => ({ + user: testUser, + isLoading: false + }); + render( + <UserAwareFlagProvider components={{ FlagProvider: customFlagProvider, useUser: customUseUser }}> + <div data-testid="child" /> + </UserAwareFlagProvider> + ); + }; });Notes:
- This removes the banned
anyand uses a minimal inline type.- It swaps
getByTestIdforscreen.queryByTestIdin expectations.- It adds the required
setupfunction at the bottom of the rootdescribewith inline param typing and no explicit return type.
10-15: Removeanytyping on customFlagProvider propsIf you prefer a smaller, localized change without the full refactor above, at minimum replace
anywith a minimal inline type to satisfy “never use any”:- const customFlagProvider = ({ config, children }: any) => ( + const customFlagProvider = ( + { config, children }: { config: { context: { userId: string } }; children?: React.ReactNode } + ) => ( <div data-testid="flag-provider"> {config.context.userId} {children} </div> );
1-31: ReplacegetBy*withqueryBy*in test expectations– In FlagProvider.spec.tsx (apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsx):
• Update the render call to destructurequeryByTestIdinstead ofgetByTestId.
• Change
diff - const { getByTestId } = render(...) - expect(getByTestId("flag-provider").textContent).toContain("my-user-id"); - expect(getByTestId("child")).toBeInTheDocument(); + const { queryByTestId } = render(...) + expect(queryByTestId("flag-provider")?.textContent).toContain("my-user-id"); + expect(queryByTestId("child")).toBeInTheDocument();– Repo-wide there are still dozens of
getBy*calls in.spec.tsxunder
apps/deploy-webandapps/provider-console. To comply with our guideline
(usequeryBy*for test expectations), please refactor these toqueryBy*.– Consider adding or enabling a lint rule/Cursor rule to flag any remaining
getBy[A-Za-z]*(usage in.spec.tsxso this doesn’t regress.
♻️ Duplicate comments (3)
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsx (1)
21-29: Use queryBy in expectations per repo standards*Per the retrieved learnings for apps/{deploy-web,provider-console}/**/.spec.tsx, switch from getBy to queryBy* in expectations:
- const { getByTestId } = render( + render( <UserAwareFlagProvider components={{ FlagProvider: customFlagProvider, useUser: customUseUser }}> <div data-testid="child" /> </UserAwareFlagProvider> ); - expect(getByTestId("flag-provider").textContent).toContain("my-user-id"); - expect(getByTestId("child")).toBeInTheDocument(); + expect(screen.queryByTestId("flag-provider")).toHaveTextContent("my-user-id"); + expect(screen.queryByTestId("child")).toBeInTheDocument();Remember to import
screen:-import { render } from "@testing-library/react"; +import { render, screen } from "@testing-library/react";apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (2)
191-194: Repeat of the inline useUser mock; reuse the shared helper.Same suggestion as Lines 143–146: replace with
useUser: mockUseUserto avoid duplication and keep type parity with the dependency.
239-242: Repeat of the inline useUser mock; reuse the shared helper.Same suggestion as Lines 143–146: replace with
useUser: mockUseUserfor consistency and maintainability.
🧹 Nitpick comments (1)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (1)
143-146: Good adaptation to new useUser shape; consider centralizing the mock.The inline mock matches the new
{ user, isLoading }contract. To DRY this across tests and keep types aligned with the dependency, extract a reusable helper typed against the dependency signature.Apply within this hunk:
- useUser: () => ({ - user: mockUser, - isLoading: false - }), + useUser: mockUseUser,Add once near the top of the file (outside this hunk) to support the change:
const mockUseUser: typeof USE_API_KEYS_DEPENDENCIES["useUser"] = () => ({ user: mockUser, isLoading: false });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsx(1 hunks)apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
apps/{deploy-web,provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations in.spec.tsxfiles
Files:
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never use type any or cast to type any. Always define the proper TypeScript types.
Files:
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code
Files:
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-21T08:24:27.953Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/query-by-in-tests.mdc:0-0
Timestamp: 2025-07-21T08:24:27.953Z
Learning: Applies to apps/{deploy-web,provider-console}/**/*.spec.tsx : Use `queryBy` methods instead of `getBy` methods in test expectations in `.spec.tsx` files
Applied to files:
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (2)
apps/deploy-web/src/context/FlagProvider/FlagProvider.spec.tsx (1)
16-19: Mock shape aligns with updated useUser API — good changeReturning
{ user, isLoading }keeps the test in sync with the new hook contract and prevents brittle destructuring at call sites.apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (1)
269-272: I’ve requested the hook files and theuseUserimplementation to confirm its return type. Once the results are in, I can verify whetheruseris already optional or needs updating before finalizing the recommendation.
4 participants
Fixes #1822
Summary by CodeRabbit
New Features
Bug Fixes