feat: MRZ data confirmation for NFC scanning#1767
Merged
transphorm merged 13 commits intodevfrom Apr 22, 2026
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a DataConfirmation screen and InputField (with date picker), MRZ date-parsing helpers and re-exports in the SDK, new analytics events, navigation wiring to DataConfirmation, utilities/tests for string diffing, and the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DataConfirmation as "DataConfirmationScreen"
participant InputField as "InputField"
participant MRZStore as "MRZ Store"
participant Analytics as "Analytics"
participant Navigation as "Navigation"
User->>DataConfirmation: open (after MRZ read)
DataConfirmation->>MRZStore: read MRZ data
MRZStore-->>DataConfirmation: initial fields
User->>InputField: edit fields (number / dates)
InputField-->>DataConfirmation: onChangeText / onDateChange
User->>DataConfirmation: tap Continue
DataConfirmation->>DataConfirmation: calculateFirstDifference(original, modified)
alt changes detected
DataConfirmation->>Analytics: track MRZ_DATA_MODIFIED
DataConfirmation->>MRZStore: setMRZForNFC(updatedData)
else no changes
DataConfirmation->>Analytics: track DATA_CONFIRMATION_COMPLETED (had_changes: false)
end
DataConfirmation->>Navigation: navigate to DocumentNFCScan
Navigation-->>User: show NFC scan
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Collaborator
Author
|
@coderabbitai review |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/InputField.tsx`:
- Around line 164-175: The shared text style object textInput in InputField.tsx
includes the web-only property outlineStyle which is not part of React Native's
TextStyle and causes TS2769; remove outlineStyle from the textInput style (or
move it behind a web-only conditional) and ensure any web-specific focus styling
is applied only in a platform-specific path (e.g., a separate web-only style or
Platform.select) so TextInput and its typing accept the style without error.
In `@app/src/screens/documents/scanning/DataConfirmationScreen.test.tsx`:
- Around line 55-57: The test currently creates a nested require('react-native')
inside the jest.mock callback which is banned; fix by removing the nested
require and instead import View and Text at the top of the test file via
top-level ES imports (e.g., import { View, Text } from 'react-native') and then
use those imported symbols inside the jest.mock('@/components/InputField', ...)
mock implementation so the mock reuses the hoisted View and Text instead of
requiring react-native at runtime.
- Around line 55-77: The current jest.mock for InputField replaces the real
component with a passive stub that doesn't forward edits, hiding the MRZ
confirmation behavior; replace the stub with a minimal mock that preserves
behavior by rendering a TextInput (or a component that calls the provided
onChangeText prop) and binds value and onChangeText so tests can drive and
observe actual user edits. Locate the mocked InputField in
DataConfirmationScreen.test.tsx (the jest.mock block) and change it to render an
interactive element that uses the onChangeText prop (and the same testID scheme)
or simply import the real InputField component for the test so
fireEvent.changeText triggers the same prop wiring the production code uses.
Ensure inputFieldCallbacks are no longer used to simulate edits; instead drive
edits via fireEvent.changeText on the rendered testID to validate real prop
forwarding.
In `@app/src/screens/documents/scanning/DataConfirmationScreen.tsx`:
- Around line 73-78: handleDateChange currently calls
formatDateToYYMMDD(date.toISOString()) which shifts the day for non-UTC users;
instead generate a local-date ISO (using date.getFullYear(), date.getMonth()+1,
date.getDate() with zero-padding) or otherwise convert the Date to a local
YYYY-MM-DD string and pass that into formatDateToYYMMDD before calling
setFields, so the MRZ YYMMDD preserves the user's selected local day while
InputField can still display its selectedDate. Ensure you update only
handleDateChange (and keep formatDateToYYMMDD usage) so setFields([... ,
[field]: mrzFormattedDate]) receives the corrected local-based MRZ value.
- Around line 89-121: The code currently sends raw MRZ characters because
calculateFirstDifference(...) results are placed into diffs and forwarded to
trackEvent(PassportEvents.MRZ_DATA_MODIFIED); replace that behavior by redacting
or replacing the diff values before sending (e.g., store only field names,
indices/positions of differences, masked snippets like first/last character or
length, or a boolean), i.e., transform filteredDiffs so values do NOT contain
actual MRZ/PII (use calculateFirstDifference only to compute metadata, then map
its output to a non-PII representation) and pass that sanitized object and
counts to trackEvent instead of raw diffs.
In `@packages/mobile-sdk-alpha/src/processing/mrz.ts`:
- Around line 341-350: parseMRZDateComponents currently only checks length and
allows non-numeric/impossible YYMMDD values through; update it to reject
malformed dates by (1) ensuring all 6 characters are digits, (2) parsing
year/month/day into numbers, (3) mapping two‑digit year to a full year the code
expects (keep existing YY->YYYY logic if present elsewhere), (4) constructing a
Date (use UTC or consistent timezone) and then verifying the Date's
year/month/day match the parsed components (to catch overflow like month=13 or
day=32); if any check fails return null. Apply the same stricter
validation/verification logic to parseMRZExpiryDate so both helpers only return
valid date objects or null.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a2045ba-c697-4fe5-8dc1-30f825b4bb32
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
app/package.jsonapp/src/components/InputField.tsxapp/src/navigation/documents.tsapp/src/navigation/types.tsapp/src/providers/selfClientProvider.tsxapp/src/screens/documents/scanning/DataConfirmationScreen.test.tsxapp/src/screens/documents/scanning/DataConfirmationScreen.tsxapp/src/utils/diffCalculator.test.tsapp/src/utils/diffCalculator.tsapp/src/utils/documentAttributes.tspackages/mobile-sdk-alpha/src/constants/analytics.tspackages/mobile-sdk-alpha/src/index.tspackages/mobile-sdk-alpha/src/mrz/index.tspackages/mobile-sdk-alpha/src/processing/mrz.tspackages/mobile-sdk-alpha/tests/processing/mrz.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/InputField.tsx`:
- Around line 14-15: The file imports the native DatePicker unconditionally and
renders it inside the InputField component, which breaks web; change to a
platform split by removing the top-level import of DatePicker and instead
require/import the native picker only on native platforms (use Platform.OS or
Platform.select) and render a web-safe fallback (e.g., an HTML input type="date"
or react-native-web compatible component) inside InputField's render/return
path; specifically, replace the unconditional DatePicker import with a
conditional lazy require (or dynamic import) and update the JSX in InputField
(the block that currently renders DatePicker) to choose between the native
DatePicker and the web input based on Platform.OS === 'web'.
- Around line 42-44: InputField currently constructs selectedDate once with
useState and formats it using local getters which causes timezone shifts, and it
imports react-native-date-picker unconditionally breaking web builds; update
InputField to (1) resync selectedDate whenever the value prop changes by adding
a useEffect that calls setSelectedDate(new Date(value)) (ensure you
normalize/parse the incoming ISO as UTC if needed), (2) when producing display
strings or extracting year/month/day use UTC getters
(getUTCFullYear/getUTCMonth/getUTCDate) instead of local getters to avoid
midnight-UTC shifts, and (3) make the react-native-date-picker import/platform
usage conditional (use Platform.OS !== 'web' or a dynamic require) and render a
web-safe fallback (e.g., input type="date" or a web picker) when on web so the
native-only library is not imported on web. Ensure references to selectedDate,
setSelectedDate, InputField, and the date formatting code are updated
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8a061e26-5b2d-40fa-b198-3664fc47381f
📒 Files selected for processing (1)
app/src/components/InputField.tsx
shazarre
force-pushed
the
shazarre/feat/mrz_data_confirmation
branch
from
02e88c1 to
45c89fe
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Greptile SummaryThis PR introduces an MRZ data confirmation screen that lets users review and correct passport/ID fields extracted via camera scanning before proceeding to NFC chip reading. It adds a new Key observations:
Confidence Score: 4/5Safe to merge after removing the console.log that leaks MRZ data to device logs. The feature is well-scoped and the previously flagged store-spreading issue has been resolved. The one blocking item is the debug console.log in validateTD1Format which logs raw passport PII; removing that single line is a trivial fix. The remaining findings (empty-value DatePicker default, unused analytics event) are non-blocking P2 cleanups. packages/mobile-sdk-alpha/src/processing/mrz.ts — remove the console.log on line 65 before shipping.
|
| Filename | Overview |
|---|---|
| app/src/screens/documents/scanning/DataConfirmationScreen.tsx | New screen for reviewing/editing extracted MRZ fields before NFC scanning; cleanly reads from MRZ store, tracks analytics, and calls setMRZForNFC only when data has changed. |
| app/src/components/InputField.tsx | New multi-mode input component supporting alphanumeric text and date picker modes; DatePicker fallback defaults to today's date when value is empty, which is an odd UX default for a birth date field. |
| packages/mobile-sdk-alpha/src/processing/mrz.ts | Added parseMRZBirthDate / parseMRZExpiryDate helpers; contains a leftover debug console.log in validateTD1Format that logs raw MRZ lines containing sensitive personal data. |
| app/src/utils/yymmdd.ts | New YYMMDD display/conversion utilities with consistent year-century cutoff (≤30 → 2000s) matching parseMRZBirthDate; well-covered by tests. |
| app/src/screens/documents/scanning/RegistrationFallbackNFCScreen.tsx | Added 'Check scanned data' button navigating to the new DataConfirmation screen, giving users an escape hatch when NFC scanning fails. |
| packages/mobile-sdk-alpha/src/constants/analytics.ts | Added DATA_CONFIRMATION_COMPLETED and MRZ_DATA_MODIFIED events; MRZ_DATA_MODIFIED is defined but not tracked anywhere in the codebase. |
| packages/mobile-sdk-alpha/src/mrz/index.ts | Re-exports newly added parseMRZBirthDate and parseMRZExpiryDate from the processing layer for external consumption. |
| app/src/navigation/documents.ts | Registers new DataConfirmation screen with HeadlessNavForEuclid header and correct statusBar wiring from the screen's static property. |
Comments Outside Diff (1)
-
packages/mobile-sdk-alpha/src/processing/mrz.ts, line 64-65 (link)Debug
console.logexposes sensitive passport datavalidateTD1Formatlogs the raw MRZ lines to the console before any validation. MRZ lines contain personally identifiable information: document number, date of birth, nationality, sex, and expiry date. In a production build this data will appear in device logs, which can be collected by crash reporters, MDM solutions, or local debugging tools.
Reviews (2): Last reviewed commit: "simplify date handling" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds support for confirming MRZ data needed for later NFC scanning utilizing euclid DataConfirmation screen.
Tested
Tested on an example MRZ data (https://www.idenfy.com/blog/machine-readable-zone/) and a real passport.
How to QA
Scan MRZ data, compare with the result on the screen (change something and observe analytics data).
Blocked by https://github.com/selfxyz/euclid/pull/71
TODO
Summary by CodeRabbit
New Features
Tests
Chores