Skip to content

Conversation

@shazarre
Copy link
Collaborator

@shazarre shazarre commented Oct 2, 2025

This PR moves all MRZ-related fields from app's userStore to a new SDK's mrzStore.

Later on it will be used for NFC scanning behind the scenes instead of being passed around by the app itself. See #1188 for reference.

Summary by CodeRabbit

  • New Features

    • Central MRZ (passport) state exposed in the SDK and used across document capture and NFC flows.
  • Improvements

    • NFC analytics configuration now runs at the scan initiation point to ensure telemetry is set up before a scan.
  • Bug Fixes

    • Resolved MRZ data inconsistencies between camera, method selection, and NFC scan screens.
  • Refactor

    • MRZ fields removed from the local user store and migrated to the shared MRZ store.
  • Tests

    • Updated NFC analytics tests to align with the new scan initialization behavior.

@shazarre shazarre self-assigned this Oct 2, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

Migrates MRZ handling from the app-local user store into a new MRZ Zustand store in the mobile SDK, exposes useMRZStore/getMRZState on the SDK client, updates app screens to use the SDK MRZ store and setMRZForNFC, and removes analytics initialization from nfcScanner.scan (moved to callers).

Changes

Cohort / File(s) Summary
Screens: MRZ via SDK store & NFC analytics
app/src/screens/document/DocumentCameraScreen.tsx, app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx, app/src/screens/document/DocumentNFCScanScreen.tsx
Replace app-local useUserStore MRZ usage with selfClient.useMRZStore; persist MRZ via setMRZForNFC/update; add configureNfcAnalytics() calls at screen level before scanning; update effect deps and selfClient destructuring to include useMRZStore.
App user store cleanup
app/src/stores/userStore.ts
Remove MRZ fields (passportNumber, dateOfBirth, dateOfExpiry, documentType, countryCode) and deleteMrzFields; drop env defaults and initial MRZ values.
NFC utils: analytics init removal
app/src/utils/nfcScanner.ts
Remove initial configureNfcAnalytics() call from scan(); core scan flow and error handling unchanged.
SDK client & types (public API extension)
packages/mobile-sdk-alpha/src/client.ts, packages/mobile-sdk-alpha/src/types/public.ts
Expose useMRZStore and getMRZState() on the SelfClient public API surface; add MRZ store wiring to client return.
SDK MRZ store (new)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx
Add new Zustand useMRZStore with MRZState fields and actions: setMRZForNFC, clearMRZ, update; export store and MRZ types.
Tests updated
app/tests/utils/nfcScanner.test.ts
Remove assertion that configureNfcAnalytics() is called pre-scan; retain PassportReader.scan assertion.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant MethodScreen as DocumentNFCMethodSelectionScreen
  participant CameraScreen as DocumentCameraScreen
  participant ScanScreen as DocumentNFCScanScreen
  participant SelfClient as SelfClient (SDK)
  participant MRZStore as useMRZStore
  participant Analytics as configureNfcAnalytics
  participant NFC as nfcScanner.scan

  Note over SelfClient,MRZStore: SDK exposes MRZ store and getters
  User->>MethodScreen: enter passport data
  MethodScreen->>SelfClient: access useMRZStore
  MethodScreen->>MRZStore: setMRZForNFC(data)

  User->>CameraScreen: capture MRZ via OCR
  CameraScreen->>SelfClient: access useMRZStore
  CameraScreen->>MRZStore: setMRZForNFC(data)

  User->>ScanScreen: start NFC scan
  ScanScreen->>Analytics: configureNfcAnalytics()
  ScanScreen->>SelfClient: access useMRZStore
  ScanScreen->>MRZStore: read MRZ fields
  ScanScreen->>NFC: scan(MRZ fields)
  NFC-->>ScanScreen: scan result
  ScanScreen->>MRZStore: setMRZForNFC(updated) [if needed]
  ScanScreen-->>User: continue flow / show errors
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

codex

Suggested reviewers

  • aaronmgdr
  • transphorm

Poem

MRZ moved out to SDK shore,
Screens now call the store once more.
Analytics waits where scans begin,
Passport fields set — the flow ticks in.
Small refactor, tidy win. 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the core change of extracting MRZ data out of the user store into its own module, making it clear to reviewers what the pull request accomplishes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch shazarre/separate_MRZ_data_into_separate_store

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@shazarre
Copy link
Collaborator Author

shazarre commented Oct 2, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (1)

113-141: Reduce repetition in MRZ field updates

Each change handler spreads all five MRZ fields to update a single value. This is verbose and error-prone—if the MRZ state shape changes, all three handlers need updates.

Consider using a single helper or leveraging the store's update method (if available) to patch individual fields:

  const onPassportNumberChange = (text: string) => {
-   setMRZForNFC({
-     passportNumber: text,
-     dateOfBirth,
-     dateOfExpiry,
-     documentType,
-     countryCode
-   });
+   setMRZForNFC({ ...useMRZStore.getState(), passportNumber: text });
  };

Or introduce a helper:

const updateMRZField = (field: keyof MRZState, value: string) => {
  setMRZForNFC({ ...useMRZStore.getState(), [field]: value });
};

const onPassportNumberChange = (text: string) => updateMRZField('passportNumber', text);

Note: The static analysis tool flagged missing trailing commas at lines 105, 119, 129, and 139. Consider fixing these for consistency with your project's formatting rules.

packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)

43-51: Optional: simplify setMRZForNFC implementation

The function explicitly maps each field. While correct, you could leverage spread syntax for brevity and maintainability.

  setMRZForNFC: data => {
-   set({
-     passportNumber: data.passportNumber,
-     dateOfBirth: data.dateOfBirth,
-     dateOfExpiry: data.dateOfExpiry,
-     countryCode: data.countryCode,
-     documentType: data.documentType,
-   });
+   set({ ...data });
  },

This assumes data contains only MRZ fields. If you want to be explicit (for clarity or type narrowing), the current form is also fine.

Bonus: Remove the unnecessary parentheses around data (linter flagged this at line 43).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25634e0 and 2615f0b.

📒 Files selected for processing (8)
  • app/src/screens/document/DocumentCameraScreen.tsx (3 hunks)
  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (2 hunks)
  • app/src/screens/document/DocumentNFCScanScreen.tsx (5 hunks)
  • app/src/stores/userStore.ts (0 hunks)
  • app/src/utils/nfcScanner.ts (0 hunks)
  • packages/mobile-sdk-alpha/src/client.ts (2 hunks)
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1 hunks)
  • packages/mobile-sdk-alpha/src/types/public.ts (2 hunks)
💤 Files with no reviewable changes (2)
  • app/src/stores/userStore.ts
  • app/src/utils/nfcScanner.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/screens/document/DocumentCameraScreen.tsx
  • app/src/screens/document/DocumentNFCScanScreen.tsx
  • app/src/screens/document/DocumentNFCMethodSelectionScreen.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/client.ts
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx
  • packages/mobile-sdk-alpha/src/types/public.ts
🧠 Learnings (1)
📚 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/client.ts
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx
  • packages/mobile-sdk-alpha/src/types/public.ts
🪛 GitHub Actions: Mobile SDK CI
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx

[error] 40-40: ESLint: 'get' is defined but never used. Allowed unused args must match /^_/u (no-unused-vars)

🪛 GitHub Actions: Workspace CI
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx

[error] 40-40: ESLint: 'get' is defined but never used. (no-unused-vars)

🪛 GitHub Check: lint
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx

[warning] 43-43:
Replace (data) with data


[failure] 40-40:
'get' is defined but never used. Allowed unused args must match /^_/u

🪛 GitHub Check: workspace-lint
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx

[warning] 43-43:
Replace (data) with data


[failure] 40-40:
'get' is defined but never used. Allowed unused args must match /^_/u

app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx

[warning] 139-139:
Insert ,


[warning] 129-129:
Insert ,


[warning] 119-119:
Insert ,


[warning] 105-105:
Insert ,

⏰ 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: analyze-android
  • GitHub Check: analyze-ios
  • GitHub Check: build-deps
  • GitHub Check: e2e-ios
  • GitHub Check: android-build-test
🔇 Additional comments (4)
app/src/screens/document/DocumentCameraScreen.tsx (1)

37-38: LGTM: Clean migration to MRZ store

The transition from the local user store to the SDK's MRZ store is implemented correctly. The destructuring pattern and dependency array updates are appropriate.

Also applies to: 105-111, 119-119

packages/mobile-sdk-alpha/src/types/public.ts (1)

11-11: LGTM: Consistent public API extension

The MRZ state type additions follow the established pattern for other stores (proving, protocol, selfApp). The public API surface is appropriately expanded.

Also applies to: 187-187, 192-192

packages/mobile-sdk-alpha/src/client.ts (1)

11-11: LGTM: MRZ store wired correctly into client

The MRZ store integration follows the established pattern for other Zustand stores. The getter and reactive hook are appropriately exposed.

Also applies to: 184-186, 192-192

app/src/screens/document/DocumentNFCScanScreen.tsx (1)

95-107: onVerifyPress includes all MRZ fields in its dependencies
No changes required.

@shazarre shazarre marked this pull request as ready for review October 2, 2025 14:37
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (1)

204-226: Consider adding format validation for MRZ date fields.

The date input fields accept keyboardType="numeric" but don't enforce or validate the YYMMDD format before proceeding. While this may be a debugging/fallback feature, invalid formats could cause NFC scan failures.

You could add basic validation:

const isValidYYMMDD = (value: string) => {
  if (value.length !== 6) return false;
  const year = parseInt(value.substring(0, 2), 10);
  const month = parseInt(value.substring(2, 4), 10);
  const day = parseInt(value.substring(4, 6), 10);
  return month >= 1 && month <= 12 && day >= 1 && day <= 31;
};

Then check in handleProceed before navigating:

if (selectedMethod === 'mrzCorrection') {
  if (!isValidYYMMDD(dateOfBirth) || !isValidYYMMDD(dateOfExpiry)) {
    setError('Please enter valid dates in YYMMDD format.');
    return;
  }
}
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2615f0b and 0d7e7c7.

📒 Files selected for processing (3)
  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (2 hunks)
  • app/tests/utils/nfcScanner.test.ts (0 hunks)
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • app/tests/utils/nfcScanner.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx
app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (app/AGENTS.md)

Type checking must pass before PRs (yarn types)

Files:

  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx
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/document/DocumentNFCMethodSelectionScreen.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-26T14:49:11.190Z
Learnt from: shazarre
PR: selfxyz/self#936
File: app/src/screens/passport/PassportNFCScanScreen.tsx:28-31
Timestamp: 2025-08-26T14:49:11.190Z
Learning: SelfClientProvider is wrapped in app/App.tsx, providing context for useSelfClient() hook usage throughout the React Native app navigation stacks.

Applied to files:

  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx
🧬 Code graph analysis (1)
app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (1)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)
  • useMRZStore (40-60)
🔇 Additional comments (1)
app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (1)

98-107: LGTM! Clean MRZ store integration.

The hook initialization pattern is correct and follows the SDK's intended usage.

};

export const scan = async (inputs: Inputs) => {
await configureNfcAnalytics();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now called before calling scan as preparation to move the code over to the SDK

@cursor
Copy link

cursor bot commented Oct 2, 2025

You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 17.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)

27-27: Remove or clarify the TODO comment.

Based on prior feedback, environment-based defaults are not appropriate for a production app handling PII. Empty string defaults are correct for MRZ fields.

Apply this diff to remove the TODO:

-// TODO: what about the defaults from @env?
 const initialState = {

Alternatively, if environment defaults are still under consideration, update the comment to explain why:

-// TODO: what about the defaults from @env?
+// NOTE: Empty defaults are intentional; MRZ data should not be pre-filled for security/privacy.
 const initialState = {
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ceced8 and a7ce32f.

📒 Files selected for processing (1)
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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/stores/mrzStore.tsx
**/*.{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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • packages/mobile-sdk-alpha/src/stores/mrzStore.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/stores/mrzStore.tsx
🧠 Learnings (1)
📚 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/stores/mrzStore.tsx
⏰ 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: e2e-ios
  • GitHub Check: build-deps
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (3)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (3)

7-25: Interface design is clean and appropriate for MRZ data.

The MRZState interface correctly models the MRZ fields as strings (matching typical MRZ format). The three actions (setMRZForNFC, clearMRZ, update) provide clear, focused mutations.

Important reminder: These fields contain PII (passport numbers, dates of birth). Ensure consuming code never logs or exposes these values in analytics, error reports, or debugging output.


36-39: Export constraint is correctly documented.

The warning about not exporting useMRZStore outside the SDK is critical to prevent multiple Zustand store instances. This aligns with best practices and the SDK's architecture (interaction via Self client).


40-60: Store implementation is correct and follows Zustand best practices.

The three actions are implemented cleanly:

  • setMRZForNFC explicitly sets all fields (safe approach).
  • clearMRZ resets to initialState.
  • update merges partial state (standard pattern).

Verify persistence decision: The store does not use Zustand's persist middleware, meaning MRZ data is ephemeral (lost on app restart). This is appropriate for PII security, but confirm this aligns with product requirements (e.g., does NFC scanning flow require MRZ data to survive app restarts?).

If persistence is unintentionally missing, consider adding encrypted persistence:

import { create } from 'zustand';
import { persist, createJSONStorage } from 'zustand/middleware';
import EncryptedStorage from 'react-native-encrypted-storage'; // Example

export const useMRZStore = create<MRZState>()(
  persist(
    set => ({
      ...initialState,
      setMRZForNFC: data => { /* ... */ },
      clearMRZ: () => { /* ... */ },
      update: (patch) => { /* ... */ },
    }),
    {
      name: 'mrz-storage',
      storage: createJSONStorage(() => EncryptedStorage),
    }
  )
);

Only add persistence if product requirements dictate it and encryption is available.

@shazarre shazarre force-pushed the shazarre/separate_MRZ_data_into_separate_store branch from a7ce32f to 8d9074b Compare October 2, 2025 17:39
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/src/screens/document/DocumentNFCScanScreen.tsx (2)

262-445: Validate MRZ fields before initiating NFC scan.

The onVerifyPress handler proceeds to scan even if MRZ fields (passportNumber, dateOfBirth, dateOfExpiry) are empty strings. This will cause the NFC scan to fail with less helpful error messages.

Add validation before calling scan():

 const onVerifyPress = useCallback(async () => {
   buttonTap();
   if (isNfcEnabled) {
+    // Validate required MRZ fields
+    if (!passportNumber || !dateOfBirth || !dateOfExpiry) {
+      openErrorModal('Missing passport information. Please scan the MRZ first.');
+      return;
+    }
+
     logNFCEvent('info', 'verify_pressed', {
       ...baseContext,
       stage: 'ui',
     });

278-321: Remove duplicate timeout setup code.

Lines 278-293 and 298-321 contain nearly identical timeout setup logic. This duplication makes the code harder to maintain and could lead to race conditions with multiple timeouts being set.

Consolidate the timeout logic:

     // Mark NFC scanning as active to prevent analytics flush interference
     setNfcScanningActive(true);
 
+    // Setup single timeout for scan
     if (scanTimeoutRef.current) {
       clearTimeout(scanTimeoutRef.current);
       scanTimeoutRef.current = null;
     }
     scanTimeoutRef.current = setTimeout(() => {
       scanCancelledRef.current = true;
+      setNfcScanningActive(false);
       trackEvent(PassportEvents.NFC_SCAN_FAILED, {
         error: 'timeout',
       });
+      trackNfcEvent(PassportEvents.NFC_SCAN_FAILED, {
+        error: 'timeout',
+      });
       logNFCEvent('warn', 'scan_timeout', {
         ...baseContext,
         stage: 'timeout',
       });
       openErrorModal('Scan timed out. Please try again.');
       setIsNfcSheetOpen(false);
       logNFCEvent('info', 'sheet_close', {
         ...baseContext,
         stage: 'ui',
       });
     }, 30000);
-
-    if (scanTimeoutRef.current) {
-      clearTimeout(scanTimeoutRef.current);
-      scanTimeoutRef.current = null;
-    }
-    scanTimeoutRef.current = setTimeout(() => {
-      scanCancelledRef.current = true;
-      setNfcScanningActive(false); // Clear scanning state on timeout
-      trackEvent(PassportEvents.NFC_SCAN_FAILED, {
-        error: 'timeout',
-      });
-      trackNfcEvent(PassportEvents.NFC_SCAN_FAILED, {
-        error: 'timeout',
-      });
-      logNFCEvent('warn', 'scan_timeout', {
-        ...baseContext,
-        stage: 'timeout',
-      });
-      openErrorModal('Scan timed out. Please try again.');
-      setIsNfcSheetOpen(false);
-      logNFCEvent('info', 'sheet_close', {
-        ...baseContext,
-        stage: 'ui',
-      });
-    }, 30000);
 
     try {
🧹 Nitpick comments (2)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (2)

43-51: Consider adding validation for MRZ data formats.

The setMRZForNFC action directly sets all fields without validation. Given that MRZ data has specific formats (e.g., passport numbers, date formats YYMMDD, country codes ISO 3166-1 alpha-3), consider adding validation to catch errors early.

Example validation patterns:

  • dateOfBirth and dateOfExpiry: YYMMDD format (6 digits)
  • passportNumber: alphanumeric, length constraints vary by country
  • countryCode: 3-letter ISO code
  • documentType: typically 1-2 characters (P, ID, etc.)

This would prevent downstream NFC scan failures due to malformed input.


57-59: The generic update action bypasses validation.

While update provides flexibility, it allows partial state updates that bypass any validation logic that might exist in setMRZForNFC. This could lead to inconsistent state if invalid data is patched in.

Consider either:

  1. Adding validation within update that checks each field in the patch
  2. Documenting that update is for internal SDK use only and external callers should use setMRZForNFC
  3. Removing update if it's not essential, to enforce validation through setMRZForNFC
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a7ce32f and 8d9074b.

📒 Files selected for processing (9)
  • app/src/screens/document/DocumentCameraScreen.tsx (3 hunks)
  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx (2 hunks)
  • app/src/screens/document/DocumentNFCScanScreen.tsx (4 hunks)
  • app/src/stores/userStore.ts (0 hunks)
  • app/src/utils/nfcScanner.ts (0 hunks)
  • app/tests/utils/nfcScanner.test.ts (0 hunks)
  • packages/mobile-sdk-alpha/src/client.ts (2 hunks)
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1 hunks)
  • packages/mobile-sdk-alpha/src/types/public.ts (2 hunks)
💤 Files with no reviewable changes (3)
  • app/tests/utils/nfcScanner.test.ts
  • app/src/utils/nfcScanner.ts
  • app/src/stores/userStore.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • app/src/screens/document/DocumentNFCMethodSelectionScreen.tsx
  • app/src/screens/document/DocumentCameraScreen.tsx
  • packages/mobile-sdk-alpha/src/client.ts
  • packages/mobile-sdk-alpha/src/types/public.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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., ***-***-1234 for passport numbers, J*** D*** for names).

Files:

  • app/src/screens/document/DocumentNFCScanScreen.tsx
  • packages/mobile-sdk-alpha/src/stores/mrzStore.tsx
app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (app/AGENTS.md)

Type checking must pass before PRs (yarn types)

Files:

  • app/src/screens/document/DocumentNFCScanScreen.tsx
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/document/DocumentNFCScanScreen.tsx
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/stores/mrzStore.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/stores/mrzStore.tsx
🧠 Learnings (2)
📓 Common learnings
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/src/processing/** : Place MRZ processing helpers in packages/mobile-sdk-alpha/src/processing/
📚 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/stores/mrzStore.tsx
🧬 Code graph analysis (1)
app/src/screens/document/DocumentNFCScanScreen.tsx (2)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (1)
  • useMRZStore (40-60)
app/src/utils/analytics.ts (1)
  • configureNfcAnalytics (212-234)
⏰ 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: e2e-ios
  • GitHub Check: build-deps
  • GitHub Check: analyze-android
  • GitHub Check: analyze-ios
🔇 Additional comments (4)
packages/mobile-sdk-alpha/src/stores/mrzStore.tsx (2)

7-25: LGTM! Clean interface design.

The MRZState interface clearly defines the required fields for NFC scanning and provides appropriate actions. The type signatures are explicit and self-documenting.


27-34: Clarify the TODO regarding @env defaults.

The TODO comment suggests there may be environment-based defaults for MRZ fields. If these defaults are intended for development/testing purposes, they should be documented or implemented. If not needed, remove the TODO to avoid confusion.

Do these fields need default values from environment variables for testing/development? If so, please clarify the use case and implement accordingly. If not, remove the TODO comment.

app/src/screens/document/DocumentNFCScanScreen.tsx (2)

95-107: LGTM! Clean migration to SDK MRZ store.

The destructuring of useMRZStore from selfClient and subsequent field access is clean and follows React hooks patterns correctly.


327-327: Good placement of analytics configuration.

Calling configureNfcAnalytics() before the scan operation ensures analytics are properly initialized. The async/await is correctly handled within the try-catch block.

@shazarre shazarre merged commit c1d30d1 into dev Oct 2, 2025
28 checks passed
@shazarre shazarre deleted the shazarre/separate_MRZ_data_into_separate_store branch October 2, 2025 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants