Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds duplicate-identity detection and recovery: document-number normalization across services, a verification lookup-by-document endpoint, recovery fallback that backfills binding documents and provisioning signatures, onboarding UI handling for duplicate results, and tests covering lookup and recovery flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/UI
participant Onboarding as Onboarding Page
participant ProvisionSvc as Provisioning Service
participant DiditAPI as DIDIT API
participant VerifSvc as Verification Service
Client->>Onboarding: Start provisioning (Didit session)
Onboarding->>ProvisionSvc: Begin provisioning flow with sessionId
ProvisionSvc->>DiditAPI: Fetch decision for sessionId
DiditAPI-->>ProvisionSvc: Decision (approved/declined/in_review/duplicate)
alt Decision is duplicate
ProvisionSvc->>VerifSvc: GET /verification/v2/lookup-by-document/:sessionId
VerifSvc-->>ProvisionSvc: { success:true, existingW3id, documentNumber } or { success:false }
ProvisionSvc-->>Onboarding: set diditResult = "duplicate" + duplicate data
Onboarding->>Client: Render duplicate UI (recover or restart)
else Not duplicate
ProvisionSvc-->>Onboarding: Continue normal provisioning
Onboarding->>Client: Complete onboarding
end
sequenceDiagram
participant Client as Recovery Client
participant RecoveryCtrl as Recovery Controller
participant FaceSearch as Face-Search Service
participant VerifSvc as Verification Service
participant Platform as Platform Token Registry
participant eVault as eVault Service
Client->>RecoveryCtrl: POST /recovery/face-search (payload)
RecoveryCtrl->>FaceSearch: Attempt face-search
alt Face match found
FaceSearch-->>RecoveryCtrl: idVerif + w3id
RecoveryCtrl-->>Client: Respond success with idVerif
else No face match
RecoveryCtrl->>VerifSvc: Query by normalized documentNumber
VerifSvc-->>RecoveryCtrl: Candidate record with linkedEName (or none)
alt Candidate found
RecoveryCtrl->>Platform: request platform token
Platform-->>RecoveryCtrl: token
RecoveryCtrl->>eVault: fetch binding document types (w3id, token)
eVault-->>RecoveryCtrl: existing types
RecoveryCtrl->>eVault: create missing binding docs (id_document, photograph)
eVault-->>RecoveryCtrl: created
RecoveryCtrl-->>Client: Respond success with w3id and idVerif
else No candidate
RecoveryCtrl-->>Client: Respond failure / no match
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/evault-core/src/controllers/LegacyVerificationController.ts`:
- Around line 202-210: The duplicate check in LegacyVerificationController
currently only queries the normalized docNumber (trimmed/uppercased), which
misses legacy rows stored with original casing/spacing; update the
verificationService.findManyAndCount call to search for both the normalized form
and legacy/raw variants (e.g., the trimmed original string and a
case-insensitive match) so historical documentId values are matched; locate the
code around document.number.value, docNumber and the findManyAndCount call and
change the query payload to include an OR/IN condition (or a case-insensitive
comparison) on documentId to cover both normalized and legacy IDs while keeping
the existing DUPLICATES_POLICY check.
In `@infrastructure/evault-core/src/controllers/RecoveryController.ts`:
- Around line 101-122: The GraphQL mutation POSTs in RecoveryController.ts
currently treat any 200 response as success and ignore GraphQL errors; update
the code after the Axios.post call that invokes the CreateBindingDocument
mutation to inspect response.data for data.createBindingDocument.errors (and
response.data.errors if present) and if any errors exist, log them with context
(including subject and mutation name) and throw or return a failed result so the
backfill/flow doesn't silently succeed; apply the same explicit error
check/handling to both places where CreateBindingDocument is called (the two
Axios.post blocks that send input { subject, type: "id_document", data,
ownerSignature } to gqlUrl with Authorization `Bearer ${token}`) so
GraphQL-level errors are surfaced and handled.
In `@infrastructure/evault-core/src/services/ProvisioningService.ts`:
- Around line 56-61: The duplicate-check only queries
verificationService.findManyAndCount by the normalized documentId
(normalizeDocumentNumber) and misses historical non-normalized rows; update the
logic in ProvisioningService where documentNumber is handled so that after
computing normalizedDocumentNumber you either (a) query
verificationService.findManyAndCount with an OR that checks both { documentId:
normalizedDocumentNumber } and { documentId: documentNumber } or (b) if the
normalized query returns zero, run a fallback findManyAndCount using the raw
documentNumber; ensure you reference the same variables (documentNumber,
normalizedDocumentNumber) and use the existing
verificationService.findManyAndCount call so docMatches covers both normalized
and legacy raw values.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/evault-core/src/controllers/LegacyVerificationController.tsinfrastructure/evault-core/src/controllers/RecoveryController.tsinfrastructure/evault-core/src/controllers/RecoveryDocumentFallback.spec.tsinfrastructure/evault-core/src/controllers/SessionIdValidation.spec.tsinfrastructure/evault-core/src/controllers/VerificationController.tsinfrastructure/evault-core/src/controllers/VerificationLookupByDocument.spec.tsinfrastructure/evault-core/src/services/ProvisioningService.ts
infrastructure/evault-core/src/controllers/LegacyVerificationController.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrastructure/eid-wallet/src/lib/ui/BottomSheet/BottomSheet.svelte`:
- Around line 50-57: The fullscreen sheet currently uses "z-50" which places it
above the backdrop, making dismissible taps unreachable; update the class logic
in BottomSheet.svelte so that when fullScreen && dismissible the sheet does not
use a higher z-index than the backdrop (e.g., remove or lower "z-50" on the
fullscreen branch and ensure the backdrop element has the higher z-index),
keeping interactive children working (you can scope a pointer-events-auto to
inner content if needed); adjust the conditional that builds the fullScreen
class string and the backdrop's z-index so taps hit the backdrop when
dismissible is true.
In
`@infrastructure/eid-wallet/src/routes/`(app)/scan-qr/components/AuthDrawer.svelte:
- Around line 32-37: The BottomSheet dialog rendered in AuthDrawer (the
BottomSheet component instance using isOpen={internalOpen}) lacks an accessible
name; add an aria-labelledby prop on the BottomSheet pointing to the id of the
heading inside the sheet (create a unique id like auth-drawer-title and assign
it to the heading element used as the dialog title), and ensure both BottomSheet
instances (the one at the top and the other at lines ~58-60) use the
corresponding aria-labelledby IDs so the dialog is properly labeled for
assistive technologies.
In
`@infrastructure/eid-wallet/src/routes/`(app)/scan-qr/components/RevealDrawer.svelte:
- Around line 32-37: The BottomSheet dialog in RevealDrawer.svelte is missing
accessible labeling; update the BottomSheet invocation(s) (the component used
with isOpen={internalOpen}) to include an aria-labelledby or aria-label prop and
bind it to the rendered title element: give the title element (e.g., the hX that
displays the drawer title) a stable id (like revealDrawerTitle) and set
aria-labelledby to that id on the BottomSheet; apply the same pattern to the
other BottomSheet usages in this file where a title is rendered so screen
readers get proper context.
In `@infrastructure/eid-wallet/src/routes/`(auth)/e-passport/+page.svelte:
- Around line 18-27: The recovery finalization currently commits side effects
(setting RECOVERY_SKIP_PROFILE_SETUP_KEY in localStorage, updating
globalState.vaultController.vault, and clearing pendingRecovery) before awaited
operations complete, risking partial state on failure; change the flow so you
first await globalState.walletSdkAdapter.ensureKey("default","onboarding") and
await globalState.vaultController.syncPublicKey(recovery.ename), then only after
both awaits succeed assign globalState.vaultController.vault = {uri:
recovery.uri, ename: recovery.ename}, set
localStorage.setItem(RECOVERY_SKIP_PROFILE_SETUP_KEY, "true"), and call
pendingRecovery.set(null) so all side effects are committed atomically;
reference these symbols: RECOVERY_SKIP_PROFILE_SETUP_KEY,
globalState.walletSdkAdapter.ensureKey, globalState.vaultController.vault,
globalState.vaultController.syncPublicKey, pendingRecovery.set.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
infrastructure/eid-wallet/package.jsoninfrastructure/eid-wallet/src/lib/ui/BottomSheet/BottomSheet.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/AuthDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/LoggedInDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/RevealDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SigningDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SocialBindingDrawer.svelteinfrastructure/eid-wallet/src/routes/(auth)/e-passport/+page.svelte
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Improvements
Tests