devop: show onboard when not initialized#630
Conversation
WalkthroughThe changes update multiple account request methods across different providers (Bitcoin, Kadena, Polkadot, and Solana) by altering their signatures to be asynchronous and return a Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Caller
participant M as Request Method
participant K as KeyRing
participant O as Onboard Process
U->>M: Request account access
M->>K: Async check: isInitialized?
alt KeyRing not initialized
K-->>M: false
M->>M: Return custom error ("Enkrypt not initialized")
M->>O: Call throttled openOnboard()
M->>M: Call handleRemainingPromises()
else KeyRing is initialized
K-->>M: true
M->>M: Proceed with account access logic
end
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
💼 Build Files |
![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 (3)
packages/extension/src/providers/kadena/methods/kda_requestAccounts.ts (1)
35-35: Consider moving initialization check.The KeyRing initialization check happens before checking if account access is pending. While this works, it would be more consistent with the other providers if this check happened after the pending check and right before handleAccountAccess is called (around line 44).
packages/extension/src/providers/polkadot/methods/dot_accounts_get.ts (2)
21-99: Consider improving error message for better user guidance.While the implementation is correct, the error message "Enkrypt not initialized" could be more helpful by including instructions on what the user should do next.
- _res(getCustomError('Enkrypt not initialized')); + _res(getCustomError('Enkrypt not initialized. Please complete the setup process in the popup window.'));🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
39-45: Consider refactoring for better code organization.The current implementation has multiple nested functions and complex promise handling. Consider refactoring
handleRemainingPromisesand other nested functions into class methods for better maintainability and testability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/extension/src/providers/bitcoin/methods/btc_requestAccounts.ts(3 hunks)packages/extension/src/providers/kadena/methods/kda_requestAccounts.ts(2 hunks)packages/extension/src/providers/polkadot/methods/dot_accounts_get.ts(2 hunks)packages/extension/src/providers/solana/methods/sol_connect.ts(3 hunks)packages/extension/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: buildAll
- GitHub Check: test
🔇 Additional comments (17)
packages/extension/package.json (1)
3-3: Version bump looks appropriate for the feature changes.The increment from 2.4.2 to 2.4.3 is consistent with the scope of changes in this PR that adds onboarding functionality when the extension is not initialized.
packages/extension/src/providers/solana/methods/sol_connect.ts (4)
6-11: LGTM: Good implementation of throttled onboarding.The throttled onboarding implementation ensures that multiple rapid calls won't spam the user with multiple onboarding windows. The 10-second throttle is a reasonable choice.
16-21: LGTM: Function signature correctly updated to async.Converting the function to async is necessary for the KeyRing initialization check. Return type properly updated to Promise.
32-32: LGTM: KeyRing initialization check added.The asynchronous check for KeyRing initialization is crucial for the new feature.
46-50: LGTM: Proper handling for uninitialized KeyRing.This block correctly handles the case when KeyRing is not initialized by:
- Returning an appropriate error message
- Opening the onboarding UI (throttled)
- Handling any remaining promises
This provides a much better user experience than silently failing.
packages/extension/src/providers/bitcoin/methods/btc_requestAccounts.ts (4)
7-11: LGTM: Good implementation of throttled onboarding.The throttled onboarding implementation is consistent with other providers, limiting onboarding window opening to once every 10 seconds.
16-21: LGTM: Function signature correctly updated to async.Converting the function to async is necessary for the KeyRing initialization check. Return type properly updated to Promise.
32-32: LGTM: KeyRing initialization check added.The asynchronous check for KeyRing initialization is crucial for the new feature.
46-50: LGTM: Proper handling for uninitialized KeyRing.This block correctly handles the case when KeyRing is not initialized by:
- Returning an appropriate error message
- Opening the onboarding UI (throttled)
- Handling any remaining promises
This provides a much better user experience than silently failing.
packages/extension/src/providers/kadena/methods/kda_requestAccounts.ts (3)
16-20: LGTM: Good implementation of throttled onboarding.The throttled onboarding implementation is consistent with other providers, limiting onboarding window opening to once every 10 seconds.
27-32: LGTM: Function signature correctly updated to async.Converting the function to async is necessary for the KeyRing initialization check. Return type properly updated to Promise.
102-106: LGTM: Proper handling for uninitialized KeyRing.This block correctly handles the case when KeyRing is not initialized by:
- Returning an appropriate error message
- Opening the onboarding UI (throttled)
- Handling any remaining promises
This provides a much better user experience than silently failing.
packages/extension/src/providers/polkadot/methods/dot_accounts_get.ts (5)
12-13: Good addition of necessary imports.The imports for
openOnboardandthrottlefrom lodash properly support the new functionality for handling uninitialized wallet states.
16-16: Well-implemented throttling mechanism.Great use of throttle to prevent multiple onboarding windows from opening simultaneously. The 10-second delay is a reasonable choice to balance responsiveness with preventing UI spam.
21-26: Correct function signature update for async operation.The method signature has been properly updated to be asynchronous, allowing the use of await for the KeyRing initialization check. The return type is appropriately changed to
Promise<void>.
29-30: Good initialization check implementation.Adding this explicit initialization check before proceeding with account access is a sensible validation step that will help prevent errors when the wallet isn't ready.
67-71: Well-handled uninitialized state.The implementation correctly handles the case when KeyRing is not initialized by:
- Returning an appropriate error message
- Triggering the onboarding process with throttling
- Managing the promise queue properly
This will improve user experience by guiding users to initialize their wallet when needed.
2 participants
Summary by CodeRabbit
New Features
Refactor
Chores
Bug Fixes