feat: can manually create a token #1985
Conversation
WalkthroughAdds JWT-based token flow gated by a feature flag to CreateCertificateButton, introduces a new useJwt hook with tests, updates analytics events and feature flags, exports SettingsProvider context type, and extends wallet types to store tokens. Tests updated/added to cover token lifecycle, UI states, and expiration handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as CreateCertificateButton
participant Flag as useFlag("jwt_instead_of_cert")
participant JWT as useJwt
participant Cert as useCertificate
participant Wallet as Storage/Wallet
participant JwtSvc as JwtToken Service
participant Analytics
User->>UI: Click button
UI->>Flag: Check flag
alt JWT enabled
UI->>JWT: createToken()
JWT->>Wallet: Load active wallet
JWT->>JwtSvc: Build + sign JWT
JwtSvc-->>JWT: token
JWT->>Wallet: Persist token
JWT->>Analytics: track("create_jwt")
JWT-->>UI: isCreatingToken=false, localToken updated
UI-->>User: Token created/regenerated
else Certificate flow
UI->>Cert: createCertificate()
Cert-->>UI: isCreatingCert=false, localCert updated
UI-->>User: Certificate created/regenerated
end
note over UI,JWT: Warning text reflects expired/missing token or cert
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jzsfkzm
force-pushed
the
features/1946-jwt-button
branch
from
d85121b to
3706ff7
Compare
| dependencies?: typeof DEPENDENCIES; | ||
| }; | ||
|
|
||
| export const JwtProvider: React.FC<Props> = ({ children, dependencies: d = DEPENDENCIES }) => { |
There was a problem hiding this comment.
thought: I assume it's possible to solve this without adding one more Provider component. We can just write a hook which stores the state in jotai store and updates atoms
There was a problem hiding this comment.
thought: I assume it's possible to solve this without adding one more Provider component. We can just write a hook which stores the state in jotai store and updates atoms
Moved it to a hook. Logic now is it's checking a flag whether to render a cert button or a token button, not sure if that's ok or if it should work some other way. Let me know if you see anything please.
jzsfkzm
force-pushed
the
features/1946-jwt-button
branch
7 times, most recently
from
7b23193 to
134abb2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx (1)
42-59: Consider explicit branching for JWT vs certificate logic.The current implementation allows both JWT and certificate conditions to be evaluated even when
isJwtEnabledis true. While the early returns prevent both warnings from showing, explicitif-elsebranching would make the mutual exclusivity clearer.const warningText = useMemo(() => { if (isJwtEnabled) { if (isLocalTokenExpired) return "Your token has expired. Please create a new one."; if (!localToken) return "You need to create a token to view deployment details."; + return undefined; - } - + } else { if (isLocalCertExpired) return "Your certificate has expired. Please create a new one."; if (!localCert) return "You need to create a certificate to view deployment details."; - return undefined; + } }, [isJwtEnabled, isLocalCertExpired, isLocalTokenExpired, localCert, localToken]);Apply similar pattern to
buttonText:const buttonText = useMemo(() => { if (isJwtEnabled) { return isLocalTokenExpired ? "Regenerate Token" : "Create Token"; + } else { + return isLocalCertExpired ? "Regenerate Certificate" : "Create Certificate"; } - - return isLocalCertExpired ? "Regenerate Certificate" : "Create Certificate"; }, [isJwtEnabled, isLocalCertExpired, isLocalTokenExpired]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx(5 hunks)apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx(2 hunks)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx(1 hunks)apps/deploy-web/src/hooks/useJwt.spec.tsx(1 hunks)apps/deploy-web/src/hooks/useJwt.ts(1 hunks)apps/deploy-web/src/services/analytics/analytics.service.ts(1 hunks)apps/deploy-web/src/types/feature-flags.ts(1 hunks)apps/deploy-web/src/utils/TransactionMessageData.ts(2 hunks)apps/deploy-web/src/utils/walletUtils.ts(1 hunks)
🧰 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/src/utils/TransactionMessageData.tsapps/deploy-web/src/utils/walletUtils.tsapps/deploy-web/src/types/feature-flags.tsapps/deploy-web/src/hooks/useJwt.tsapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsxapps/deploy-web/src/hooks/useJwt.spec.tsxapps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsxapps/deploy-web/src/services/analytics/analytics.service.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/utils/TransactionMessageData.tsapps/deploy-web/src/utils/walletUtils.tsapps/deploy-web/src/types/feature-flags.tsapps/deploy-web/src/hooks/useJwt.tsapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsxapps/deploy-web/src/hooks/useJwt.spec.tsxapps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsxapps/deploy-web/src/services/analytics/analytics.service.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/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/hooks/useJwt.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/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/hooks/useJwt.spec.tsx
🧬 Code graph analysis (5)
apps/deploy-web/src/hooks/useJwt.ts (4)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx (1)
DEPENDENCIES(12-20)packages/jwt/src/jwt-token.ts (1)
createToken(43-67)apps/deploy-web/src/utils/TransactionMessageData.ts (1)
TransactionMessageData(21-309)packages/http-sdk/src/tx-http/tx-http.service.ts (1)
signAndBroadcastTx(22-33)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (1)
apps/deploy-web/src/hooks/useJwt.ts (1)
LocalToken(12-15)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx (3)
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx (1)
DEPENDENCIES(70-110)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (3)
DEPENDENCIES(63-68)isLocalCertExpired(367-369)localCert(363-365)packages/jwt/src/jwt-token.ts (1)
createToken(43-67)
apps/deploy-web/src/hooks/useJwt.spec.tsx (3)
apps/deploy-web/src/hooks/useJwt.ts (1)
useJwt(27-127)apps/deploy-web/src/utils/walletUtils.ts (1)
LocalWallet(27-27)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
AppDIContainer(18-18)
apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (2)
apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (1)
ContextType(42-59)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (1)
ContextType(38-59)
🔇 Additional comments (12)
apps/deploy-web/src/services/analytics/analytics.service.ts (1)
40-40: LGTM!The new
"create_jwt"event type follows the existing naming convention and integrates cleanly with the JWT creation workflow introduced in this PR.apps/deploy-web/src/utils/walletUtils.ts (1)
11-11: LGTM!The optional
tokenfield appropriately extends the wallet interface to support JWT storage without introducing breaking changes.apps/deploy-web/src/types/feature-flags.ts (1)
9-10: LGTM!The new feature flag
"jwt_instead_of_cert"clearly indicates its purpose and enables controlled rollout of the JWT-based authentication flow.apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
32-32: LGTM!Exporting
ContextTypefollows the established pattern in other providers (e.g.,WalletProvider,CertificateProvider) and enables proper type checking for external consumers.apps/deploy-web/src/utils/TransactionMessageData.ts (1)
72-82: LGTM!The
getCreateJwtMsghelper follows the established pattern fromgetCreateCertificateMsgand correctly base64-encodes the JWT token string for transmission.apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (2)
2-2: LGTM!The new test cases comprehensively cover the JWT flow:
- Token button rendering when feature flag is enabled
- Token creation callback invocation
- Token expiration warning display
All tests follow the existing patterns and coding guidelines.
Also applies to: 15-19, 31-39, 53-63
66-89: LGTM!The test setup properly extends the dependency injection pattern to include JWT-related hooks (
useJwt,useFlag) and parameters, maintaining consistency with the existing test infrastructure.Also applies to: 118-131
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx (3)
9-10: LGTM!The dependency injection pattern is properly extended to include
useJwtanduseFlaghooks, maintaining consistency with the existing architecture.Also applies to: 17-20, 29-32
38-41: LGTM!The
_createTokencallback correctly mirrors the pattern established by_createCertificate, ensuring consistent behavior across both flows.
66-84: Verify className inconsistency is intentional.Line 68 includes
"ml-2"in the JWT button's className, while line 77 (certificate button) does not. Ensure this difference is intentional, as it may cause layout inconsistencies between the two flows.apps/deploy-web/src/hooks/useJwt.spec.tsx (2)
1-279: LGTM!Comprehensive test suite covering all critical paths:
- Initial states and loading scenarios
- Token expiration logic
- Token creation (success and failure paths)
- Edge cases (multiple wallets, decoding errors, missing tokens)
- Error handling and transaction broadcast failures
The tests follow all coding guidelines and demonstrate thorough validation of the JWT workflow.
280-368: LGTM!The test setup helper follows coding guidelines perfectly:
- Single parameter with inline type definition
- No shared state
- No return type annotation
- Positioned at the bottom of the describe block
- Uses
jest-mock-extendedfor dependency mocking- Comprehensive mock coverage of all dependencies
The mocking strategy effectively isolates the hook for unit testing.
| const jwtToken = new d.JwtToken({} as any); | ||
| return jwtToken.decodeToken(localToken.token); |
There was a problem hiding this comment.
Stop casting to any when decoding the token
Line 51 constructs new d.JwtToken({} as any) solely to call decodeToken, which violates our “no any” TypeScript rule and sidesteps the dependencies the JwtToken ctor expects (wallet + signer). That leaves the instance in an invalid state and will break as soon as JwtToken touches those fields. Please switch to a decode helper that doesn’t require instantiation—e.g., expose or import a static decodeToken—instead of faking the ctor. As per coding guidelines.
| const jwtToken = new d.JwtToken({} as any); | ||
| return jwtToken.decodeToken(localToken.token); | ||
| } catch (error) { | ||
| return null; |
There was a problem hiding this comment.
should this be reported to sentry? should any error be shown to a user?
| setIsCreatingToken(false); | ||
|
|
||
| throw error; | ||
| } |
There was a problem hiding this comment.
non-blocking. finally could be used to setIsCreatingToken(false); for all cases
apps/deploy-web/src/hooks/useJwt.ts
Outdated
|
|
||
| const token = await jwtToken.createToken({ | ||
| version: "v1", | ||
| iss: "https://example.com", |
There was a problem hiding this comment.
this seems to be failing the validation
apps/deploy-web/src/hooks/useJwt.ts
Outdated
| const message = TransactionMessageData.getCreateJwtMsg(address, token); | ||
| const response = await signAndBroadcastTx([message]); |
There was a problem hiding this comment.
FYI: token is created offchain, no need for trx
There was a problem hiding this comment.
I'm sorry, I don't understand this sentence, probably it's the lack of my knowledge. What do you mean? No signAndBroadcastTx needed here?
There was a problem hiding this comment.
No worries
"offchain" means outside of blockchain and transaction is not needed. The whole point of adding JWT is to get rid of blockchain dependency, and trx -> shorthand for transaction (In this context blockchain transaction).
jzsfkzm
force-pushed
the
features/1946-jwt-button
branch
from
134abb2 to
e9f77f4
Compare
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/utils/walletUtils.ts (1)
7-13: Persisting JWT in localStorage expands XSS blast radiusIn apps/deploy-web/src/utils/walletUtils.ts, BaseLocalWallet.token is saved and loaded via localStorage (getStorageWallets/updateStorageWallets around lines 92–111); move tokens to a more secure store (httpOnly cookie, sessionStorage for short-lived tokens, or in-memory) and ensure all tokens are purged on logout or wallet deletion.
♻️ Duplicate comments (2)
apps/deploy-web/src/hooks/useJwt.ts (2)
47-56: Stop instantiating JwtToken withas anyjust to decodeDon’t fake the ctor to call decode. Decode without constructing or inject a pure decode dependency.
As per coding guidelines.
Apply this diff to use a safe payload decoder:
- const parsedLocalToken = useMemo(() => { - if (!localToken) return null; - - try { - const jwtToken = new d.JwtToken({} as any); - return jwtToken.decodeToken(localToken.token); - } catch (error) { - return null; - } - }, [localToken]); + const parsedLocalToken = useMemo(() => { + if (!localToken) return null; + return decodeJwtPayload(localToken.token); + }, [localToken]);Add this helper (outside the selected range):
function decodeJwtPayload(token: string): JwtTokenPayload | null { try { const parts = token.split("."); if (parts.length < 2) return null; const payload = parts[1].replace(/-/g, "+").replace(/_/g, "/"); const json = typeof atob === "function" ? atob(payload) : Buffer.from(payload, "base64").toString("utf8"); return JSON.parse(json) as JwtTokenPayload; } catch { return null; } }
70-108: Spinner can get stuck true on token creation errorsErrors thrown before the current try/finally leave isCreatingToken=true. Wrap the whole flow in try/finally.
const createToken = useCallback(async () => { setIsCreatingToken(true); - - const { pubkey } = await getAccount(); - - const jwtToken = new d.JwtToken({ - signArbitrary, - address, - pubkey - }); - - const token = await jwtToken.createToken({ - version: "v1", - iss: address, - exp: Math.floor(Date.now() / 1000) + 3600, - iat: Math.floor(Date.now() / 1000) - }); - - try { - const message = TransactionMessageData.getCreateJwtMsg(address, token); - const response = await signAndBroadcastTx([message]); - if (response) { - d.updateWallet(address, wallet => { - return { - ...wallet, - token - }; - }); - loadLocalToken(); - - analyticsService.track("create_jwt", { - category: "certificates", - label: "Created jwt" - }); - } - } finally { - setIsCreatingToken(false); - } + try { + const { pubkey } = await getAccount(); + const jwtToken = new d.JwtToken({ signArbitrary, address, pubkey }); + const token = await jwtToken.createToken({ + version: "v1", + iss: address, + exp: Math.floor(Date.now() / 1000) + 3600, + iat: Math.floor(Date.now() / 1000) + }); + const message = TransactionMessageData.getCreateJwtMsg(address, token); + const response = await signAndBroadcastTx([message]); + if (response) { + d.updateWallet(address, wallet => ({ ...wallet, token })); + loadLocalToken(); + analyticsService.track("create_jwt", { category: "certificates", label: "Created jwt" }); + } + } finally { + setIsCreatingToken(false); + } }, [getAccount, signArbitrary, address, signAndBroadcastTx, d, loadLocalToken, analyticsService]);
🧹 Nitpick comments (3)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (2)
9-13: Use queryBy in expectations per test guidelines*Switch getByRole to queryByRole in assertions.
As per coding guidelines.
- expect(screen.getByRole("button", { name: /create certificate/i })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /create certificate/i })).toBeInTheDocument();
15-19: Use queryBy in expectations per test guidelines*Switch getByRole to queryByRole in assertions.
As per coding guidelines.
- expect(screen.getByRole("button", { name: /create token/i })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /create token/i })).toBeInTheDocument();apps/deploy-web/src/hooks/useJwt.ts (1)
58-69: loadLocalToken doesn’t need to be asyncIt’s synchronous; drop async to avoid unnecessary Promise scheduling. Optionally use a for‑of loop instead of find+side effect.
- const loadLocalToken = useCallback(async () => { + const loadLocalToken = useCallback(() => { const wallets = d.getStorageWallets(); - wallets.find(wallet => { + wallets.find(wallet => { const token: LocalToken | null = wallet.token ? { token: wallet.token, address: wallet.address } : null; if (wallet.address === address) { setLocalToken(token); return true; } }); }, [address]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx(5 hunks)apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx(2 hunks)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx(1 hunks)apps/deploy-web/src/hooks/useJwt.spec.tsx(1 hunks)apps/deploy-web/src/hooks/useJwt.ts(1 hunks)apps/deploy-web/src/services/analytics/analytics.service.ts(1 hunks)apps/deploy-web/src/types/feature-flags.ts(1 hunks)apps/deploy-web/src/utils/TransactionMessageData.ts(3 hunks)apps/deploy-web/src/utils/walletUtils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/utils/TransactionMessageData.ts
- apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx
- apps/deploy-web/src/services/analytics/analytics.service.ts
- apps/deploy-web/src/types/feature-flags.ts
🧰 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/src/utils/walletUtils.tsapps/deploy-web/src/hooks/useJwt.tsapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/hooks/useJwt.spec.tsxapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.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/utils/walletUtils.tsapps/deploy-web/src/hooks/useJwt.tsapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/hooks/useJwt.spec.tsxapps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx
**/*.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/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/hooks/useJwt.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/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/hooks/useJwt.spec.tsx
🧬 Code graph analysis (4)
apps/deploy-web/src/hooks/useJwt.ts (3)
packages/jwt/src/jwt-token.ts (1)
createToken(43-67)apps/deploy-web/src/utils/TransactionMessageData.ts (1)
TransactionMessageData(22-310)packages/http-sdk/src/tx-http/tx-http.service.ts (1)
signAndBroadcastTx(22-33)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (2)
apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (1)
LocalCert(13-17)apps/deploy-web/src/hooks/useJwt.ts (1)
LocalToken(12-15)
apps/deploy-web/src/hooks/useJwt.spec.tsx (3)
apps/deploy-web/src/hooks/useJwt.ts (1)
useJwt(27-123)apps/deploy-web/src/utils/walletUtils.ts (1)
LocalWallet(27-27)apps/deploy-web/src/context/ServicesProvider/ServicesProvider.tsx (1)
AppDIContainer(19-19)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx (3)
apps/deploy-web/src/components/new-deployment/CreateLease/CreateLease.tsx (1)
DEPENDENCIES(70-110)apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (3)
DEPENDENCIES(63-68)isLocalCertExpired(369-371)localCert(365-367)packages/jwt/src/jwt-token.ts (1)
createToken(43-67)
⏰ 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/hooks/useJwt.ts (1)
88-95: Confirm if broadcasting a tx is required for JWT creationA prior review noted tokens are created off‑chain; if no on‑chain registration is needed, consider removing the tx path to reduce latency and failure modes.
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx (1)
29-33: JWT gating and hook wiring look goodFlag check + useJwt integration and state wiring are coherent.
| const mockUseSettings = mock<SettingsProviderContextType>({ | ||
| settings: { | ||
| apiEndpoint: "https://api.example.com", | ||
| rpcEndpoint: "https://rpc.example.com", | ||
| isCustomNode: false, | ||
| nodes: [], | ||
| selectedNode: null, | ||
| customNode: null, | ||
| isBlockchainDown: false | ||
| } | ||
| }); |
There was a problem hiding this comment.
Mock useSettings misses isSettingsInit; tests may not reflect intended state
setup(input).isSettingsInit isn’t passed into the mock, so token loading paths won’t run when expected.
Apply this diff:
const mockUseSettings = mock<SettingsProviderContextType>({
settings: {
apiEndpoint: "https://api.example.com",
rpcEndpoint: "https://rpc.example.com",
isCustomNode: false,
nodes: [],
selectedNode: null,
customNode: null,
isBlockchainDown: false
- }
+ },
+ isSettingsInit: input.isSettingsInit ?? true
});📝 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.
| const mockUseSettings = mock<SettingsProviderContextType>({ | |
| settings: { | |
| apiEndpoint: "https://api.example.com", | |
| rpcEndpoint: "https://rpc.example.com", | |
| isCustomNode: false, | |
| nodes: [], | |
| selectedNode: null, | |
| customNode: null, | |
| isBlockchainDown: false | |
| } | |
| }); | |
| const mockUseSettings = mock<SettingsProviderContextType>({ | |
| settings: { | |
| apiEndpoint: "https://api.example.com", | |
| rpcEndpoint: "https://rpc.example.com", | |
| isCustomNode: false, | |
| nodes: [], | |
| selectedNode: null, | |
| customNode: null, | |
| isBlockchainDown: false | |
| }, | |
| isSettingsInit: input.isSettingsInit ?? true | |
| }); |
🤖 Prompt for AI Agents
In apps/deploy-web/src/hooks/useJwt.spec.tsx around lines 324 to 334, the
mockUseSettings object lacks the isSettingsInit property so tests that rely on
setup(input).isSettingsInit won't drive the token-loading paths; update the mock
to include isSettingsInit: setup(input).isSettingsInit (or the corresponding
value from the test setup variable) alongside settings so the mock reflects the
intended initialization state and triggers the correct code paths.
| useSettings: () => mockUseSettings, | ||
| getStorageWallets: mockGetStorageWallets, | ||
| updateWallet: mockUpdateWallet, | ||
| JwtToken: MockJwtToken as any |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid as any in tests
Replace as any with a safe cast to the JwtToken constructor type.
As per coding guidelines.
- JwtToken: MockJwtToken as any
+ JwtToken: MockJwtToken as unknown as typeof JwtTokenAdd this import at the top of the file:
import { JwtToken } from "@akashnetwork/jwt";🤖 Prompt for AI Agents
In apps/deploy-web/src/hooks/useJwt.spec.tsx around line 352, the test uses
"JwtToken: MockJwtToken as any" which violates the guideline to avoid `as any`;
import the JwtToken type from "@akashnetwork/jwt" at the top of the file and
replace the `as any` cast with a safe cast to the JwtToken constructor type
(e.g., cast MockJwtToken to the JwtToken constructor/interface) so the mock
matches the expected JwtToken shape.
|
hey @jzsfkzm don't spend time on this PR, it's on me from now |
jzsfkzm
force-pushed
the
features/1946-jwt-button
branch
from
e9f77f4 to
d516089
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (1)
15-19: Consider usingqueryByRolefor the button expectation.The test uses
getByRolein the expectation on line 17. According to the coding guidelines for.spec.tsxfiles, preferqueryBymethods in test expectations for better error messages and explicit null checks.As per coding guidelines.
Apply this diff:
it("renders create token button", () => { setup({ isJwtEnabled: true }); - expect(screen.getByRole("button", { name: /create token/i })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /create token/i })).toBeInTheDocument(); expect(screen.queryByRole("alert")).toHaveTextContent(/You need to create a token to view deployment details./); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx(5 hunks)apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx(2 hunks)apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx(1 hunks)apps/deploy-web/src/hooks/useJwt.spec.tsx(1 hunks)apps/deploy-web/src/hooks/useJwt.ts(1 hunks)apps/deploy-web/src/services/analytics/analytics.service.ts(1 hunks)apps/deploy-web/src/types/feature-flags.ts(1 hunks)apps/deploy-web/src/utils/walletUtils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/deploy-web/src/utils/walletUtils.ts
- apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.tsx
- apps/deploy-web/src/hooks/useJwt.spec.tsx
- apps/deploy-web/src/hooks/useJwt.ts
🧰 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/components/deployments/CreateCertificateButton/CreateCertificateButton.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/components/deployments/CreateCertificateButton/CreateCertificateButton.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/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsxapps/deploy-web/src/types/feature-flags.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/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsxapps/deploy-web/src/services/analytics/analytics.service.tsapps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsxapps/deploy-web/src/types/feature-flags.ts
🧬 Code graph analysis (2)
apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (2)
apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (1)
LocalCert(13-17)apps/deploy-web/src/hooks/useJwt.ts (1)
LocalToken(11-14)
apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (2)
apps/deploy-web/src/context/CertificateProvider/CertificateProviderContext.tsx (1)
ContextType(38-59)apps/deploy-web/src/context/WalletProvider/WalletProvider.tsx (1)
ContextType(42-59)
⏰ 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/src/services/analytics/analytics.service.ts (1)
40-40: LGTM!The new
"create_jwt"event follows the existing naming convention and integrates cleanly with the analytics tracking system.apps/deploy-web/src/types/feature-flags.ts (1)
9-10: LGTM!The new
"jwt_instead_of_cert"feature flag is clearly named and properly integrated into the union type.apps/deploy-web/src/context/SettingsProvider/SettingsProviderContext.tsx (1)
32-39: LGTM!Exporting
ContextTypefollows the established pattern used in other provider contexts (e.g.,CertificateProviderContext,WalletProvider) and enables type-safe external usage without modifying the type structure.apps/deploy-web/src/components/deployments/CreateCertificateButton/CreateCertificateButton.spec.tsx (4)
31-39: LGTM!The test properly verifies that
createTokenis invoked when the token button is clicked, providing good coverage for the JWT flow.
53-63: LGTM!The test correctly validates the token expiration warning message, mirroring the certificate expiration test pattern.
65-89: LGTM!The extended
setupfunction follows coding guidelines:
- Single parameter with inline type definition
- No explicit return type
- No shared state
- Positioned at the bottom of the describe block
118-131: LGTM!The mock implementations for
useJwtanduseFlagproperly support the JWT/token test scenarios without using module-leveljest.mock(), adhering to the coding guidelines.
|
@jzsfkzm thanks for your work on this PR, I'll move them into my local branch. Will close this in favor of a new one |
refs #1946
Summary by CodeRabbit
New Features
Chores
Tests