Feat/wallet more verification docs support #357
Conversation
WalkthroughSplits Drawer lifecycle into separate init and open/close effects; adds document-type selection UI and state; implements back-side document capture, retake and upload error handling; defers selfie progression to SSE/websocket; tightens websocket typing and key-manager guards; updates pnpm built-dependencies and version bump. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Drawer
participant VerifyPage
participant Stores as Stores (verifStep, documentType, DocFront/DocBack...)
participant Camera
participant Backend as Backend/SSE
rect rgba(200,220,255,0.12)
note right of Drawer: Init vs open/close split
Drawer->>Drawer: onMount -> create CupertinoPane (init)
Drawer-->>Drawer: onDestroy -> destroy pane
Drawer->>Drawer: reactive open state -> present / destroy (post-init)
end
rect rgba(220,255,220,0.12)
User->>VerifyPage: Open verification
VerifyPage->>Stores: verifStep = 0
VerifyPage->>User: Render DocumentType
User->>VerifyPage: Select document type
VerifyPage->>Stores: documentType = chosen / verifStep = 1
end
rect rgba(255,245,200,0.12)
VerifyPage->>Camera: Start stream
User->>Camera: Capture front -> upload -> Stores.DocFront
alt documentType != passport
User->>Camera: Capture back -> upload -> Stores.DocBack
end
User->>VerifyPage: Continue to selfie
VerifyPage->>Camera: Stop stream
Note right of VerifyPage: Await SSE/websocket to advance verifStep
end
rect rgba(255,220,220,0.12)
VerifyPage->>Backend: Subscribe websocket/SSE
Backend-->>VerifyPage: Events (progress, duplicate {w3id}, completed)
alt Duplicate
VerifyPage->>Stores: reset DocFront/DocBack/Selfie, set verifStep per duplicate flow
else Normal
Backend-->>Stores: set verifStep to next step(s)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
sosweetham
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/selfie.svelte (1)
65-67: Progression via SSE/WebSocket only — add a fallbackGood to defer step advancement to server events. Consider a timeout fallback (e.g., show “Retry” or re-enable capture) if no event arrives to avoid UI dead-ends.
Confirm the event that sets verifStep=3 is always emitted after PATCH; if not, add a fallback timer.
infrastructure/eid-wallet/src/routes/(auth)/verify/store.ts (1)
11-11: documentType union looks good; consider centralizing the typeOptional: export a DocumentType type and reuse it across components to avoid union drift.
export type DocumentType = "passport" | "id" | "permit" | "dl"; export const documentType = writable<DocumentType | null>(null);infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (2)
51-94: Camera selection complexity (torch probing) likely unnecessaryYou detect torch support but never enable it; iterating all devices with getUserMedia can be heavy and prompt repeatedly on some browsers. Consider simplifying to facingMode: "environment" and avoid per-device streams. If torch is needed, use applyConstraints when available.
// Simplified approach async function getMainCameraStream() { return navigator.mediaDevices.getUserMedia({ video: { facingMode: "environment" } }); }Ensure iOS/Safari behavior meets your UX with the simpler constraint; otherwise, add torch via track.applyConstraints({ advanced: [{ torch: true }] }) when low-light is detected.
141-141: Remove debug logs or guard behind dev flagconsole.log in production UI can be noisy.
- console.log("Switched to back image capture, image =", image); + if (import.meta.env?.DEV) console.debug("Switched to back image capture", { image });Also applies to: 173-173
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
272-279: Encode w3id when building the resolve URLUse URLSearchParams to avoid malformed queries if w3id contains special characters.
- const response = await axios.get( - new URL( - `resolve?w3id=${existingW3id}`, - PUBLIC_REGISTRY_URL, - ).toString(), - ); + const url = new URL("resolve", PUBLIC_REGISTRY_URL); + url.searchParams.set("w3id", existingW3id); + const response = await axios.get(url.toString());
118-145: Close the EventSource on component destroy to prevent leaksPersist the EventSource and close it in onDestroy.
- import { getContext, onMount } from "svelte"; + import { getContext, onMount, onDestroy } from "svelte";+ let verificationEventSource: EventSource | null = null;- const eventSource = new EventSource(sseUrl); + verificationEventSource = new EventSource(sseUrl);- eventSource.onopen = () => { + verificationEventSource.onopen = () => { @@ - eventSource.onmessage = (e) => { + verificationEventSource.onmessage = (e) => {+ onDestroy(() => { + verificationEventSource?.close(); + verificationEventSource = null; + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
infrastructure/eid-wallet/src-tauri/gen/android/.idea/kotlinc.xmlis excluded by!**/gen/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelte(2 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte(7 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/steps/document-type.svelte(1 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte(3 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/steps/selfie.svelte(1 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/store.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check Code
package.json
[error] 1-1: turbo run check failed: Failed to add workspace 'rest-express'; it already exists at 'platforms/dreamSync/package.json'.
🪛 GitHub Actions: Check Format
package.json
[error] 1-1: Failed to add workspace 'rest-express' from 'platforms/dreamSync/package.json'; it already exists at 'platforms/marketplace/package.json'.
🪛 GitHub Actions: Check Lint
package.json
[error] 1-1: Turbo: Failed to add workspace 'rest-express' from 'platforms/dreamSync/package.json'; it already exists at 'platforms/marketplace/package.json'.
🔇 Additional comments (8)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/document-type.svelte (1)
1-50: LGTM — clear UX and state flowSelecting a document type updates the store and advances to capture step. Matches store union.
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (2)
118-134: Good: front image upload now wrapped in try/catch with user-facing errorError path halts loading and provides feedback. Solid improvement.
144-177: Good: back image capture/upload with error handling and state updatesBack side flow and previews align with non-passport types.
infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelte (1)
30-50: Confirm Svelte 5 migration and legacy syntax support
Svelte ^5.0.0 is declared and runes ($effect, $props, $state, $derived, $bindable) are used widely, yet legacyexport letremains in many *.svelte files. Ensure your build config supports both syntaxes or complete migration to runes to avoid build errors.infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (4)
22-22: Good: reset back-side doc and selfie on resubmissionDocBack reset complements DocFront/Selfie resets to avoid stale state. LGTM.
Also applies to: 140-141
105-105: Narrowed websocketData type is saferTyping to { w3id?: string } | null is an improvement. Please confirm the backend always includes w3id for duplicate events.
188-191: Key manager guards and init check look goodExplicit not-initialized throws and the exists→generate flow reduce runtime surprises. LGTM.
Also applies to: 207-210, 232-236
143-144: Confirm step index change to 3Jumping to step 3 on any message bypasses step 2. Verify this aligns with the updated step model (DocumentType=0, Passport=1, Selfie=2, Results=3) for approved/duplicate/resubmission flows.
| // Handle open/close state separately | ||
| $effect(() => { | ||
| if (!pane) return; | ||
| if (isPaneOpen) { | ||
| pane.present({ animate: true }); | ||
| } else { | ||
| pane.destroy({ animate: true }); | ||
| } | ||
| return () => pane.destroy(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Destroying pane on close prevents reopening (pane not re-initialized)
Calling pane.destroy() when isPaneOpen=false removes the instance; later openings won’t recreate it because the init effect doesn’t rerun. Use hide/close (non-destructive) on close, and reserve destroy for unmount.
Apply:
- if (isPaneOpen) {
- pane.present({ animate: true });
- } else {
- pane.destroy({ animate: true });
- }
+ if (isPaneOpen) {
+ pane.present({ animate: true });
+ } else {
+ // Prefer non-destructive close
+ if (typeof (pane as any).hide === "function") {
+ (pane as any).hide({ animate: true });
+ } else {
+ // As a fallback, keep the instance; avoid destroy here
+ }
+ }Option B: if hide isn’t available, gate destroy to unmount and re-init pane when reopening.
🌐 Web query:
Does CupertinoPane expose a non-destructive method (e.g., hide/close/dismiss) to close the pane without destroying it?
💡 Result:
Yes. CupertinoPane provides a non‑destructive hide() that removes the pane from view but keeps it in the DOM (you can check isHidden() and call present() to show it again). [1][2]
Sources:
- GitHub docs (public methods: hide, destroy, isHidden). [1]
- npm package docs (showing hide(), destroy(), present(), isHidden()). [2]
Replace pane.destroy() with pane.hide() on close
CupertinoPane provides hide(), which non-destructively closes the pane and lets you call present() to reopen it.
File: infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelte lines 51–60
- if (isPaneOpen) {
- pane.present({ animate: true });
- } else {
- pane.destroy({ animate: true });
- }
+ if (isPaneOpen) {
+ pane.present({ animate: true });
+ } else {
+ pane.hide({ animate: true });
+ }📝 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.
| // Handle open/close state separately | |
| $effect(() => { | |
| if (!pane) return; | |
| if (isPaneOpen) { | |
| pane.present({ animate: true }); | |
| } else { | |
| pane.destroy({ animate: true }); | |
| } | |
| return () => pane.destroy(); | |
| }); | |
| // Handle open/close state separately | |
| $effect(() => { | |
| if (!pane) return; | |
| if (isPaneOpen) { | |
| pane.present({ animate: true }); | |
| } else { | |
| pane.hide({ animate: true }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In infrastructure/eid-wallet/src/lib/ui/Drawer/Drawer.svelte around lines 51 to
60, the code currently calls pane.destroy({ animate: true }) when closing;
replace that with pane.hide({ animate: true }) so the pane is closed
non-destructively and can be reopened with pane.present(), keeping the existing
null check for pane; ensure the call signature matches the CupertinoPane API
(use hide with the animate option) and remove any logic that assumes the pane is
destroyed.
| import DocumentType from "./steps/document-type.svelte"; | ||
| import Passport from "./steps/passport.svelte"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Generalize UI and stored labels for non-passport docs
Hardcoded “Passport” persists in user metadata and UI, which mislabels non-passport flows. Import documentType, add a label map, and switch labels to be document‑type aware. Also generalize the hero subtitle and alt text.
Apply:
- import {
- DocFront,
- DocBack,
- Selfie as SelfiePic,
- reason,
- status,
- verifStep,
- verificaitonId,
- } from "./store";
+ import {
+ DocFront,
+ DocBack,
+ Selfie as SelfiePic,
+ reason,
+ status,
+ verifStep,
+ verificaitonId,
+ documentType,
+ } from "./store";- let hardwareKeyCheckComplete = $state(false);
+ let hardwareKeyCheckComplete = $state(false);
+ const docLabel = {
+ passport: "Passport",
+ id: "National ID",
+ permit: "Residence Permit",
+ dl: "Driver's License",
+ } as const;- "ID submitted": `Passport - ${person.nationality.value}`,
- "Passport Number": document.number.value,
+ "ID submitted": `${docLabel[$documentType ?? "passport"]} - ${person.nationality.value}`,
+ "Document Number": document.number.value,- Get your passport ready. You’ll be directed to present your
- passport and take a quick selfie.
+ Get your document ready. You’ll be directed to present your
+ chosen ID and take a quick selfie.- <img class="mx-auto mt-20" src="images/Passport.svg" alt="passport" />
+ <img class="mx-auto mt-20" src="images/Passport.svg" alt="identity document" />Also applies to: 358-362
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte
Show resolved
Hide resolved
| export const verifStep = writable(0); | ||
| export const permissionGranted = writable<boolean>(false); | ||
| export const DocFront = writable<string | null>(); | ||
| export const DocBack = writable<string | null>(); |
There was a problem hiding this comment.
writable requires an initial value
This will fail type-checking and may break at runtime. Initialize to null.
-export const DocBack = writable<string | null>();
+export const DocBack = writable<string | null>(null);Also align adjacent stores for consistency:
// Outside this hunk but recommended
export const DocFront = writable<string | null>(null);
export const Selfie = writable<string | null>(null);
export const status = writable<string>("");
export const reason = writable<string>("");🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(auth)/verify/store.ts around line 6,
the DocBack writable is declared without an initial value which fails TypeScript
checks and can break at runtime; initialize it to null (writable<string |
null>(null)) and while here, make adjacent store declarations consistent by
explicitly initializing DocFront, Selfie to null and status, reason to empty
strings as shown in the comment.
| "onlyBuiltDependencies": [ | ||
| "@firebase/util", | ||
| "@tailwindcss/oxide", | ||
| "bufferutil", | ||
| "core-js", | ||
| "@biomejs/biome", | ||
| "cpu-features", | ||
| "es5-ext", | ||
| "esbuild", | ||
| "msw", | ||
| "protobufjs", | ||
| "sharp", | ||
| "ssh2", | ||
| "svelte-preprocess", | ||
| "unrs-resolver" | ||
| "svelte-preprocess" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
CI is failing: duplicate workspace name; fix before merge
Turbo reports a duplicate workspace "rest-express" (dreamSync vs marketplace). Rename one or adjust its package.json "name" to be unique, then update references.
Run to locate duplicates:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# List all package.json names and paths, highlight duplicates
fd -H -t f package.json \
| xargs -I '{}' sh -c 'printf "%s\t%s\n" "$(jq -r .name "{}")" "{}"' \
| sort \
| awk -F'\t' '{a[$1]=a[$1] ? a[$1]" | "$2 : $2} END{for (k in a) if (split(a[k], b, /\|/) > 1) printf "DUPLICATE\t%s\t%s\n", k, a[k]}'Length of output: 193
Fix duplicate workspace names
CI fails due to duplicate workspaces; ensure each name in package.json is unique:
web3-adapter(ininfrastructure/web3-adapter/package.json&services/web3-adapter/package.json)rest-express(inplatforms/dreamSync/package.json&platforms/marketplace/package.json)
Rename or adjust one instance of each before merging.
🤖 Prompt for AI Agents
In package.json around lines 25 to 34, CI is failing due to duplicate workspace
package names: web3-adapter appears in infrastructure/web3-adapter/package.json
and services/web3-adapter/package.json, and rest-express appears in
platforms/dreamSync/package.json and platforms/marketplace/package.json; fix by
renaming one of each pair (or adjusting package.json "name" fields) to unique,
descriptive names (e.g., web3-adapter-infra or web3-adapter-service,
dreamSync-rest-express or marketplace-rest-express) and update any internal
references/imports or workspace globs that rely on the old names to match the
new names before merging.
| "@biomejs/biome", | ||
| "cpu-features", | ||
| "es5-ext", | ||
| "esbuild", | ||
| "msw", | ||
| "protobufjs", | ||
| "sharp", | ||
| "ssh2", | ||
| "svelte-preprocess", | ||
| "unrs-resolver" | ||
| "svelte-preprocess" |
There was a problem hiding this comment.
🧩 Analysis chain
onlyBuiltDependencies changes look fine
The additions align with the toolchain. Ensure all entries listed here are actual deps in the repo to avoid noise.
Quick check:
🏁 Script executed:
#!/bin/bash
for p in "@biomejs/biome" "es5-ext" "svelte-preprocess"; do
rg -nP "\"$p\"" -g '**/package.json'
doneLength of output: 334
🏁 Script executed:
#!/usr/bin/env bash
python3 - << 'PYCODE'
import json
with open('package.json') as f:
data = json.load(f)
declared = set()
for scope in ['dependencies','devDependencies','peerDependencies','optionalDependencies']:
declared.update(data.get(scope, {}).keys())
pkgs = ["@biomejs/biome","cpu-features","es5-ext","esbuild","msw","protobufjs","ssh2","svelte-preprocess"]
missing = [p for p in pkgs if p not in declared]
print("Missing:", missing)
PYCODELength of output: 97
Remove or declare missing onlyBuiltDependencies entries
The following packages in onlyBuiltDependencies aren’t declared in package.json: cpu-features, es5-ext, esbuild, msw, protobufjs, ssh2, svelte-preprocess. Either add them to dependencies/devDependencies or remove them to avoid noise.
🤖 Prompt for AI Agents
In package.json around lines 26 to 33, the onlyBuiltDependencies list references
packages (cpu-features, es5-ext, esbuild, msw, protobufjs, ssh2,
svelte-preprocess) that are not declared in dependencies or devDependencies;
update package.json by either adding each referenced package to the appropriate
dependencies/devDependencies entry with sensible version specifiers (or moving
existing build-only tools to devDependencies) or remove the unused names from
onlyBuiltDependencies to keep the manifest consistent and avoid noise.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (1)
241-242: Instruction text logic is still incorrect (duplicate issue).This issue was flagged in the previous review but remains unfixed. The instruction text incorrectly uses
$DocFront ? "back" : "photo", which:
- Shows "photo" initially (when $DocFront is null), which is wrong for non-passport documents
- Shows "back" after front capture for all documents, which is wrong for passports
For passports, it should always show "photo" (single-page document). For other documents, it should show "front" initially, then "back" after the front is captured.
Apply this diff to fix the instruction text:
<p> Please place your document's - {$DocFront ? "back" : "photo"} + {#if $documentType === "passport"} + photo + {:else} + {image === 1 ? "front" : "back"} + {/if} page within the rectangle and press the take photo button </p>
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (1)
22-23: Optional: Consider using plain reactive variables for local state.
image1Capturedandimage2Capturedare only used within this component and don't need to be writable stores. Plain reactive variables would be simpler and more performant for local-only state.Apply this diff to simplify the state management:
-let image1Captured = writable(false); -let image2Captured = writable(false); +let image1Captured = false; +let image2Captured = false;Then update the usages:
- image1Captured.set(true); + image1Captured = true;- image2Captured.set(true); + image2Captured = true;- image1Captured.set(false); - image2Captured.set(false); + image1Captured = false; + image2Captured = false;And in the template:
-{#if ($documentType !== "passport" && $image1Captured && $image2Captured) || ($documentType === "passport" && $image1Captured)} +{#if ($documentType !== "passport" && image1Captured && image2Captured) || ($documentType === "passport" && image1Captured)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte(3 hunks)
🔇 Additional comments (6)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (6)
7-14: LGTM! Imports support multi-document verification.The addition of
DocBackanddocumentTypeimports correctly supports the extended verification flow for multiple document types with front/back capture.
180-208: LGTM! Camera lifecycle and retake flow are well-implemented.The helper functions properly manage camera lifecycle and enable retake functionality:
retakeImagescomprehensively resets capture statestopCameraproperly releases media trackscontinueToSelfiecleanly transitions to the next step
210-217: LGTM! Proper cleanup prevents resource leaks.The onMount cleanup properly releases camera resources when the component unmounts, following Svelte best practices.
222-224: LGTM! Improved error display.The styled error message block provides clear user feedback when upload failures occur.
229-239: LGTM! Dynamic heading supports multiple document types.The heading correctly adapts to the selected document type, enhancing user clarity.
291-320: LGTM! Conditional button logic correctly handles all document types.The button rendering logic properly handles:
- Showing "Retake" and "Continue to Selfie" when all required photos are captured (both for non-passport, single for passport)
- Showing appropriate capture button text during the capture process
| try { | ||
| await axios.post( | ||
| new URL( | ||
| `/verification/${$verificaitonId}/media`, | ||
| PUBLIC_PROVISIONER_URL, | ||
| ).toString(), | ||
| { | ||
| img: dataUrl, | ||
| type: "document-front", | ||
| }, | ||
| ); | ||
| } catch (err) { | ||
| console.error("Failed to upload front image:", err); | ||
| error = "Failed to upload image. Please try again."; | ||
| loading = false; | ||
| return; | ||
| } |
There was a problem hiding this comment.
Good error handling, but remove debug logging.
The try-catch error handling for image uploads is excellent and prevents silent failures with user-facing error messages.
However, remove the console.log statement at line 141 for production:
loading = false;
image1Captured.set(true);
-
- // Don't stop camera - keep it running for retake or back photo
if ($documentType !== "passport") {
image = 2; // Go to back image capture for non-passport documents
- console.log("Switched to back image capture, image =", image);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte
around lines 118-134, remove the debug console.error("Failed to upload front
image:", err) call from the catch block; keep the existing error = "Failed to
upload image. Please try again."; loading = false; and return statements so
user-facing handling remains, or replace the console call with a production-safe
logger/reporting call if required by the app's logging strategy.
| try { | ||
| await axios.post( | ||
| new URL( | ||
| `/verification/${$verificaitonId}/media`, | ||
| PUBLIC_PROVISIONER_URL, | ||
| ).toString(), | ||
| { | ||
| img: dataUrl, | ||
| type: "document-back", | ||
| }, | ||
| ); | ||
| } catch (err) { | ||
| console.error("Failed to upload back image:", err); | ||
| error = "Failed to upload image. Please try again."; | ||
| loading = false; | ||
| return; | ||
| } | ||
| verifStep.set(1); | ||
| loading = false; | ||
| image2Captured.set(true); | ||
| console.log("Back image captured"); | ||
| // Don't stop camera - keep it running for retake option | ||
| // Camera will be stopped when navigating away or on component cleanup | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove debug logging from back image capture.
The back image capture implementation correctly mirrors the front image logic with proper error handling.
Remove the console.log statement at line 173:
loading = false;
image2Captured.set(true);
- console.log("Back image captured");
// Don't stop camera - keep it running for retake option📝 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.
| try { | |
| await axios.post( | |
| new URL( | |
| `/verification/${$verificaitonId}/media`, | |
| PUBLIC_PROVISIONER_URL, | |
| ).toString(), | |
| { | |
| img: dataUrl, | |
| type: "document-back", | |
| }, | |
| ); | |
| } catch (err) { | |
| console.error("Failed to upload back image:", err); | |
| error = "Failed to upload image. Please try again."; | |
| loading = false; | |
| return; | |
| } | |
| verifStep.set(1); | |
| loading = false; | |
| image2Captured.set(true); | |
| console.log("Back image captured"); | |
| // Don't stop camera - keep it running for retake option | |
| // Camera will be stopped when navigating away or on component cleanup | |
| } | |
| } | |
| try { | |
| await axios.post( | |
| new URL( | |
| `/verification/${$verificaitonId}/media`, | |
| PUBLIC_PROVISIONER_URL, | |
| ).toString(), | |
| { | |
| img: dataUrl, | |
| type: "document-back", | |
| }, | |
| ); | |
| } catch (err) { | |
| console.error("Failed to upload back image:", err); | |
| error = "Failed to upload image. Please try again."; | |
| loading = false; | |
| return; | |
| } | |
| loading = false; | |
| image2Captured.set(true); | |
| // Don't stop camera - keep it running for retake option | |
| // Camera will be stopped when navigating away or on component cleanup | |
| } | |
| } |
🤖 Prompt for AI Agents
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte around
lines 154 to 177: remove the debug console.log("Back image captured") on line
173 so debug output is not left in production; keep the surrounding logic, error
handling, and state updates intact (do not alter other console.error or
functional code).
| <div class="text-center text-xs text-gray-400 mb-4 px-4"> | ||
| <!-- Accepted documents: Driver's License, Residence Permit, Passport, ID Card. --> |
There was a problem hiding this comment.
Remove commented-out code.
Line 264 contains commented-out text that should be removed to keep the codebase clean.
Apply this diff:
<div class="text-center text-xs text-gray-400 mb-4 px-4">
- <!-- Accepted documents: Driver's License, Residence Permit, Passport, ID Card. -->
{#if $documentType === "passport"}📝 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.
| <div class="text-center text-xs text-gray-400 mb-4 px-4"> | |
| <!-- Accepted documents: Driver's License, Residence Permit, Passport, ID Card. --> | |
| <div class="text-center text-xs text-gray-400 mb-4 px-4"> | |
| {#if $documentType === "passport"} |
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte
around lines 263-264, remove the commented-out HTML comment ("<!-- Accepted
documents: Driver's License, Residence Permit, Passport, ID Card. -->") so the
component doesn't contain dead/commented code; simply delete that comment line
and ensure surrounding indentation and markup remain valid.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (6)
130-130: Remove debug logging for production.The
console.errorstatement should be removed or replaced with a production-safe logging mechanism.
141-141: Remove debug logging for production.The
console.logstatement should be removed to avoid debug output in production.
166-166: Remove debug logging for production.The
console.errorstatement should be removed or replaced with a production-safe logging mechanism.
173-173: Remove debug logging for production.The
console.logstatement should be removed to avoid debug output in production.
264-264: Remove commented-out code.The commented-out HTML comment should be removed to keep the codebase clean.
241-242: Instruction text logic is still incorrect.Despite the past review comment being marked as addressed, the instruction text still uses
{$DocFront ? "back" : "photo"}which is incorrect. For passports, it should always show "photo" (single page). For other documents, it should show "front" or "back" based on the current capture state (theimagevariable), not based on whether$DocFrontexists.Apply this diff to fix the logic:
<p> - Please place your document's {$DocFront ? "back" : "photo"} page within the rectangle + Please place your document's + {#if $documentType === "passport"} + photo + {:else} + {image === 1 ? "front" : "back"} + {/if} + page within the rectangle and press the take photo button </p>
🧹 Nitpick comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (2)
241-242: Redundant instruction text in two locations.The component has instruction text at both lines 241-242 (brief) and lines 263-289 (detailed). The detailed version (lines 263-289) provides better context by showing the current capture state (front/back). Consider removing or simplifying the instruction at lines 241-242 to avoid confusion, or consolidate both into a single, clear instruction.
Also applies to: 263-289
1-1: Consider renaming the component file.The component is named
passport.sveltebut now handles multiple document types (passport, ID card, driver's license, residence permit). Consider renaming it to something more generic likedocument-capture.svelteto better reflect its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte(3 hunks)
🔇 Additional comments (3)
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (3)
221-225: Good error handling UI.The styled error message block provides clear feedback to users when uploads fail. This is a significant improvement over silent failures.
291-320: Conditional button logic is well-implemented.The button logic correctly shows different controls based on the capture state: retake/continue buttons when all required photos are captured, or a single capture button otherwise. The button text also dynamically updates based on the current capture stage.
321-375: Preview thumbnails enhance UX.The preview implementation for front/back document images is well-designed, with clear visual indicators for captured vs. pending photos. The passport exemption from previews is also correctly handled.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
17-28: ImportdocumentTypefrom the store.The
documentTypestore is still not imported despite the past review comment. This store is needed to make user-facing labels document-type aware (as suggested in the previous review for lines 252-258).Add the import:
import { DocFront, DocBack, Selfie as SelfiePic, reason, status, verifStep, verificaitonId, + documentType, } from "./store";
247-258: UsedocumentTypestore for consistent labeling.The code now handles different document types from the backend response (
document.type.value), but it doesn't use thedocumentTypestore that captures the user's selection. This creates a potential inconsistency if the backend classification differs from what the user selected.As suggested in the previous review, import and use the
documentTypestore:"ID submitted": - document.type.value === "passport" - ? `Passport - ${document.country.value}` - : document.type.value === "drivers_license" - ? `Driving License - ${document.country.value}` - : `ID Card - ${document.country.value}`, + (() => { + const docLabel = { + passport: "Passport", + id: "National ID", + permit: "Residence Permit", + dl: "Driver's License", + } as const; + return `${docLabel[$documentType ?? "passport"]} - ${document.country.value}`; + })(),
🧹 Nitpick comments (1)
infrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelte (1)
94-94: Terminology update aligns with multi-document support.The change from "DEMO PASSPORT"/"VERIFIED PASSPORT" to "DEMO ID"/"VERIFIED ID" appropriately generalizes the badge text to support multiple document types beyond passports.
Consider extracting these strings to an i18n/localization file if the application supports multiple languages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
infrastructure/eid-wallet/src-tauri/gen/android/app/build.gradle.ktsis excluded by!**/gen/**
📒 Files selected for processing (5)
infrastructure/eid-wallet/package.json(1 hunks)infrastructure/eid-wallet/src-tauri/tauri.conf.json(1 hunks)infrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelte(2 hunks)infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte(1 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte
🔇 Additional comments (6)
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (6)
138-143: LGTM!The resubmission handling correctly resets all captured document images (front, back, and selfie) and advances to the results step. The step number change from 2 to 3 properly reflects the new step mapping that includes DocumentType selection at step 0.
188-190: LGTM!The explicit guard after
initializeKeyManager()is good defensive programming. The clear error message "Key manager not initialized" will help with debugging if initialization silently fails.
207-209: LGTM!Consistent defensive guard with a clear error message, matching the pattern in
generateApplicationKeyPair().
232-237: LGTM!The conditional check for key existence only when
keyManageris defined prevents null reference errors and aligns with the defensive programming pattern used throughout the component.
330-331: LGTM!The subtitle text has been generalized to support multiple document types, addressing the past review concern about hardcoded "Passport" references.
362-367: LGTM!The step mapping correctly implements the new flow with document type selection as the initial step. The sequence (0: DocumentType → 1: Passport → 2: Selfie → 3+: Results) aligns with the changes throughout the component, including the
verifStep.set(3)at line 143.
| "targets": "all", | ||
| "android": { | ||
| "versionCode": 2 | ||
| "versionCode": 6 |
There was a problem hiding this comment.
Keep Tauri app version in sync with the release.
versionCode is bumped, but the same file still reports "version": "0.1.8", while package.json is now 0.3.0. This mismatch will surface in the built app (and any distribution metadata). Please update the Tauri version field to 0.3.0 so the Android bundle and desktop binaries advertise the intended release number.
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src-tauri/tauri.conf.json around line 29, the Tauri
config's "version" field is out of sync with package.json (file shows "version":
"0.1.8" while package.json is 0.3.0); update the "version" value to "0.3.0" so
it matches package.json/versionCode and ensure any other version fields in this
file match the new release number before committing.
| let loading = $state(false); | ||
| let keyManager: KeyManager | null = $state(null); | ||
| let websocketData: any = $state(null); // Store websocket data for duplicate case | ||
| let websocketData: { w3id?: string } | null = $state(null); // Store websocket data for duplicate case |
There was a problem hiding this comment.
Type narrowing conflicts with stored data.
websocketData is typed as { w3id?: string } | null, but line 137 assigns the full websocket message (which includes status, reason, person, document, and other properties). This type mismatch defeats the purpose of type safety.
Choose one of these solutions:
Option 1 (if only w3id is needed): Store only the required property.
- websocketData = data; // Store the full websocket data
+ websocketData = data.w3id ? { w3id: data.w3id } : null;Option 2 (if full data is needed): Define and use the complete type.
+ type WebsocketData = {
+ status: string;
+ reason: string;
+ person: Person;
+ document: Document;
+ w3id?: string;
+ };
+
- let websocketData: { w3id?: string } | null = $state(null);
+ let websocketData: WebsocketData | null = $state(null);📝 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.
| let websocketData: { w3id?: string } | null = $state(null); // Store websocket data for duplicate case | |
| // in your WebSocket message handler (around line 137) | |
| websocketData = data.w3id ? { w3id: data.w3id } : null; |
| let websocketData: { w3id?: string } | null = $state(null); // Store websocket data for duplicate case | |
| <script lang="ts"> | |
| // … other imports | |
| // Define the full shape of the incoming message | |
| type WebsocketData = { | |
| status: string; | |
| reason: string; | |
| person: Person; | |
| document: Document; | |
| w3id?: string; | |
| }; | |
| -// Store only w3id (too narrow) | |
| // Store the full payload with proper typing | |
| let websocketData: WebsocketData | null = $state(null); | |
| // … rest of your code | |
| </script> |
🤖 Prompt for AI Agents
in infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte around line
105 (and referenced assignment at line 137), websocketData is declared as {
w3id?: string } | null but later assigned the entire websocket message (which
includes status, reason, person, document, etc.), causing a type mismatch;
either restrict stored value to only the needed property (Option 1) by changing
assignments to extract and store just message.w3id and keep the current type, or
switch to Option 2 by declaring a full interface/type that matches the websocket
message shape and update websocketData’s type to that interface so the full
object can be stored safely—pick the appropriate option and update both the type
declaration and all assignments/usages accordingly.
Description of change
adds ui flows to support documents other than passport
Issue Number
closes #356
Type of change
How the change has been tested
Manual
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores