feat(auth): enhance API key management with new hooks and tests#1813
feat(auth): enhance API key management with new hooks and tests#1813
Conversation
WalkthroughAdds wallet-managed gating to API key queries and UI; introduces dependency injection for API key hooks; changes paywall actions to receive a close callback; adds tests and seeders for API keys and wallets; and adds an explicit return type for usePopup. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ApiKeyList
participant Hooks as useUserApiKeys / Mutations
participant Wallet as useWallet (DI)
participant User as useUser (DI)
participant API as ApiKeyHttpService
UI->>Hooks: invoke query/mutation
Hooks->>Wallet: read isManaged
Hooks->>User: read user, isTrialing
alt isManaged && user && !isTrialing
Hooks->>API: get/create/delete
API-->>Hooks: response
Hooks-->>UI: data/result
else not eligible
Hooks-->>UI: undefined / no-op
end
sequenceDiagram
participant Handler as usePayingCustomerRequiredEventHandler
participant Require as requireAction / Popup
participant Router as router
Handler->>Require: actions = ({ close }) => [...]
Require-->>Handler: render actions
Handler->>Router: push(payment)
Handler->>Require: close()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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
🪧 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 (
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1813 +/- ##
==========================================
- Coverage 42.62% 42.44% -0.18%
==========================================
Files 938 933 -5
Lines 26444 26271 -173
Branches 6924 6911 -13
==========================================
- Hits 11273 11152 -121
- Misses 13997 14757 +760
+ Partials 1174 362 -812
*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}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
packages/ui/context/PopupProvider/PopupProvider.tsx (1)
6-15: Strengthen CustomPopupProps typing to support functional actionsYou’re supporting function-based actions at runtime, but the current type doesn’t explicitly allow it. Tighten the type to encode the functional form and avoid implicit anys.
-type ConfirmPopupProps = string | (Omit<CommonProps, "onClose" | "open"> & Omit<ConfirmProps, "onValidate" | "onCancel" | "variant">); -type SelectPopupProps = Omit<CommonProps, "onClose" | "open"> & Omit<SelectProps, "onValidate" | "onCancel" | "variant">; -type CustomPopupProps = Omit<CommonProps, "onClose" | "open"> & Omit<CustomPrompt, "onValidate" | "onCancel" | "variant">; +type ConfirmPopupProps = string | (Omit<CommonProps, "onClose" | "open"> & Omit<ConfirmProps, "onValidate" | "onCancel" | "variant">); +type SelectPopupProps = Omit<CommonProps, "onClose" | "open"> & Omit<SelectProps, "onValidate" | "onCancel" | "variant">; +type BaseCustomPopupProps = Omit<CommonProps, "onClose" | "open"> & Omit<CustomPrompt, "onValidate" | "onCancel" | "variant" | "actions">; +type CustomPopupProps = BaseCustomPopupProps & { + actions?: CustomPrompt["actions"] | ((ctx: { close: () => void }) => CustomPrompt["actions"]); +}; type PopupProviderContext = { confirm: (messageOrProps: ConfirmPopupProps) => Promise<boolean>; select: (props: SelectPopupProps) => Promise<string | undefined>; requireAction: (props: CustomPopupProps) => Promise<undefined>; createCustom: (props: CustomPopupProps) => void; };apps/deploy-web/src/components/api-keys/ApiKeyList.tsx (1)
81-84: Avoid mutating props: sort a copy before mappingTable rendering currently sorts apiKeys in place, mutating props. Clone before sort to avoid side effects.
- {apiKeys - ?.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) - ?.map(key => ( + {(apiKeys ?? []) + .slice() + .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) + .map(key => (
🧹 Nitpick comments (6)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx (2)
19-21: Stop event propagation to avoid unintended parent handlersYou already prevent default. Add stopPropagation() to ensure no parent onClick handlers run and accidentally trigger navigation or other side effects.
- const preventer: MouseEventHandler = event => { - event.preventDefault(); + const preventer: MouseEventHandler = event => { + event.preventDefault(); + event.stopPropagation(); requireAction({
43-44: Include router in useCallback deps to avoid stale closuresrouter is captured in the callback; include it in the dependency array to satisfy exhaustive-deps and avoid potential staleness.
- [isTrialing, isManaged, requireAction, user?.userId] + [isTrialing, isManaged, requireAction, user?.userId, router]packages/ui/context/PopupProvider/PopupProvider.tsx (1)
87-112: Use Subject for requireAction clarityrequireAction does not return a meaningful value; a Subject communicates that intent better than Subject<SelectOption["value"] | undefined>.
- const requireAction: PopupProviderContext["requireAction"] = useCallback( - (props: CustomPopupProps) => { - let subject: Subject<SelectOption["value"] | undefined> | undefined = new Subject<SelectOption["value"] | undefined>(); + const requireAction: PopupProviderContext["requireAction"] = useCallback( + (props: CustomPopupProps) => { + let subject: Subject<void> | undefined = new Subject<void>(); const close = () => { if (subject) { - subject.next(undefined); + subject.next(); subject.complete(); setPopupProps(undefined); subject = undefined; } }; setPopupProps({ title: "Action Required", ...props, actions: typeof props.actions === "function" ? props.actions({ close }) : props.actions, open: true, variant: "custom", onClose: close }); - return firstValueFrom(subject).then(() => undefined); + return firstValueFrom(subject).then(() => undefined); }, [setPopupProps] );apps/deploy-web/tests/seeders/wallet.ts (1)
12-15: Preserve function signatures with typed jest mocksType the mocks against the WalletProviderContextType function signatures to retain type safety in tests.
- connectManagedWallet: jest.fn(), - logout: jest.fn(), - signAndBroadcastTx: jest.fn(), + connectManagedWallet: jest.fn() as jest.MockedFunction<WalletProviderContextType["connectManagedWallet"]>, + logout: jest.fn() as jest.MockedFunction<WalletProviderContextType["logout"]>, + signAndBroadcastTx: jest.fn() as jest.MockedFunction<WalletProviderContextType["signAndBroadcastTx"]>, isManaged: true, @@ - switchWalletType: jest.fn(), + switchWalletType: jest.fn() as jest.MockedFunction<WalletProviderContextType["switchWalletType"]>,Also applies to: 21-21
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (2)
19-28: Test assertion should verify query is disabled, not just undefined dataThe test title says "should return null" but the assertion checks for
undefined. Additionally, whenenabledis false in React Query, the query won't execute anddatawill be undefined by default. Consider also asserting that the query is not fetching to make the test more explicit.it("should return null when user is not provided", async () => { const { result } = setupApiKeysQuery({ user: undefined, wallet: mockWallet }); await waitFor(() => { expect(result.current.query.data).toBeUndefined(); + expect(result.current.query.isFetching).toBe(false); }); });
30-39: Consistent test assertions needed for disabled query statesSimilar to the previous comment, these tests should also verify that the query is not fetching when disabled due to
isTrialingor!isManagedconditions.it("should return null when user is trialing", async () => { const { result } = setupApiKeysQuery({ user: mockUser, wallet: { ...mockWallet, isTrialing: true } }); await waitFor(() => { expect(result.current.query.data).toBeUndefined(); + expect(result.current.query.isFetching).toBe(false); }); }); it("should return null when wallet is not managed", async () => { const { result } = setupApiKeysQuery({ user: mockUser, wallet: { ...mockWallet, isManaged: false } }); await waitFor(() => { expect(result.current.query.data).toBeUndefined(); + expect(result.current.query.isFetching).toBe(false); }); });Also applies to: 41-50
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/deploy-web/src/components/api-keys/ApiKeyList.tsx(2 hunks)apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx(1 hunks)apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx(1 hunks)apps/deploy-web/src/queries/useApiKeysQuery.ts(2 hunks)apps/deploy-web/tests/seeders/apiKey.ts(1 hunks)apps/deploy-web/tests/seeders/index.ts(1 hunks)apps/deploy-web/tests/seeders/wallet.ts(1 hunks)packages/ui/context/PopupProvider/PopupProvider.tsx(1 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/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/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/queries/useApiKeysQuery.spec.tsxapps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsxapps/deploy-web/tests/seeders/wallet.tsapps/deploy-web/tests/seeders/apiKey.tsapps/deploy-web/src/components/api-keys/ApiKeyList.tsxpackages/ui/context/PopupProvider/PopupProvider.tsxapps/deploy-web/src/queries/useApiKeysQuery.tsapps/deploy-web/tests/seeders/index.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/queries/useApiKeysQuery.spec.tsxapps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsxapps/deploy-web/tests/seeders/wallet.tsapps/deploy-web/tests/seeders/apiKey.tsapps/deploy-web/src/components/api-keys/ApiKeyList.tsxpackages/ui/context/PopupProvider/PopupProvider.tsxapps/deploy-web/src/queries/useApiKeysQuery.tsapps/deploy-web/tests/seeders/index.ts
🧠 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/queries/useApiKeysQuery.spec.tsx
🧬 Code Graph Analysis (3)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (6)
apps/deploy-web/tests/seeders/apiKey.ts (1)
buildApiKey(4-13)apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/tests/seeders/user.ts (1)
buildUser(7-25)apps/deploy-web/tests/seeders/wallet.ts (1)
buildWallet(7-25)packages/http-sdk/src/api-key/api-key-http.service.ts (1)
ApiKeyHttpService(4-16)apps/deploy-web/src/queries/useApiKeysQuery.ts (4)
USE_API_KEYS_DEPENDENCIES(10-13)useCreateApiKey(34-53)useDeleteApiKey(55-69)useUserApiKeys(15-32)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx (1)
apps/deploy-web/src/utils/urlUtils.ts (1)
UrlService(16-81)
apps/deploy-web/src/queries/useApiKeysQuery.ts (3)
apps/api/src/auth/http-schemas/api-key.schema.ts (1)
ApiKeyResponse(58-58)packages/http-sdk/src/api-key/api-key-http.types.ts (1)
ApiKeyResponse(32-32)apps/deploy-web/src/queries/queryKeys.ts (1)
QueryKeys(1-107)
⏰ 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). (5)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (11)
apps/deploy-web/src/hooks/usePayingCustomerRequiredEventHandler.tsx (2)
27-35: Good use of close callback in popup actionsSwitching actions to a function and invoking close() after navigation is correct and aligns with the updated Popup API.
41-41: Confirm gating logic matches product intentWe’ve verified that across the codebase all “paying customer” gates use the exact same condition:
user?.userId && isManaged && !isTrialingKey instances include:
- usePayingCustomerRequiredEventHandler (this hook)
- useApiKeysQuery.ts (enabled:
!!user?.userId && !isTrialing && isManaged)- useVerifiedPayingCustomerLoginRequiredEventHandler (composes the above)
That means custodial (non-managed) wallets are currently blocked even if they hold on-chain funds.
Please confirm whether we should:
- Continue blocking custodial wallets entirely, or
- Broaden this condition (e.g. allow non-managed wallets once a deposit/payment is verified)
packages/ui/context/PopupProvider/PopupProvider.tsx (1)
140-146: Explicit return type for usePopup is a solid improvementTyping usePopup to return PopupProviderContext improves DX and prevents accidental misuse.
apps/deploy-web/src/components/api-keys/ApiKeyList.tsx (1)
81-112: Render API key rows or the empty state — never bothExtract and sort your keys once, then decide up front whether to show rows or the “No API keys found” message:
@@ apps/deploy-web/src/components/api-keys/ApiKeyList.tsx - const [isCreateModalOpen, setIsCreateModalOpen] = useState(false); + const [isCreateModalOpen, setIsCreateModalOpen] = useState(false); + // Sort once for readability and performance + const sortedApiKeys = (apiKeys ?? []) + .slice() + .sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()); + // Decide whether to render rows or the empty-state + const shouldShowEmptyState = + sortedApiKeys.length === 0 || isTrialing || !isManaged; @@ - {apiKeys - ?.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime()) - ?.map(key => ( + {!shouldShowEmptyState + ? sortedApiKeys.map(key => ( <TableRow key={key.id}> <TableCell>{key.name}</TableCell> {/* … */} </TableRow> )) + : ( + <TableRow> + <TableCell colSpan={5} className="text-center"> + No API keys found + </TableCell> + </TableRow> + )}• Sorting in render can be costly; hoisting it improves clarity and performance.
• Using a single boolean makes the JSX much easier to follow.
• Please confirm: Should trialing or non-managed wallets always show the empty state (hiding any existing keys), or only when there truly are zero keys?apps/deploy-web/tests/seeders/index.ts (1)
1-14: Barrel file looks goodClean aggregator for test seeders; reduces import noise and matches added modules.
apps/deploy-web/tests/seeders/apiKey.ts (2)
1-13: LGTM!The seeder implementation is well-structured with proper faker usage for generating realistic test data. The
buildApiKeyfunction correctly spreads overrides last to allow customization of generated values.
15-16: LGTM!The
buildApiKeyshelper correctly creates multiple API keys with the same overrides pattern, using a functional approach withArray.from.apps/deploy-web/src/queries/useApiKeysQuery.ts (4)
10-13: LGTM! Clean dependency injection patternThe introduction of
USE_API_KEYS_DEPENDENCIESprovides a clean separation of concerns and enables better testability by allowing dependency injection.
19-26: LGTM! Proper implementation of isManaged gatingThe implementation correctly gates API key operations behind the
isManagedflag along with existing conditions, and properly uses dependency injection.
34-52: LGTM! Clean dependency injection for create mutationThe
useCreateApiKeyhook properly uses dependency injection and correctly updates the query cache on success.
55-68: LGTM! Well-structured delete mutation with callback supportThe
useDeleteApiKeyhook properly implements dependency injection, updates the cache correctly by filtering out the deleted key, and supports an optional success callback.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
apps/deploy-web/src/queries/useApiKeysQuery.ts (1)
34-53: Guard useCreateApiKey against missing user, trial, and unmanaged walletsEven though the “Create Key” button is wrapped in a paying-customer guard, it’s safer to enforce this at the hook level so that any future callers won’t accidentally write under an empty user key or invoke the API in unsupported wallet states.
• apps/deploy-web/src/queries/useApiKeysQuery.ts (lines 34–53):
– DestructureisTrialingandisManagedfromdependencies.useWallet()
– InmutationFn, reject if!user?.userId,isTrialing, or!isManaged
– InonSuccess, skip cache updates whenuser?.userIdis falsyexport function useCreateApiKey( - dependencies: typeof USE_API_KEYS_DEPENDENCIES = USE_API_KEYS_DEPENDENCIES + dependencies: typeof USE_API_KEYS_DEPENDENCIES = USE_API_KEYS_DEPENDENCIES ) { const user = dependencies.useUser(); + const { isTrialing, isManaged } = dependencies.useWallet(); const queryClient = useQueryClient(); const { apiKey } = useServices(); return useMutation<ApiKeyResponse, Error, string>({ - mutationFn: (name: string) => - apiKey.createApiKey({ data: { name } }), + mutationFn: (name: string) => { + if (!user?.userId) { + return Promise.reject(new Error("Missing user ID")); + } + if (isTrialing) { + return Promise.reject(new Error("Cannot create API key during trial")); + } + if (!isManaged) { + return Promise.reject(new Error("Cannot create API key with unmanaged wallet")); + } + return apiKey.createApiKey({ data: { name } }); + }, onSuccess: _response => { - queryClient.setQueryData(QueryKeys.getApiKeysKey(user?.userId ?? ""), (oldData) => { - if (!oldData) return [_response]; + if (!user?.userId) return; + queryClient.setQueryData(QueryKeys.getApiKeysKey(user.userId), (oldData) => { + if (!oldData) return [_response]; return [...oldData, _response]; }); } }); }
🧹 Nitpick comments (5)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (5)
31-40: Align description and harden the trialing testThe title says “null” but the assertion checks
undefined. Also assert the query is disabled and the service is not called whenisTrialingis true.Apply this diff:
- it("should return null when user is trialing", async () => { - const { result } = setupApiKeysQuery({ - user: mockUser, - wallet: { ...mockWallet, isTrialing: true } - }); - - await waitFor(() => { - expect(result.current.query.data).toBeUndefined(); - }); - }); + it("should return undefined and not fetch when user is trialing", async () => { + const apiKeyService = mock<ApiKeyHttpService>({ + getApiKeys: jest.fn() + }); + const { result } = setupApiKeysQuery({ + user: mockUser, + wallet: { ...mockWallet, isTrialing: true }, + services: { + apiKey: () => apiKeyService + } + }); + + expect(result.current.query.fetchStatus).toBe("idle"); + expect(apiKeyService.getApiKeys).not.toHaveBeenCalled(); + expect(result.current.query.data).toBeUndefined(); + });
42-51: Harden the “wallet not managed” testMirror the pattern above: assert disabled state and that the service is not invoked.
Apply this diff:
- it("should return null when wallet is not managed", async () => { - const { result } = setupApiKeysQuery({ - user: mockUser, - wallet: { ...mockWallet, isManaged: false } - }); - - await waitFor(() => { - expect(result.current.query.data).toBeUndefined(); - }); - }); + it("should return undefined and not fetch when wallet is not managed", async () => { + const apiKeyService = mock<ApiKeyHttpService>({ + getApiKeys: jest.fn() + }); + const { result } = setupApiKeysQuery({ + user: mockUser, + wallet: { ...mockWallet, isManaged: false }, + services: { + apiKey: () => apiKeyService + } + }); + + expect(result.current.query.fetchStatus).toBe("idle"); + expect(apiKeyService.getApiKeys).not.toHaveBeenCalled(); + expect(result.current.query.data).toBeUndefined(); + });
103-138: Also assert cache is updated by create mutationThe hook updates the query cache on success. Strengthen the test by asserting the cache now holds the created key at the expected key.
Apply this diff:
- const { result } = setupQuery( + const { result, queryClient } = setupQuery( () => { const dependencies: typeof USE_API_KEYS_DEPENDENCIES = { ...USE_API_KEYS_DEPENDENCIES, useUser: () => mockUser, useWallet: () => mockWallet }; return useCreateApiKey(dependencies); }, { services: { apiKey: () => apiKeyService } } ); @@ await waitFor(() => { expect(result.current.isSuccess).toBe(true); }); expect(apiKeyService.createApiKey).toHaveBeenCalledWith({ data: { name: "New Key" } }); + + const cached = queryClient.getQueryData<ApiKeyResponse[]>(["API_KEYS", mockUser.userId]); + expect(cached).toEqual([newApiKey]);
140-171: Pre-seed and assert deletion updates the cacheThe delete hook filters the cached list on success. Seed the cache with the key under test and assert it’s removed afterwards.
Apply this diff:
- const { result } = setupQuery( + const { result, queryClient } = setupQuery( () => { const dependencies: typeof USE_API_KEYS_DEPENDENCIES = { ...USE_API_KEYS_DEPENDENCIES, useUser: () => mockUser, useWallet: () => mockWallet }; return useDeleteApiKey("key-1", undefined, dependencies); }, { services: { apiKey: () => apiKeyService } } ); - act(() => { + // Seed cache with two keys, one will be deleted + const existing = [buildApiKey({ id: "key-1" }), buildApiKey({ id: "key-2" })]; + act(() => { + queryClient.setQueryData(["API_KEYS", mockUser.userId], existing); result.current.mutate(); }); @@ expect(apiKeyService.deleteApiKey).toHaveBeenCalledWith("key-1"); + + const cached = queryClient.getQueryData<ApiKeyResponse[]>(["API_KEYS", mockUser.userId]); + expect(cached?.map(k => k.id)).toEqual(["key-2"]);
214-221: Optional: Avoid exposing queryClient via result.currentYou’re pulling
queryClientviauseQueryClient()and returning it throughresult.current. IfsetupQueryalready returnsqueryClient, prefer destructuring it from the setup return value to reduce coupling to the hook’s return shape.Apply this diff to simplify:
- () => { - const queryClient = useQueryClient(); - return { - query: useUserApiKeys({}, dependencies), - dependencies, - queryClient - }; - }, + () => useUserApiKeys({}, dependencies),Then in the tests that need it:
- const { result } = setupApiKeysQuery({ /* ... */ }); + const { result, queryClient } = setupApiKeysQuery({ /* ... */ });
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx(1 hunks)apps/deploy-web/src/queries/useApiKeysQuery.ts(2 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/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/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/queries/useApiKeysQuery.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.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/queries/useApiKeysQuery.spec.tsxapps/deploy-web/src/queries/useApiKeysQuery.ts
🧠 Learnings (4)
📚 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/queries/useApiKeysQuery.spec.tsx
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test files
Applied to files:
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
📚 Learning: 2025-07-27T12:16:08.566Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-07-27T12:16:08.566Z
Learning: Applies to **/*.{ts,tsx} : Never use type any or cast to type any. Always define the proper TypeScript types.
Applied to files:
apps/deploy-web/src/queries/useApiKeysQuery.ts
🧬 Code Graph Analysis (2)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (6)
apps/deploy-web/tests/seeders/apiKey.ts (1)
buildApiKey(4-13)apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/tests/seeders/user.ts (1)
buildUser(7-25)apps/deploy-web/tests/seeders/wallet.ts (1)
buildWallet(7-25)packages/http-sdk/src/api-key/api-key-http.service.ts (1)
ApiKeyHttpService(4-16)apps/deploy-web/src/queries/useApiKeysQuery.ts (4)
USE_API_KEYS_DEPENDENCIES(10-13)useCreateApiKey(34-53)useDeleteApiKey(55-69)useUserApiKeys(15-32)
apps/deploy-web/src/queries/useApiKeysQuery.ts (3)
apps/api/src/auth/http-schemas/api-key.schema.ts (1)
ApiKeyResponse(58-58)packages/http-sdk/src/api-key/api-key-http.types.ts (1)
ApiKeyResponse(32-32)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
useServices(27-29)
⏰ 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). (6)
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (6)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (3)
53-72: LGTM: Positive-path fetch is correctly assertedMocks, DI wiring, and expectations look correct, and the test validates both service invocation and returned data.
74-100: LGTM: Correctly validates the query key in the cacheThis addresses the earlier review note about actually verifying the cache key. Nice job asserting a single match with the expected key.
173-204: LGTM: onSuccess callback is properly invoked after deletionClear assertion of the callback behavior with DI and mutation.
apps/deploy-web/src/queries/useApiKeysQuery.ts (3)
10-13: LGTM: DI boundary extracted for hooksExported
USE_API_KEYS_DEPENDENCIEScleanly enables dependency injection in tests and consumers.
15-31: Good: typed UseQueryOptions and managed-wallet gating added
- Replaced
anywithApiKeyResponse[]in UseQueryOptions generics.enablednow also requiresisManagedand not trialing. Matches product intent.
55-69: No guard needed for useDeleteApiKey – call sites already ensure a valid user and key
All instances ofuseDeleteApiKeyare invoked only after both the user and the API‐key ID are defined (the hook re-runs with the actual ID on state update, and the pages themselves require a logged-in user). The mutation’sonSuccesswill therefore never run against the["API_KEYS", ""]cache. No additional guard or early return is required here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (1)
257-263: Remove unsafe type assertion in setup (return possibly undefined user without casting).Casting
input?.usertoCustomUserProfileis unsafe and unnecessary. The dependency contract supports an undefined user, and your gating tests rely on that. Returning the value as-is improves type safety and aligns with the DI behavior in production.Apply this diff:
- const dependencies: typeof USE_API_KEYS_DEPENDENCIES = { + const dependencies: typeof USE_API_KEYS_DEPENDENCIES = { ...USE_API_KEYS_DEPENDENCIES, - useUser: () => input?.user as CustomUserProfile, + useUser: () => input?.user, useWallet: () => input?.wallet || mockWallet };
🧹 Nitpick comments (1)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (1)
34-43: Strengthen disabled-state assertion (explicitly verify fetchStatus is "idle").You already assert the service wasn’t invoked and the query didn’t transition to loading/success. Add an explicit check for the disabled fetch status for completeness and consistency with the other gating tests.
Apply this diff:
// Verify the service was not called since the query is disabled expect(apiKeyService.getApiKeys).not.toHaveBeenCalled(); + // Disabled queries remain idle + expect(result.current.query.fetchStatus).toBe("idle");
📜 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 settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx(1 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/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/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/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/queries/useApiKeysQuery.spec.tsx
🧠 Learnings (4)
📚 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/queries/useApiKeysQuery.spec.tsx
📚 Learning: 2025-07-21T08:24:24.269Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/no-jest-mock.mdc:0-0
Timestamp: 2025-07-21T08:24:24.269Z
Learning: Applies to **/*.spec.{ts,tsx} : Don't use `jest.mock()` to mock dependencies in test files. Instead, use `jest-mock-extended` to create mocks and pass mocks as dependencies to the service under test.
Applied to files:
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : `setup` function must be at the bottom of the root `describe` block in test files
Applied to files:
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
📚 Learning: 2025-07-21T08:25:07.474Z
Learnt from: CR
PR: akash-network/console#0
File: .cursor/rules/setup-instead-of-before-each.mdc:0-0
Timestamp: 2025-07-21T08:25:07.474Z
Learning: Applies to **/*.spec.{ts,tsx} : Use `setup` function instead of `beforeEach` in test files
Applied to files:
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx
🧬 Code Graph Analysis (1)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (6)
apps/deploy-web/tests/seeders/apiKey.ts (1)
buildApiKey(4-13)apps/deploy-web/src/types/user.ts (1)
CustomUserProfile(19-19)apps/deploy-web/tests/seeders/user.ts (1)
buildUser(7-25)apps/deploy-web/tests/seeders/wallet.ts (1)
buildWallet(7-25)packages/http-sdk/src/api-key/api-key-http.service.ts (1)
ApiKeyHttpService(4-16)apps/deploy-web/src/queries/useApiKeysQuery.ts (4)
USE_API_KEYS_DEPENDENCIES(10-13)useCreateApiKey(34-53)useDeleteApiKey(55-69)useUserApiKeys(15-32)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Validate local packages
🔇 Additional comments (4)
apps/deploy-web/src/queries/useApiKeysQuery.spec.tsx (4)
45-60: LGTM: Gating correctly disables fetching for trialing and non-managed wallets.Both tests clearly assert idle fetch status, no service invocation, and undefined data. This validates the
enabledlogic againstisTrialingandisManaged.Also applies to: 62-77
79-98: LGTM: Happy path fetch asserts success and data shape.Verifies the service is called and the returned data is surfaced correctly.
100-128: Resolved: Test now verifies the query key via the QueryClient cache.This addresses prior feedback by asserting the exact query key used to cache the result. The approach of injecting a dedicated QueryClient is solid.
131-172: LGTM: Mutations update the cache correctly (create, delete) and invoke callbacks.
- Create: onSuccess appends to cache; assertion checks the new key present.
- Delete: pre-seeded cache removes target entry;
onSuccesscallback validated.Well-isolated dependencies via DI and per-test QueryClient instances keep tests hermetic.
Also applies to: 175-221, 223-254
* feat(auth): enhance API key management with new hooks and tests * fix(auth): enhance query caching and improve type safety in useUserApiKeys * fix(auth): update useUserApiKeys tests to verify disabled state when user is not provided * fix(auth): update useApiKeysQuery tests
2 participants
#1798
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Refactor