expose useReadMRZ hook for DocumentCameraScreen#1188
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an SDK-driven MRZ read flow: new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant DC as DocumentCameraScreen
participant SDK as SDK (useReadMRZ)
participant EVT as Sdk Event Bus
participant APP as SelfClientProvider
participant NAV as Navigation
U->>DC: Start passport scan
DC->>SDK: onPassportRead(result | error)
alt MRZ success
SDK->>SDK: format & validate MRZ\nupdate MRZ NFC state
SDK-->>EVT: Emit DOCUMENT_MRZ_READ_SUCCESS
EVT-->>APP: Event received
APP->>NAV: navigate('DocumentNFCScan')
else MRZ failure
SDK-->>EVT: Emit DOCUMENT_MRZ_READ_FAILURE
EVT-->>APP: Event received
APP->>NAV: navigate('DocumentCameraTrouble')
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/providers/selfClientProvider.tsx(1 hunks)app/src/screens/document/DocumentCameraScreen.tsx(2 hunks)app/src/utils/utils.ts(0 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/processing/mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/types/events.ts(2 hunks)
💤 Files with no reviewable changes (1)
- app/src/utils/utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
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/providers/selfClientProvider.tsxapp/src/screens/document/DocumentCameraScreen.tsx
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/src/types/events.ts
🧠 Learnings (3)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.tspackages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md
Applied to files:
packages/mobile-sdk-alpha/src/types/events.ts
🪛 GitHub Actions: Mobile SDK CI
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
packages/mobile-sdk-alpha/src/processing/mrz.ts
[warning] 1-1: Code style issues found by Prettier. Run 'prettier --write' to fix.
🪛 GitHub Check: lint
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
[warning] 74-74:
Replace ⏎··········!checkScannedInfo(⏎············documentNumber,⏎············formattedDateOfBirth,⏎············formattedDateOfExpiry,⏎··········)⏎········ with !checkScannedInfo(documentNumber,·formattedDateOfBirth,·formattedDateOfExpiry)
[warning] 71-71:
Delete ⏎·········
[warning] 69-69:
Delete ⏎·········
[warning] 61-61:
Replace ⏎··········documentNumber,⏎··········dateOfBirth,⏎··········dateOfExpiry,⏎··········documentType,⏎··········issuingCountry,⏎······· with ·documentNumber,·dateOfBirth,·dateOfExpiry,·documentType,·issuingCountry
[warning] 27-27:
Insert ;
[warning] 23-23:
Replace ⏎····(Date.now()·-·scanStartTimeRef.current)·/⏎····1000⏎·· with (Date.now()·-·scanStartTimeRef.current)·/·1000
[failure] 5-5:
Run autofix to sort these imports!
packages/mobile-sdk-alpha/src/processing/mrz.ts
[warning] 356-356:
Replace ⏎··passportNumber:·string,⏎··dateOfBirth:·string,⏎··dateOfExpiry:·string,⏎ with passportNumber:·string,·dateOfBirth:·string,·dateOfExpiry:·string
[failure] 356-356:
Expected checkScannedInfo before formatDateToYYMMDD
⏰ 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-ios
- GitHub Check: analyze-android
| if (passportNumber.length > 9) { | ||
| return false; | ||
| } | ||
| if (dateOfBirth.length !== 6) { | ||
| return false; | ||
| } | ||
| if (dateOfExpiry.length !== 6) { | ||
| return false; | ||
| } | ||
| return true; |
There was a problem hiding this comment.
Prevent empty passport numbers from passing MRZ validation.
checkScannedInfo currently lets an empty passport number through because length > 9 is the only guard. That means we will emit DOCUMENT_MRZ_READ_SUCCESS and move the user into NFC with a blank document number, guaranteeing NFC/auth failures and a broken flow. Please ensure we reject zero-length passport numbers before declaring success.
- if (passportNumber.length > 9) {
+ if (passportNumber.length === 0 || passportNumber.length > 9) {
return false;
}📝 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.
| if (passportNumber.length > 9) { | |
| return false; | |
| } | |
| if (dateOfBirth.length !== 6) { | |
| return false; | |
| } | |
| if (dateOfExpiry.length !== 6) { | |
| return false; | |
| } | |
| return true; | |
| if (passportNumber.length === 0 || passportNumber.length > 9) { | |
| return false; | |
| } | |
| if (dateOfBirth.length !== 6) { | |
| return false; | |
| } | |
| if (dateOfExpiry.length !== 6) { | |
| return false; | |
| } | |
| return true; |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/processing/mrz.ts around lines 361 to 370, the
MRZ validation allows an empty passportNumber because it only checks for length
> 9; update the guard to reject zero-length passport numbers (e.g. return false
if passportNumber.length === 0 or if passportNumber.length < 1 ||
passportNumber.length > 9) before returning true so empty document numbers
cannot pass validation.
shazarre
changed the title
useReadMRZ hook for DocumentCameraScreen
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
shazarre
force-pushed
the
shazarre/separate_MRZ_data_into_separate_store
branch
from
a7ce32f to
8d9074b
Compare
shazarre
force-pushed
the
shazarre/expose_useDocumentScan_hook_for_DocumentCameraScreen
branch
from
aac2210 to
97d5b37
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/mobile-sdk-alpha/src/processing/mrz.ts (1)
188-199: Block empty passport numbers before declaring success.
checkScannedInfostill accepts a zero-length passport number because it only rejects values longer than 9 characters. That lets an empty document number reach NFC, guaranteeing downstream failures. Please fail fast whenpassportNumber.length === 0(and, ideally, enforce the expected 9-character length window).export function checkScannedInfo(passportNumber: string, dateOfBirth: string, dateOfExpiry: string): boolean { - if (passportNumber.length > 9) { + if (passportNumber.length === 0 || passportNumber.length > 9) { return false; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/providers/selfClientProvider.tsx(1 hunks)app/src/screens/document/DocumentCameraScreen.tsx(2 hunks)app/src/utils/utils.ts(0 hunks)packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/processing/mrz.ts(1 hunks)packages/mobile-sdk-alpha/src/types/events.ts(2 hunks)
💤 Files with no reviewable changes (1)
- app/src/utils/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/screens/document/DocumentCameraScreen.tsx
- app/src/providers/selfClientProvider.tsx
🧰 Additional context used
📓 Path-based instructions (4)
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/src/types/events.tspackages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
packages/mobile-sdk-alpha/src/processing/**
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
Files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
packages/mobile-sdk-alpha/src/processing/mrz.ts
🧬 Code graph analysis (2)
packages/mobile-sdk-alpha/src/types/events.ts (2)
packages/mobile-sdk-alpha/src/index.ts (1)
SdkEvents(71-71)packages/mobile-sdk-alpha/src/browser.ts (1)
SdkEvents(41-41)
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (3)
packages/mobile-sdk-alpha/src/context.tsx (1)
useSelfClient(64-68)packages/mobile-sdk-alpha/src/constants/analytics.ts (1)
PassportEvents(116-136)packages/mobile-sdk-alpha/src/processing/mrz.ts (2)
formatDateToYYMMDD(339-367)checkScannedInfo(188-199)
⏰ 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). (5)
- GitHub Check: e2e-ios
- GitHub Check: android-build-test
- GitHub Check: build-deps
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
| onPassportRead: useCallback( | ||
| (error: Error | null, result?: MRZInfo) => { | ||
| const scanDurationSeconds = calculateScanDurationSeconds(scanStartTimeRef); | ||
|
|
||
| if (error) { | ||
| console.error(error); | ||
|
|
||
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | ||
| reason: 'unknown_error', | ||
| error: error.message || 'Unknown error', | ||
| duration_seconds: parseFloat(scanDurationSeconds), | ||
| }); | ||
|
|
||
| // TODO: Add error handling here | ||
| return; | ||
| } | ||
|
|
||
| if (!result) { | ||
| console.error('No result from passport scan'); | ||
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | ||
| reason: 'invalid_input', | ||
| error: 'No result from scan', | ||
| duration_seconds: parseFloat(scanDurationSeconds), | ||
| }); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const { documentNumber, dateOfBirth, dateOfExpiry, documentType, issuingCountry } = result; | ||
|
|
||
| const formattedDateOfBirth = Platform.OS === 'ios' ? formatDateToYYMMDD(dateOfBirth) : dateOfBirth; | ||
| const formattedDateOfExpiry = Platform.OS === 'ios' ? formatDateToYYMMDD(dateOfExpiry) : dateOfExpiry; | ||
|
|
||
| if (!checkScannedInfo(documentNumber, formattedDateOfBirth, formattedDateOfExpiry)) { | ||
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | ||
| reason: 'invalid_format', | ||
| passportNumberLength: documentNumber.length, | ||
| dateOfBirthLength: formattedDateOfBirth.length, | ||
| dateOfExpiryLength: formattedDateOfExpiry.length, | ||
| duration_seconds: parseFloat(scanDurationSeconds), | ||
| }); | ||
|
|
||
| selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE); | ||
| return; |
There was a problem hiding this comment.
Emit MRZ failure events on errors to unblock navigation.
Both the error and “no result” branches track analytics but never emit DOCUMENT_MRZ_READ_FAILURE. Consumers (e.g., navigation handlers) won’t move the user to the trouble screen, so the flow stalls on camera failures. Please emit the failure event in these branches—mirroring the invalid-format path—to maintain parity with the new contract.
if (error) {
console.error(error);
selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, {
reason: 'unknown_error',
error: error.message || 'Unknown error',
duration_seconds: parseFloat(scanDurationSeconds),
});
- // TODO: Add error handling here
+ selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE);
return;
}
if (!result) {
console.error('No result from passport scan');
selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, {
reason: 'invalid_input',
error: 'No result from scan',
duration_seconds: parseFloat(scanDurationSeconds),
});
+ selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE);
return;
}📝 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.
| onPassportRead: useCallback( | |
| (error: Error | null, result?: MRZInfo) => { | |
| const scanDurationSeconds = calculateScanDurationSeconds(scanStartTimeRef); | |
| if (error) { | |
| console.error(error); | |
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | |
| reason: 'unknown_error', | |
| error: error.message || 'Unknown error', | |
| duration_seconds: parseFloat(scanDurationSeconds), | |
| }); | |
| // TODO: Add error handling here | |
| return; | |
| } | |
| if (!result) { | |
| console.error('No result from passport scan'); | |
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | |
| reason: 'invalid_input', | |
| error: 'No result from scan', | |
| duration_seconds: parseFloat(scanDurationSeconds), | |
| }); | |
| return; | |
| } | |
| const { documentNumber, dateOfBirth, dateOfExpiry, documentType, issuingCountry } = result; | |
| const formattedDateOfBirth = Platform.OS === 'ios' ? formatDateToYYMMDD(dateOfBirth) : dateOfBirth; | |
| const formattedDateOfExpiry = Platform.OS === 'ios' ? formatDateToYYMMDD(dateOfExpiry) : dateOfExpiry; | |
| if (!checkScannedInfo(documentNumber, formattedDateOfBirth, formattedDateOfExpiry)) { | |
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | |
| reason: 'invalid_format', | |
| passportNumberLength: documentNumber.length, | |
| dateOfBirthLength: formattedDateOfBirth.length, | |
| dateOfExpiryLength: formattedDateOfExpiry.length, | |
| duration_seconds: parseFloat(scanDurationSeconds), | |
| }); | |
| selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE); | |
| return; | |
| onPassportRead: useCallback( | |
| (error: Error | null, result?: MRZInfo) => { | |
| const scanDurationSeconds = calculateScanDurationSeconds(scanStartTimeRef); | |
| if (error) { | |
| console.error(error); | |
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | |
| reason: 'unknown_error', | |
| error: error.message || 'Unknown error', | |
| duration_seconds: parseFloat(scanDurationSeconds), | |
| }); | |
| selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE); | |
| return; | |
| } | |
| if (!result) { | |
| console.error('No result from passport scan'); | |
| selfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED, { | |
| reason: 'invalid_input', | |
| error: 'No result from scan', | |
| duration_seconds: parseFloat(scanDurationSeconds), | |
| }); | |
| selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts around lines 31 to
74, the error and "no result" branches track CAMERA_SCAN_FAILED but do not emit
SdkEvents.DOCUMENT_MRZ_READ_FAILURE; add a
selfClient.emit(SdkEvents.DOCUMENT_MRZ_READ_FAILURE) call in both branches after
tracking (before returning) so consumers receive the failure event just like the
invalid-format path.
There was a problem hiding this comment.
code was moved as is: no change to logic right now, although that seems indeed like a good idea
There was a problem hiding this comment.
@shazarre, thanks for clarifying! Since you're deferring the logic improvement to emit failure events on errors, would you like me to create a GitHub issue to track adding DOCUMENT_MRZ_READ_FAILURE emissions in the error and "no result" branches?
🧠 Learnings used
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
|
|
||
| // Add a ref to track when the camera screen is mounted | ||
| const scanStartTimeRef = useRef(Date.now()); | ||
| const { onPassportRead } = useReadMRZ(scanStartTimeRef); |
There was a problem hiding this comment.
Bug: Scan Duration Error Due to Persistent Start Time
The scanStartTimeRef is initialized once on mount and isn't reset. This leads to incorrect scan duration calculations, causing inflated duration_seconds in analytics events when users navigate away and return, or attempt multiple scans within the same session.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}: Do NOT mock @selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('@selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)
Use actual imports from @selfxyz/mobile-sdk-alpha in tests
Write integration tests that exercise the real validation logic (not mocks)
Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Ensure parseNFCResponse() works with representative, synthetic NFC data
Never use real user PII in tests; use only synthetic, anonymized, or approved test vectors
Files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx}
📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)
packages/mobile-sdk-alpha/**/*.{ts,tsx}: Use strict TypeScript type checking across the codebase
Follow ESLint TypeScript-specific rules
Avoid introducing circular dependencies
Files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
**/*.{js,ts,tsx,jsx,sol,nr}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,tsx,jsx,sol,nr}: NEVER log sensitive data including PII (names, DOB, passport numbers, addresses), credentials, tokens, API keys, private keys, or session identifiers.
ALWAYS redact/mask sensitive fields in logs using consistent patterns (e.g.,***-***-1234for passport numbers,J*** D***for names).
Files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
**/*.{test,spec}.{ts,js,tsx,jsx}
⚙️ CodeRabbit configuration file
**/*.{test,spec}.{ts,js,tsx,jsx}: Review test files for:
- Test coverage completeness
- Test case quality and edge cases
- Mock usage appropriateness
- Test readability and maintainability
Files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
packages/mobile-sdk-alpha/**/*.{ts,tsx,js,jsx}: Review alpha mobile SDK code for:
- API consistency with core SDK
- Platform-neutral abstractions
- Performance considerations
- Clear experimental notes or TODOs
Files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Verify extractMRZInfo() using published sample MRZ strings (e.g., ICAO examples)
Applied to files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Ensure parseNFCResponse() works with representative, synthetic NFC data
Applied to files:
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts
🧬 Code graph analysis (1)
packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts (2)
packages/mobile-sdk-alpha/src/flows/onboarding/read-mrz.ts (1)
useReadMRZ(27-94)packages/mobile-sdk-alpha/src/constants/analytics.ts (1)
PassportEvents(116-136)
⏰ 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). (4)
- GitHub Check: build-android
- GitHub Check: test
- GitHub Check: build-ios
- GitHub Check: e2e-ios
| vi.mock('../../../src/processing/mrz', () => ({ | ||
| checkScannedInfo: vi.fn(() => true), | ||
| formatDateToYYMMDD: vi.fn((date: string) => { | ||
| // Simple mock implementation for testing | ||
| if (date === '1974-08-12') return '740812'; | ||
| if (date === '2012-04-15') return '120415'; | ||
| return date; | ||
| }), | ||
| })); |
There was a problem hiding this comment.
Critical: Remove mocks for MRZ processing functions.
Lines 19-27 mock checkScannedInfo and formatDateToYYMMDD, which violates the coding guideline: "Write integration tests that exercise the real validation logic (not mocks)". By mocking checkScannedInfo to always return true, these tests cannot verify that invalid MRZ data is properly rejected.
As per coding guidelines for this path pattern, you must use actual imports and test the real validation logic with synthetic ICAO sample MRZ strings.
Remove the mock and import the real functions:
-// Mock the MRZ processing functions
-vi.mock('../../../src/processing/mrz', () => ({
- checkScannedInfo: vi.fn(() => true),
- formatDateToYYMMDD: vi.fn((date: string) => {
- // Simple mock implementation for testing
- if (date === '1974-08-12') return '740812';
- if (date === '2012-04-15') return '120415';
- return date;
- }),
-}));
+// Import real MRZ processing functions for integration testing
+import { checkScannedInfo, formatDateToYYMMDD } from '../../../src/processing/mrz';Then add test cases that verify the hook's behavior when checkScannedInfo returns false (e.g., with malformed dates or invalid passport numbers).
Based on coding guidelines.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts around
lines 19-27, remove the vi.mock that replaces checkScannedInfo and
formatDateToYYMMDD and instead import the real functions from the MRZ module so
the tests exercise actual validation logic; update tests to feed synthetic
ICAO-format MRZ samples (both valid and malformed—e.g., bad dates, invalid
passport numbers) and assert the hook rejects invalid MRZ by checking that
checkScannedInfo returns false and the hook behaves accordingly; ensure no
mocking of MRZ processing functions remains anywhere in this test file.
| vi.mock('../../../src/context', () => ({ | ||
| useSelfClient: vi.fn(), | ||
| })); |
There was a problem hiding this comment.
Critical: Remove mock for useSelfClient context.
Lines 30-32 mock useSelfClient from @selfxyz/mobile-sdk-alpha, which directly violates the guideline: "Do NOT mock @selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('@selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)".
This prevents the tests from exercising the real client behavior and state management.
Remove this mock and provide a real SelfClient instance (or use a test provider that wraps the hook with the actual context). For example, wrap renderHook with the real SelfClientProvider initialized with test configuration.
Based on coding guidelines.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts around
lines 30-32, remove the vi.mock(...) that replaces useSelfClient; instead
instantiate or provide a real SelfClient via the library's provider when
rendering hooks/components. Replace the mock with wrapping renderHook/render
calls in the SelfClientProvider (or a test provider helper) initialized with a
test configuration/client instance so the actual context hook is used; ensure
any test setup imports the real SelfClient/SelfClientProvider from
@selfxyz/mobile-sdk-alpha and that you properly teardown/reset the
provider/client between tests.
| it('handles successful MRZ read with valid data on iOS', () => { | ||
| // Mock Platform.OS to be 'ios' | ||
| vi.mocked(Platform).OS = 'ios'; | ||
|
|
||
| const { result } = renderHook(() => useReadMRZ(scanStartTimeRef)); | ||
| const { onPassportRead } = result.current; | ||
|
|
||
| const mockMRZInfo: MRZInfo = { | ||
| documentNumber: 'L898902C3', | ||
| dateOfBirth: '1974-08-12', | ||
| dateOfExpiry: '2012-04-15', | ||
| documentType: 'P', | ||
| issuingCountry: 'UTO', | ||
| validation: { | ||
| format: true, | ||
| passportNumberChecksum: true, | ||
| dateOfBirthChecksum: true, | ||
| dateOfExpiryChecksum: true, | ||
| compositeChecksum: true, | ||
| overall: true, | ||
| }, | ||
| }; | ||
|
|
||
| // Call the callback with valid data | ||
| onPassportRead(null, mockMRZInfo); | ||
|
|
||
| // Verify MRZ state was set correctly | ||
| expect(mockMRZState.setMRZForNFC).toHaveBeenCalledWith({ | ||
| passportNumber: 'L898902C3', | ||
| dateOfBirth: '740812', // Formatted for iOS | ||
| dateOfExpiry: '120415', // Formatted for iOS | ||
| documentType: 'P', | ||
| countryCode: 'UTO', | ||
| }); | ||
|
|
||
| // Verify success analytics event was tracked | ||
| expect(mockSelfClient.trackEvent).toHaveBeenCalledWith(PassportEvents.CAMERA_SCAN_SUCCESS, { | ||
| duration_seconds: expect.any(Number), | ||
| }); | ||
|
|
||
| // Verify success event was emitted | ||
| expect(mockSelfClient.emit).toHaveBeenCalledWith(SdkEvents.DOCUMENT_MRZ_READ_SUCCESS); | ||
| }); | ||
|
|
||
| it('handles successful MRZ read with valid data on Android', () => { | ||
| // Mock Platform.OS to be 'android' | ||
| vi.mocked(Platform).OS = 'android'; | ||
|
|
||
| const { result } = renderHook(() => useReadMRZ(scanStartTimeRef)); | ||
| const { onPassportRead } = result.current; | ||
|
|
||
| const mockMRZInfo: MRZInfo = { | ||
| documentNumber: 'L898902C3', | ||
| dateOfBirth: '740812', // Already in YYMMDD format | ||
| dateOfExpiry: '120415', // Already in YYMMDD format | ||
| documentType: 'P', | ||
| issuingCountry: 'UTO', | ||
| validation: { | ||
| format: true, | ||
| passportNumberChecksum: true, | ||
| dateOfBirthChecksum: true, | ||
| dateOfExpiryChecksum: true, | ||
| compositeChecksum: true, | ||
| overall: true, | ||
| }, | ||
| }; | ||
|
|
||
| // Call the callback with valid data | ||
| onPassportRead(null, mockMRZInfo); | ||
|
|
||
| // Verify MRZ state was set correctly (dates not formatted on Android) | ||
| expect(mockMRZState.setMRZForNFC).toHaveBeenCalledWith({ | ||
| passportNumber: 'L898902C3', | ||
| dateOfBirth: '740812', // Not formatted on Android | ||
| dateOfExpiry: '120415', // Not formatted on Android | ||
| documentType: 'P', | ||
| countryCode: 'UTO', | ||
| }); | ||
|
|
||
| // Verify success analytics event was tracked | ||
| expect(mockSelfClient.trackEvent).toHaveBeenCalledWith(PassportEvents.CAMERA_SCAN_SUCCESS, { | ||
| duration_seconds: expect.any(Number), | ||
| }); | ||
|
|
||
| // Verify success event was emitted | ||
| expect(mockSelfClient.emit).toHaveBeenCalledWith(SdkEvents.DOCUMENT_MRZ_READ_SUCCESS); | ||
| }); | ||
|
|
||
| it('trims whitespace from document type and country code', () => { | ||
| const { result } = renderHook(() => useReadMRZ(scanStartTimeRef)); | ||
| const { onPassportRead } = result.current; | ||
|
|
||
| const mockMRZInfo: MRZInfo = { | ||
| documentNumber: 'L898902C3', | ||
| dateOfBirth: '740812', | ||
| dateOfExpiry: '120415', | ||
| documentType: ' P ', // With whitespace | ||
| issuingCountry: ' uto ', // With whitespace and lowercase | ||
| validation: { | ||
| format: true, | ||
| passportNumberChecksum: true, | ||
| dateOfBirthChecksum: true, | ||
| dateOfExpiryChecksum: true, | ||
| compositeChecksum: true, | ||
| overall: true, | ||
| }, | ||
| }; | ||
|
|
||
| // Call the callback with valid data | ||
| onPassportRead(null, mockMRZInfo); | ||
|
|
||
| // Verify MRZ state was set with trimmed and uppercased values | ||
| expect(mockMRZState.setMRZForNFC).toHaveBeenCalledWith({ | ||
| passportNumber: 'L898902C3', | ||
| dateOfBirth: '740812', | ||
| dateOfExpiry: '120415', | ||
| documentType: 'P', // Trimmed | ||
| countryCode: 'UTO', // Trimmed and uppercased | ||
| }); | ||
| }); | ||
|
|
||
| it('handles empty document type and country code gracefully', () => { | ||
| const { result } = renderHook(() => useReadMRZ(scanStartTimeRef)); | ||
| const { onPassportRead } = result.current; | ||
|
|
||
| const mockMRZInfo: MRZInfo = { | ||
| documentNumber: 'L898902C3', | ||
| dateOfBirth: '740812', | ||
| dateOfExpiry: '120415', | ||
| documentType: '', // Empty | ||
| issuingCountry: '', // Empty | ||
| validation: { | ||
| format: true, | ||
| passportNumberChecksum: true, | ||
| dateOfBirthChecksum: true, | ||
| dateOfExpiryChecksum: true, | ||
| compositeChecksum: true, | ||
| overall: true, | ||
| }, | ||
| }; | ||
|
|
||
| // Call the callback with valid data | ||
| onPassportRead(null, mockMRZInfo); | ||
|
|
||
| // Verify MRZ state was set with empty strings | ||
| expect(mockMRZState.setMRZForNFC).toHaveBeenCalledWith({ | ||
| passportNumber: 'L898902C3', | ||
| dateOfBirth: '740812', | ||
| dateOfExpiry: '120415', | ||
| documentType: '', // Empty string | ||
| countryCode: '', // Empty string | ||
| }); | ||
| }); | ||
|
|
||
| it('calculates scan duration correctly', () => { | ||
| const { result } = renderHook(() => useReadMRZ(scanStartTimeRef)); | ||
| const { onPassportRead } = result.current; | ||
|
|
||
| const mockMRZInfo: MRZInfo = { | ||
| documentNumber: 'L898902C3', | ||
| dateOfBirth: '740812', | ||
| dateOfExpiry: '120415', | ||
| documentType: 'P', | ||
| issuingCountry: 'UTO', | ||
| validation: { | ||
| format: true, | ||
| passportNumberChecksum: true, | ||
| dateOfBirthChecksum: true, | ||
| dateOfExpiryChecksum: true, | ||
| compositeChecksum: true, | ||
| overall: true, | ||
| }, | ||
| }; | ||
|
|
||
| // Call the callback with valid data | ||
| onPassportRead(null, mockMRZInfo); | ||
|
|
||
| // Verify the duration was calculated and passed to analytics | ||
| expect(mockSelfClient.trackEvent).toHaveBeenCalledWith(PassportEvents.CAMERA_SCAN_SUCCESS, { | ||
| duration_seconds: expect.any(Number), | ||
| }); | ||
|
|
||
| // The duration should be approximately 2.5 seconds (2500ms / 1000) | ||
| const trackEventCall = mockSelfClient.trackEvent.mock.calls[0]; | ||
| const durationSeconds = trackEventCall[1].duration_seconds; | ||
| expect(durationSeconds).toBeCloseTo(2.5, 1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Major: Test suite lacks coverage for invalid MRZ scenarios.
Because checkScannedInfo is mocked to always return true (line 20), none of these tests verify the hook's behavior when real validation logic detects invalid MRZ data. The coding guideline requires: "Write integration tests that exercise the real validation logic (not mocks)" and "Test isPassportDataValid() with realistic synthetic passport data".
Without exercising real validation, these tests miss critical edge cases:
- Invalid date formats (e.g.,
990230for Feb 30) - Passport numbers with wrong check digits
- Expired or future dates
- Missing or malformed fields
After removing the mocks (per previous comments), add test cases such as:
it('emits DOCUMENT_MRZ_READ_FAILURE when checkScannedInfo returns false', () => {
const { result } = renderHook(() => useReadMRZ(scanStartTimeRef));
const { onPassportRead } = result.current;
const invalidMRZInfo: MRZInfo = {
documentNumber: 'INVALID', // Too short or wrong checksum
dateOfBirth: '990230', // Invalid date
dateOfExpiry: '120415',
documentType: 'P',
issuingCountry: 'UTO',
validation: { /* ... */ },
};
onPassportRead(null, invalidMRZInfo);
expect(mockSelfClient.emit).toHaveBeenCalledWith(SdkEvents.DOCUMENT_MRZ_READ_FAILURE);
expect(mockSelfClient.trackEvent).toHaveBeenCalledWith(
PassportEvents.CAMERA_SCAN_FAILED,
expect.objectContaining({ reason: 'invalid_format' })
);
});Use published ICAO sample MRZ strings (as noted in learnings) to construct realistic test data.
Based on coding guidelines and learnings.
🤖 Prompt for AI Agents
In packages/mobile-sdk-alpha/tests/flows/onboarding/read-mrz.test.ts around
lines 64-251 (note checkScannedInfo is currently mocked to always return true
earlier in the file), the suite never exercises real MRZ validation; remove or
stop mocking checkScannedInfo so the hook uses the real validation logic, then
add new tests that call onPassportRead with realistic invalid MRZ samples (e.g.,
invalid date like 990230, passport number with wrong checksum, expired/future
expiry dates, and missing/malformed fields using ICAO sample MRZ strings) and
assert that the hook emits SdkEvents.DOCUMENT_MRZ_READ_FAILURE and calls
mockSelfClient.trackEvent(PassportEvents.CAMERA_SCAN_FAILED,
expect.objectContaining({ reason: '<expected_reason>' })) for each invalid
scenario.
shazarre
deleted the
shazarre/expose_useDocumentScan_hook_for_DocumentCameraScreen
branch
2 participants
This PR exposes
useReadMRZhook that abstracts away the callback of passport camera screen to read the MRZ data. It takes care of the new store interaction and moves analytics setup outside as well as hooks navigating away from that screen to an event instead of inline navigation call.Blocked by merging #1187 first.
Summary by CodeRabbit
New Features
Refactor
Tests