Skip to content

move common aadhaar functions to msdk#1200

Merged
aaronmgdr merged 3 commits intodevfrom
aaronmgdr/move-aadhaar
Oct 3, 2025
Merged

move common aadhaar functions to msdk#1200
aaronmgdr merged 3 commits intodevfrom
aaronmgdr/move-aadhaar

Conversation

@aaronmgdr
Copy link
Contributor

@aaronmgdr aaronmgdr commented Oct 3, 2025

also

  • add info that functions are duplicated and to use the msdk one
  • use listeners to navigate
  • improve/fix the Sdk event descriptions

Summary by CodeRabbit

  • New Features
    • Added Aadhaar QR upload flow with clear success and error outcomes.
    • Introduced dedicated success and error screens with contextual messaging.
  • Improvements
    • Enhanced error feedback for Aadhaar QR issues (including expired codes) with clearer titles and descriptions.
    • More reliable QR processing and storage via a unified flow, improving consistency across the app.
  • Documentation
    • Updated guidance to mark certain document selection events as required.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 2025

Walkthrough

Adds Aadhaar QR onboarding flow via a new SDK hook, centralizes error messaging, wires new success/failure events with navigation handlers, updates event typings, refactors Aadhaar screens to use the hook, and introduces duplicate public exports in passportDataProvider.

Changes

Cohort / File(s) Summary
Aadhaar onboarding hook and helpers
packages/mobile-sdk-alpha/src/flows/onboarding/import-aadhaar.ts
Adds useAadhaar hook with processAadhaarQRCode, validation pipeline, storage, analytics, and event emission; introduces getErrorMessages('general' | 'expired').
SDK event types
packages/mobile-sdk-alpha/src/types/events.ts
Adds PROVING_AADHAAR_UPLOAD_SUCCESS and PROVING_AADHAAR_UPLOAD_FAILURE to SdkEvents and SDKEventMap; updates some doc comments.
App listeners for navigation
app/src/providers/selfClientProvider.tsx
Appends listeners for Aadhaar upload success/failure; navigates to success or error screens (passes errorType on failure).
Aadhaar upload screen refactor
app/src/screens/document/aadhaar/AadhaarUploadScreen.tsx
Removes local QR parse/validate/store logic; adopts useAadhaar.processAadhaarQRCode; simplifies dependencies.
Aadhaar error screen messaging
app/src/screens/document/aadhaar/AadhaarUploadErrorScreen.tsx
Replaces inline branching with imported getErrorMessages(errorType).
Passport data provider exports
app/src/providers/passportDataProvider.tsx
Adds duplicate public exports for storeDocumentWithDeduplication and storePassportData (wrapper), altering module surface with duplicated declarations.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant AUS as AadhaarUploadScreen
  participant H as useAadhaar Hook
  participant V as Validators
  participant P as passportDataProvider
  participant C as SDK Client
  participant SCP as SelfClientProvider
  participant Nav as Navigation

  U->>AUS: Select QR image
  AUS->>H: processAadhaarQRCode(qrText)
  H->>V: validate format + parse + timestamp
  alt Valid
    H->>P: storePassportData(AadhaarData)
    alt Store OK
      H->>C: emit PROVING_AADHAAR_UPLOAD_SUCCESS
      C-->>SCP: Event dispatched
      SCP->>Nav: Navigate to AadhaarUploadSuccess
    else Store Error
      H->>C: emit PROVING_AADHAAR_UPLOAD_FAILURE{general}
      C-->>SCP: Event dispatched
      SCP->>Nav: Navigate to AadhaarUploadError{general}
    end
  else Invalid or Expired
    H->>C: emit PROVING_AADHAAR_UPLOAD_FAILURE{expired|general}
    C-->>SCP: Event dispatched
    SCP->>Nav: Navigate to AadhaarUploadError{expired|general}
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–70 minutes

  • New hook introducing end-to-end control flow, validation, storage, and analytics.
  • Event surface expansion and app-level listeners altering navigation.
  • Screen refactors to consume the hook.
  • Duplicate exports in data provider require careful resolution to avoid conflicts.

Possibly related PRs

Suggested reviewers

  • remicolin

Poem

A QR hums, a hook takes flight,
Parsing threads through day and night.
Success rings out, the screens align,
Errors whisper, still by design.
Events now guide the user’s way—
Aadhaar arrives without delay. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly captures the primary intent of the changeset, which is to relocate common Aadhaar-related functions into the mobile SDK (msdk), and is concise and specific enough for team members to understand the main objective at a glance. It avoids unnecessary detail and focuses on the core refactoring effort.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch aaronmgdr/move-aadhaar

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.

@aaronmgdr
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 3, 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

Caution

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

⚠️ Outside diff range comments (1)
app/src/screens/document/aadhaar/AadhaarUploadScreen.tsx (1)

114-129: Stop overriding SDK-driven Aadhaar failure navigation.

processAadhaarQRCode now raises PROVING_AADHAAR_UPLOAD_FAILURE with an errorType. This catch immediately calls navigation.navigate(..., { errorType: 'general' }), so the listener we just wired in SelfClientProvider loses the specific 'expired' | 'general' payload and the user only sees the generic copy. Gate this branch to library-scan errors (e.g. “No QR code found”) and let the SDK listener drive navigation for processing failures.

-      if (
-        errorMessage.includes('No QR code found') ||
-        errorMessage.includes('QR code') ||
-        errorMessage.includes('Failed to process') ||
-        errorMessage.includes('Invalid')
-      ) {
-        (navigation.navigate as any)('AadhaarUploadError', {
-          errorType: 'general' as const,
-        });
-        return;
-      }
-
-      // Handle any other errors by showing error screen
-      (navigation.navigate as any)('AadhaarUploadError', {
-        errorType: 'general' as const,
-      });
+      if (errorMessage.includes('No QR code found')) {
+        (navigation.navigate as any)('AadhaarUploadError', {
+          errorType: 'general' as const,
+        });
+        return;
+      }
+
+      // Let PROVING_AADHAAR_UPLOAD_FAILURE listener handle SDK processing errors so we keep its errorType payload.
+      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 429d9f9 and b136afa.

📒 Files selected for processing (6)
  • app/src/providers/passportDataProvider.tsx (2 hunks)
  • app/src/providers/selfClientProvider.tsx (1 hunks)
  • app/src/screens/document/aadhaar/AadhaarUploadErrorScreen.tsx (2 hunks)
  • app/src/screens/document/aadhaar/AadhaarUploadScreen.tsx (3 hunks)
  • packages/mobile-sdk-alpha/src/flows/onboarding/import-aadhaar.ts (1 hunks)
  • packages/mobile-sdk-alpha/src/types/events.ts (3 hunks)
🧰 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.tsx
  • app/src/screens/document/aadhaar/AadhaarUploadErrorScreen.tsx
  • app/src/screens/document/aadhaar/AadhaarUploadScreen.tsx
  • app/src/providers/passportDataProvider.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/import-aadhaar.ts
  • packages/mobile-sdk-alpha/src/types/events.ts
🧠 Learnings (1)
📚 Learning: 2025-09-10T14:47:40.945Z
Learnt from: shazarre
PR: selfxyz/self#1041
File: app/src/providers/passportDataProvider.tsx:297-301
Timestamp: 2025-09-10T14:47:40.945Z
Learning: In app/src/providers/passportDataProvider.tsx: The deleteDocumentDirectlyFromKeychain function is a low-level utility used by the DocumentsAdapter and should not include error handling since callers like deleteDocument() already implement appropriate try/catch with logging for Keychain operations.

Applied to files:

  • app/src/providers/passportDataProvider.tsx

Comment on lines +39 to +47
const currentTimestamp = new Date().getTime();
const timestampDate = new Date(timestamp);
const timestampTimestamp = timestampDate.getTime();
const diff = currentTimestamp - timestampTimestamp;
const diffMinutes = diff / (1000 * 60);

const allowedWindow = await getAadharRegistrationWindow();
const isValid = diffMinutes <= allowedWindow;

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix timestamp parsing to avoid false expiry

new Date(timestamp) cannot reliably parse the 'YYYY-MM-DD HH:MM' string we get from the QR data. On React Native’s JavaScriptCore (iOS) this returns Invalid Date, so every valid QR fails the window check and is reported as expired. Normalize the string to ISO (e.g. replace the space with T) and guard against unparseable or missing values before computing the diff.

-      const timestampDate = new Date(timestamp);
-      const timestampTimestamp = timestampDate.getTime();
-      const diff = currentTimestamp - timestampTimestamp;
-      const diffMinutes = diff / (1000 * 60);
+      if (!timestamp) {
+        selfClient.trackEvent(AadhaarEvents.TIMESTAMP_VALIDATION_FAILED);
+        return false;
+      }
+
+      const isoTimestamp = timestamp.replace(' ', 'T');
+      const timestampDate = new Date(isoTimestamp);
+      const timestampTimestamp = timestampDate.getTime();
+      if (Number.isNaN(timestampTimestamp)) {
+        selfClient.trackEvent(AadhaarEvents.TIMESTAMP_VALIDATION_FAILED);
+        return false;
+      }
+
+      const diff = currentTimestamp - timestampTimestamp;
+      const diffMinutes = diff / (1000 * 60);
📝 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.

Suggested change
const currentTimestamp = new Date().getTime();
const timestampDate = new Date(timestamp);
const timestampTimestamp = timestampDate.getTime();
const diff = currentTimestamp - timestampTimestamp;
const diffMinutes = diff / (1000 * 60);
const allowedWindow = await getAadharRegistrationWindow();
const isValid = diffMinutes <= allowedWindow;
const currentTimestamp = new Date().getTime();
if (!timestamp) {
selfClient.trackEvent(AadhaarEvents.TIMESTAMP_VALIDATION_FAILED);
return false;
}
const isoTimestamp = timestamp.replace(' ', 'T');
const timestampDate = new Date(isoTimestamp);
const timestampTimestamp = timestampDate.getTime();
if (Number.isNaN(timestampTimestamp)) {
selfClient.trackEvent(AadhaarEvents.TIMESTAMP_VALIDATION_FAILED);
return false;
}
const diff = currentTimestamp - timestampTimestamp;
const diffMinutes = diff / (1000 * 60);
const allowedWindow = await getAadharRegistrationWindow();
const isValid = diffMinutes <= allowedWindow;

@aaronmgdr aaronmgdr merged commit a00cb7e into dev Oct 3, 2025
28 checks passed
@aaronmgdr aaronmgdr deleted the aaronmgdr/move-aadhaar branch October 3, 2025 12:37
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.

1 participant