fix: navigate home #813
fix: navigate home #813
Conversation
WalkthroughThis update introduces document registration state tracking across the app. It extends document metadata with an Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Screen
participant DataProvider
participant Validator
User->>Screen: Press Cancel/Exit
Screen->>Validator: hasAnyValidRegisteredDocument()
Validator-->>Screen: true/false
alt Registered document exists
Screen->>Screen: Navigate to "Home"
else No registered document
Screen->>Screen: Navigate to "Launch"
end
sequenceDiagram
participant AppStart
participant SplashScreen
participant DataProvider
participant Validator
AppStart->>SplashScreen: Mount
SplashScreen->>DataProvider: checkIfAnyDocumentsNeedMigration()
DataProvider-->>SplashScreen: true/false
alt Migration needed
SplashScreen->>Validator: checkAndUpdateRegistrationStates()
end
SplashScreen->>Validator: hasAnyValidRegisteredDocument()
Validator-->>SplashScreen: true/false
alt Registered document exists
SplashScreen->>SplashScreen: Navigate to "Home"
else
SplashScreen->>SplashScreen: Navigate to "Launch"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
app/src/screens/recovery/PassportDataNotFoundScreen.tsx (2)
17-18: Duplicate optimization opportunity: Consider consolidating navigation hooksSame performance optimization opportunity as in UnsupportedPassportScreen. The dual navigation hook pattern is repeated across multiple screens.
Consider creating a custom hook that handles conditional navigation to reduce code duplication across screens:
const useConditionalNavigation = () => { const navigation = useNavigation(); const navigateBasedOnDocuments = useCallback(async () => { try { const hasValidDocument = await hasAnyValidRegisteredDocument(); const targetScreen = hasValidDocument ? 'Home' : 'Launch'; navigation.navigate(targetScreen); // Add haptic feedback here } catch (error) { console.error('Error checking document validation:', error); navigation.navigate('Launch'); } }, [navigation]); return navigateBasedOnDocuments; };
20-27: Critical: Missing error handling in async functionSame critical issue as UnsupportedPassportScreen - the async function needs error handling to prevent crashes.
Apply the same error handling pattern:
const onPress = async () => { + try { const hasValidDocument = await hasAnyValidRegisteredDocument(); if (hasValidDocument) { navigateToHome(); } else { navigateToLaunch(); } + } catch (error) { + console.error('Error checking document validation:', error); + navigateToLaunch(); + } };
🧹 Nitpick comments (6)
app/src/screens/passport/UnsupportedPassportScreen.tsx (1)
23-24: Consider consolidating navigation hooks for better performanceUsing two separate navigation hooks when only one will be called could be optimized. Consider using a single navigation hook and passing the route dynamically.
- const navigateToLaunch = useHapticNavigation('Launch'); - const navigateToHome = useHapticNavigation('Home'); + const navigate = useNavigation(); + const hapticFeedback = useCallback(() => { + // Add haptic feedback logic here + }, []);app/src/screens/passport/PassportNFCScanScreen.web.tsx (2)
28-35: Robust conditional navigation logicThe async function properly handles document validation before navigation. Good error handling pattern where async operations are contained within the function.
Consider adding error handling for the async validation call:
const onCancelPress = async () => { - const hasValidDocument = await hasAnyValidRegisteredDocument(); - if (hasValidDocument) { - navigateToHome(); - } else { - navigateToLaunch(); - } + try { + const hasValidDocument = await hasAnyValidRegisteredDocument(); + if (hasValidDocument) { + navigateToHome(); + } else { + navigateToLaunch(); + } + } catch (error) { + console.error('Error checking document registration:', error); + navigateToLaunch(); // Fallback to Launch screen + } };
18-18: Address TypeScript linting issuesThe static analysis correctly identifies TypeScript best practices violations.
Apply these fixes for better TypeScript compliance:
-interface PassportNFCScanScreenProps {} +type PassportNFCScanScreenProps = Record<string, never>; -const PassportNFCScanScreen: React.FC<PassportNFCScanScreenProps> = ({}) => { +const PassportNFCScanScreen: React.FC<PassportNFCScanScreenProps> = () => {Also applies to: 20-20
app/src/screens/aesop/PassportOnboardingScreen.tsx (1)
22-22: Fix TypeScript compliance issuesSame TypeScript improvements needed as in the web version.
-interface PassportOnboardingScreenProps {} +type PassportOnboardingScreenProps = Record<string, never>; -> = ({}) => { +> = () => {Also applies to: 26-26
app/src/utils/proving/provingMachine.ts (1)
242-253: Consider extracting common registration logicBoth registration marking blocks use identical patterns. Consider extracting to reduce duplication:
const markDocumentAsRegisteredSafely = async (context: string) => { try { await markCurrentDocumentAsRegistered(); console.log(`Document marked as registered (${context})`); } catch (error) { console.error(`Error marking document as registered (${context}):`, error); } }; // Then use: if (get().circuitType === 'register') { markDocumentAsRegisteredSafely('on completion'); } // And: markDocumentAsRegisteredSafely('already on-chain');Also applies to: 728-737
app/src/utils/proving/validateDocument.ts (1)
341-418: Consider performance optimization for large document setsThe function processes documents sequentially, which could be slow with many documents. Consider batch processing:
export async function checkAndUpdateRegistrationStates(): Promise<void> { const allDocuments = await getAllDocuments(); const documentIds = Object.keys(allDocuments); // Process in batches of 5 to avoid overwhelming the system const BATCH_SIZE = 5; for (let i = 0; i < documentIds.length; i += BATCH_SIZE) { const batch = documentIds.slice(i, i + BATCH_SIZE); await Promise.allSettled( batch.map(async (documentId) => { // ... existing document processing logic }) ); } console.log('Registration state check and update completed'); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/src/providers/passportDataProvider.tsx(6 hunks)app/src/screens/aesop/PassportOnboardingScreen.tsx(1 hunks)app/src/screens/misc/SplashScreen.tsx(2 hunks)app/src/screens/passport/PassportCameraScreen.tsx(2 hunks)app/src/screens/passport/PassportNFCScanScreen.tsx(2 hunks)app/src/screens/passport/PassportNFCScanScreen.web.tsx(1 hunks)app/src/screens/passport/UnsupportedPassportScreen.tsx(1 hunks)app/src/screens/recovery/PassportDataNotFoundScreen.tsx(1 hunks)app/src/utils/proving/provingMachine.ts(3 hunks)app/src/utils/proving/validateDocument.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/src/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit Configuration File
app/src/**/*.{ts,tsx,js,jsx}: Review React Native TypeScript code for:
- Component architecture and reusability
- State management patterns
- Performance optimizations
- TypeScript type safety
- React hooks usage and dependencies
- Navigation patterns
Files:
app/src/screens/recovery/PassportDataNotFoundScreen.tsxapp/src/screens/misc/SplashScreen.tsxapp/src/screens/passport/UnsupportedPassportScreen.tsxapp/src/screens/aesop/PassportOnboardingScreen.tsxapp/src/screens/passport/PassportNFCScanScreen.web.tsxapp/src/screens/passport/PassportNFCScanScreen.tsxapp/src/utils/proving/provingMachine.tsapp/src/screens/passport/PassportCameraScreen.tsxapp/src/utils/proving/validateDocument.tsapp/src/providers/passportDataProvider.tsx
🧠 Learnings (2)
app/src/screens/passport/UnsupportedPassportScreen.tsx (1)
Learnt from: transphorm
PR: #636
File: app/src/utils/cameraPermission.ts:19-21
Timestamp: 2025-07-12T22:00:02.041Z
Learning: The Android permission handling in app/src/utils/cameraPermission.ts is robust and properly implemented using PermissionsAndroid.request() and PermissionsAndroid.check(). Previous issues with Android permission assumptions have been resolved in the current implementation.
app/src/screens/passport/PassportCameraScreen.tsx (1)
Learnt from: transphorm
PR: #636
File: app/src/utils/cameraPermission.ts:19-21
Timestamp: 2025-07-12T22:00:02.041Z
Learning: The Android permission handling in app/src/utils/cameraPermission.ts is robust and properly implemented using PermissionsAndroid.request() and PermissionsAndroid.check(). Previous issues with Android permission assumptions have been resolved in the current implementation.
🪛 Biome (2.1.2)
app/src/screens/aesop/PassportOnboardingScreen.tsx
[error] 21-22: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 26-26: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
app/src/screens/passport/PassportNFCScanScreen.web.tsx
[error] 17-18: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 20-20: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: analyze-android
- GitHub Check: analyze-ios
🔇 Additional comments (23)
app/src/screens/passport/UnsupportedPassportScreen.tsx (1)
17-17: LGTM: Clean import of validation utilityThe import follows consistent patterns used across other screens in this PR.
app/src/screens/recovery/PassportDataNotFoundScreen.tsx (1)
12-12: LGTM: Consistent import patternThe import follows the same pattern established across other screens in this PR.
app/src/screens/misc/SplashScreen.tsx (3)
10-13: LGTM: Well-organized imports for migration functionalityThe imports are clearly grouped and support the new migration functionality effectively.
Also applies to: 17-20
44-55: Excellent migration implementation with proper conditional logicThe migration logic is well-implemented with:
- Conditional execution based on actual need
- Clear logging for debugging
- Proper integration with existing error handling
This approach prevents unnecessary work and provides good visibility into the migration process.
57-57: Good integration of document validation in navigation logicThe use of
hasAnyValidRegisteredDocument()here is consistent with other screens and properly handles the navigation decision after migration is complete.app/src/screens/passport/PassportCameraScreen.tsx (2)
26-26: LGTM: Consistent import pattern maintainedThe import follows the established pattern across all updated screens.
117-122: Good: Preserved cancel action tracking in navigation hooksThe navigation hooks maintain the
action: 'cancel'parameter, which is important for analytics and user behavior tracking.app/src/screens/passport/PassportNFCScanScreen.tsx (2)
49-49: LGTM: Final consistent import in the navigation update seriesThe import pattern is consistent across all five updated screens, which aids maintainability.
279-284: Good: Maintained cancel action tracking consistencyLike PassportCameraScreen, this preserves the important
action: 'cancel'parameter for proper analytics tracking.app/src/screens/passport/PassportNFCScanScreen.web.tsx (2)
16-16: Import organization looks goodThe import of
hasAnyValidRegisteredDocumentis properly placed and follows the existing import structure.
21-26: Well-structured navigation hooksGood separation of concerns by creating dedicated navigation hooks for different destinations. The action metadata properly tracks user intent.
app/src/screens/aesop/PassportOnboardingScreen.tsx (2)
20-20: Consistent import patternGood consistency with other screens in using the same validation utility.
28-41: Excellent pattern consistencyThe navigation logic follows the exact same pattern as other passport screens, which provides a consistent user experience across the app.
app/src/utils/proving/provingMachine.ts (3)
22-22: Proper import integrationThe import is correctly placed within the existing provider imports section.
241-253: Well-implemented async registration markingThe IIFE pattern properly handles the async operation without blocking the state machine flow. Error handling is appropriate - logging for debugging while allowing the app to continue functioning.
727-737: Consistent error handling patternGood consistency in using the same async IIFE pattern and error handling approach as the completed state handler.
app/src/utils/proving/validateDocument.ts (2)
333-336: Clean and focused utility functionSimple, well-named function with clear responsibility. Proper async/await usage and leverages existing catalog loading logic.
341-418: Comprehensive migration and validation logicExcellent error handling with detailed logging for debugging. The function properly handles invalid documents, missing data, and protocol store operations. Good use of analytics tracking for monitoring.
app/src/providers/passportDataProvider.tsx (5)
68-68: Well-designed interface extensionThe optional
isRegisteredproperty maintains backward compatibility while adding the needed functionality. Good use of optional property for gradual migration.
189-189: Proper default initializationCorrectly initializes new documents as not registered, which is the appropriate default state.
527-532: Clean interface additionsThe new method signatures are well-defined with clear parameter types and return promises appropriately.
692-722: Robust implementation of registration state managementAll three functions have proper error handling, logging, and follow the established patterns in the codebase. The
checkIfAnyDocumentsNeedMigrationfunction efficiently usessome()for early termination.
553-555: Proper context integrationThe new functions are correctly integrated into both the context default and the provider state, maintaining consistency with existing patterns.
Also applies to: 612-614
| const onCancelPress = async () => { | ||
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | ||
| if (hasValidDocument) { | ||
| navigateToHome(); | ||
| } else { | ||
| navigateToLaunch(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Critical: Add error handling to prevent crashes
The async function needs error handling like the other screens to prevent app crashes if document validation fails.
const onCancelPress = async () => {
+ try {
const hasValidDocument = await hasAnyValidRegisteredDocument();
if (hasValidDocument) {
navigateToHome();
} else {
navigateToLaunch();
}
+ } catch (error) {
+ console.error('Error checking document validation:', error);
+ navigateToLaunch();
+ }
};📝 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 onCancelPress = async () => { | |
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | |
| if (hasValidDocument) { | |
| navigateToHome(); | |
| } else { | |
| navigateToLaunch(); | |
| } | |
| }; | |
| const onCancelPress = async () => { | |
| try { | |
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | |
| if (hasValidDocument) { | |
| navigateToHome(); | |
| } else { | |
| navigateToLaunch(); | |
| } | |
| } catch (error) { | |
| console.error('Error checking document validation:', error); | |
| navigateToLaunch(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In app/src/screens/passport/PassportCameraScreen.tsx around lines 124 to 131,
the async function onCancelPress lacks error handling which can cause app
crashes if hasAnyValidRegisteredDocument fails. Wrap the await call and
subsequent navigation logic in a try-catch block. In the catch block, handle or
log the error appropriately to prevent the app from crashing.
| const onCancelPress = async () => { | ||
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | ||
| if (hasValidDocument) { | ||
| navigateToHome(); | ||
| } else { | ||
| navigateToLaunch(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Critical: Complete the error handling pattern across all screens
This is the final screen missing error handling. For consistency and robustness, apply the same error handling pattern used in the other screens.
const onCancelPress = async () => {
+ try {
const hasValidDocument = await hasAnyValidRegisteredDocument();
if (hasValidDocument) {
navigateToHome();
} else {
navigateToLaunch();
}
+ } catch (error) {
+ console.error('Error checking document validation:', error);
+ navigateToLaunch();
+ }
};Consider creating the shared custom hook I mentioned earlier to eliminate code duplication and ensure consistent error handling across all screens.
📝 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 onCancelPress = async () => { | |
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | |
| if (hasValidDocument) { | |
| navigateToHome(); | |
| } else { | |
| navigateToLaunch(); | |
| } | |
| }; | |
| const onCancelPress = async () => { | |
| try { | |
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | |
| if (hasValidDocument) { | |
| navigateToHome(); | |
| } else { | |
| navigateToLaunch(); | |
| } | |
| } catch (error) { | |
| console.error('Error checking document validation:', error); | |
| navigateToLaunch(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In app/src/screens/passport/PassportNFCScanScreen.tsx around lines 286 to 293,
the onCancelPress function lacks error handling, unlike other screens. Wrap the
async call to hasAnyValidRegisteredDocument and the subsequent navigation calls
in a try-catch block. In the catch block, handle or log the error appropriately
to maintain consistency and robustness. Additionally, consider refactoring this
logic into a shared custom hook to avoid duplication and ensure uniform error
handling across all screens.
| const onPress = async () => { | ||
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | ||
| if (hasValidDocument) { | ||
| navigateToHome(); | ||
| } else { | ||
| navigateToLaunch(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add error handling for async document validation
The async function lacks error handling, which could cause the app to crash if hasAnyValidRegisteredDocument() throws an error.
const onPress = async () => {
- const hasValidDocument = await hasAnyValidRegisteredDocument();
- if (hasValidDocument) {
- navigateToHome();
- } else {
- navigateToLaunch();
- }
+ try {
+ const hasValidDocument = await hasAnyValidRegisteredDocument();
+ if (hasValidDocument) {
+ navigateToHome();
+ } else {
+ navigateToLaunch();
+ }
+ } catch (error) {
+ console.error('Error checking document validation:', error);
+ // Fallback to Launch screen on error
+ navigateToLaunch();
+ }
};📝 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 onPress = async () => { | |
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | |
| if (hasValidDocument) { | |
| navigateToHome(); | |
| } else { | |
| navigateToLaunch(); | |
| } | |
| }; | |
| const onPress = async () => { | |
| try { | |
| const hasValidDocument = await hasAnyValidRegisteredDocument(); | |
| if (hasValidDocument) { | |
| navigateToHome(); | |
| } else { | |
| navigateToLaunch(); | |
| } | |
| } catch (error) { | |
| console.error('Error checking document validation:', error); | |
| // Fallback to Launch screen on error | |
| navigateToLaunch(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In app/src/screens/passport/UnsupportedPassportScreen.tsx around lines 26 to 33,
the async function onPress calls hasAnyValidRegisteredDocument() without error
handling, risking app crashes if the promise rejects. Wrap the await call in a
try-catch block to catch any errors, and handle them gracefully, such as logging
the error and providing fallback navigation or user feedback.
| action: 'cancel', | ||
| }); | ||
| const onCancelPress = async () => { | ||
| const hasValidDocument = await hasAnyValidRegisteredDocument(); |
There was a problem hiding this comment.
this is what you where mentioning during the meet, nice that you found a solution
Changes
isRegisteredto the metadata ofDocumentCatalogisRegisteredflag is done on the splashScreen during the first launch.Summary by CodeRabbit
New Features
Bug Fixes
Chores