new UI/UX for home screen - add IdDetailsScreen #1006
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* update workflow to use workload identity federation * add token permissions * correct provider name * chore: incrementing android build version for version 2.6.4 [github action] --------- Co-authored-by: Self GitHub Actions <[email protected]>
* SDK Go version (#920) * feat: helper functions and constant for go-sdk * feat: formatRevealedDataPacked in go * chore: refactor * feat: define struct for selfBackendVerifier * feat: verify function for selfBackendVerifier * feat(wip): custom hasher * feat: SelfVerifierBacked in go * test(wip): scope and userContextHash is failing * test: zk proof verified * fix: MockConfigStore getactionId function * chore: refactor * chore: remove abi duplicate files * chore: move configStore to utils * chore: modified VcAndDiscloseProof struct * chore: more review changes * feat: impl DefaultConfig and InMemoryConfigStore * chore: refactor and export functions * fix: module import and README * chore: remove example folder * chore: remove pointers from VerificationConfig * chore: coderabbit review fixes * chore: more coderabbit review fix * chore: add license * fix: convert attestationIdd to int * chore: remove duplicate code --------- Co-authored-by: ayman <[email protected]> * Moving proving Utils to common (#935) * remove react dom * moves proving utils to the common * need to use rn components * fix imports * add proving-utils and dedeuplicate entry configs for esm and cjs. * must wrap in text component * fix metro bundling * fix mock import * fix builds and tests * please save me * solution? * fix test * Move proving inputs to the common package (#937) * create ofactTree type to share * move proving inputs from app to register inputs in common * missed reexport * ok * add some validations as suggested by our ai overlords * Fix mock passport flow (#942) * fix dev screens * add hint * rename * fix path * fix mobile-ci path * fix: extractMRZ (#938) * fix: extractMRZ * yarn nice && yarn types * fix test: remove unused * fix mobile ci * add script --------- Co-authored-by: Justin Hernandez <[email protected]> * Move Proving attest and cose (#950) * moved attest and cose utils to common with cursor converted tests in common to use vitest and converted coseVerify.test to vitest after moving from app to common what does cryptoLoader do? * moved away * get buff Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * SELF-253 feat: add user email feedback (#889) * feat: add sentry feedback * add sentry feedback to web * feat: add custom feedback modal & fix freeze on IOS * yarn nice * update lock * feat: show feedback widget on NFC scan issues (#948) * feat: show feedback widget on NFC scan issues * fix ref * clean up * fix report issue screen * abstract send user feedback email logic * fixes * change text to Report Issue * sanitize email and track event messge * remove unnecessary sanitization * add sanitize error message tests * fix tests * save wip. almost done * fix screen test * fix screen test * remove non working test --------- Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> * chore: centralize license header checks (#952) * chore: centralize license header scripts * chore: run license header checks from root * add header to other files * add header to bundle * add migration script and update check license headers * convert license to mobile sdk * migrate license headers * remove headers from common; convert remaining * fix headers * add license header checks * update unsupported passport screen (#953) * update unsupported passport screen * yarn nice --------- Co-authored-by: Vishalkulkarni45 <[email protected]> Co-authored-by: ayman <[email protected]> Co-authored-by: Aaron DeRuvo <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]> Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* update lock files * bump and build android * update build artifacts * show generate mock document button * update lock * fix formatting and update failing e2e test * revert podfile * fixes
…ction] (#976) Co-authored-by: remicolin <[email protected]>
…ction] (#985) Co-authored-by: remicolin <[email protected]>
…ction] (#987) Co-authored-by: remicolin <[email protected]>
…n mobile deploy workflow" This reverts commit 60fc1f2.
* fix android french id card * fix common ci cache * feat: log apdu (#988) --------- Co-authored-by: Justin Hernandez <[email protected]> Co-authored-by: Seshanth.S🐺 <[email protected]>
Contributor
WalkthroughAdds ID-details UX and proof-history integration: new IdDetails screen and NavBar, adaptive Id card, ProofHistoryList, Home catalog/document flow, documentId plumbing in schema/store with runtime migration, ProveScreen inclusion of documentId, new blur dependency, and color/font token updates. (47 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant HS as HomeScreen
participant PDP as PassportDataProvider
participant Nav as Navigation
participant IDS as IdDetailsScreen
participant PHL as ProofHistoryList
participant DB as Proof DB
U->>HS: Focus Home
HS->>PDP: loadDocumentCatalog() + getAllDocuments()
PDP-->>HS: {catalog, documents}
alt no documents or error
HS->>Nav: navigate(Launch)
else
HS-->>U: Render list of IdCardLayout
U->>Nav: tap card ➜ navigate(IdDetails, {documentId})
Nav->>IDS: mount
IDS->>PDP: getAllDocuments(), loadDocumentCatalog()
PDP-->>IDS: {doc data, catalog}
IDS-->>U: Render IdCard + actions
U->>IDS: Scroll proof history
IDS->>PHL: render(documentId)
PHL->>DB: initDatabase(), loadMoreHistory(documentId)
DB-->>PHL: rows grouped by period
end
sequenceDiagram
autonumber
participant PS as ProveScreen
participant PDP as PassportDataProvider
participant PHS as proofHistoryStore
participant DB as Proof DB
PS->>PDP: loadDocumentCatalog()
PDP-->>PS: { selectedDocumentId }
PS->>PHS: addProofHistory({... , documentId})
PHS->>DB: insertProof(proof w/ documentId)
alt schema has documentId
DB-->>PHS: {id, timestamp, rowsAffected}
else missing column
DB->>DB: ALTER TABLE ... ADD COLUMN documentId TEXT NOT NULL DEFAULT ''
DB->>DB: retry insert
DB-->>PHS: {id, timestamp, rowsAffected}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/utils/colors.ts (1)
33-41: Token collision: slate50 and slate100 are identical
slate100is set to#F8FAFC, same asslate50. This erodes semantic meaning of the scale and may break designs relying on hierarchy (e.g., disabled vs. surface). Use a distinct value forslate100(common choice:#F1F5F9).-export const slate100 = '#F8FAFC'; +export const slate100 = '#F1F5F9';app/src/screens/prove/ProveScreen.tsx (1)
49-50: Type-safe navigation: removeas nevercastUse a typed
useNavigationto avoid unsafe casts and catch route/param mistakes at compile time.+import type { NativeStackNavigationProp } from '@react-navigation/native-stack'; +import type { RootStackParamList } from '@/navigation'; @@ - const { navigate } = useNavigation(); + const { navigate } = + useNavigation<NativeStackNavigationProp<RootStackParamList>>(); @@ - navigate('ProofRequestStatusScreen' as never); + navigate('ProofRequestStatusScreen');Also applies to: 162-163
🧹 Nitpick comments (1)
app/src/screens/home/HomeScreen.tsx (1)
73-91: Load catalog and documents in parallel to cut home-screen time-to-render.Both calls are I/O-bound and independent. Parallelizing avoids the second Keychain/DB round-trip waiting on the first.
const loadDocuments = useCallback(async () => { setLoading(true); try { - const catalog = await loadDocumentCatalog(); - const docs = await getAllDocuments(); + const [catalog, docs] = await Promise.all([ + loadDocumentCatalog(), + getAllDocuments(), + ]); setDocumentCatalog(catalog); setAllDocuments(docs); if (catalog.documents.length === 0) { navigation.navigate('Launch' as never); } } catch (error) { console.warn('Failed to load documents:', error); navigation.navigate('Launch' as never); } setLoading(false); }, [loadDocumentCatalog, getAllDocuments, navigation]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
app/ios/Podfile.lockis excluded by!**/*.lockapp/src/images/icons/checkmark_gray.svgis excluded by!**/*.svgapp/src/images/icons/epassport.svgis excluded by!**/*.svgapp/src/images/logo_gray.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
app/package.json(1 hunks)app/src/components/NavBar/HomeNavBar.tsx(2 hunks)app/src/components/NavBar/IdDetailsNavBar.tsx(1 hunks)app/src/components/NavBar/index.ts(1 hunks)app/src/components/homeScreen/idCard.tsx(1 hunks)app/src/navigation/home.ts(3 hunks)app/src/navigation/index.tsx(2 hunks)app/src/screens/home/HomeScreen.tsx(4 hunks)app/src/screens/home/IdDetailsScreen.tsx(1 hunks)app/src/screens/home/ProofHistoryList.tsx(1 hunks)app/src/screens/misc/SplashScreen.tsx(1 hunks)app/src/screens/prove/ProveScreen.tsx(3 hunks)app/src/stores/database.ts(3 hunks)app/src/stores/proof-types.ts(1 hunks)app/src/stores/proofHistoryStore.ts(1 hunks)app/src/utils/colors.ts(2 hunks)app/src/utils/fonts.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/technical-specification.mdc)
**/*.{ts,tsx}: Define IdentityCommitment with fields: commitment (Poseidon hash), nullifier (domain-separated), timestamp (UTC number), version (circuit version), documentType ('passport' | 'eu_id_card')
Define DSCKeyCommitment with fields: publicKeyHash (Poseidon hash), certificateChain (hashes), revocationStatus (boolean), issuer (country code)
Define VerificationConfig with fields: circuitVersion (semver), complianceRules array, timeWindow (seconds, 24h), clockDrift (±5 min), trustAnchors, revocationRoots, timeSource (NTP), nullifierScope (domain separation)
Files:
app/src/stores/proof-types.tsapp/src/screens/misc/SplashScreen.tsxapp/src/components/NavBar/IdDetailsNavBar.tsxapp/src/components/NavBar/HomeNavBar.tsxapp/src/components/NavBar/index.tsapp/src/navigation/index.tsxapp/src/screens/home/IdDetailsScreen.tsxapp/src/components/homeScreen/idCard.tsxapp/src/screens/home/HomeScreen.tsxapp/src/utils/colors.tsapp/src/utils/fonts.tsapp/src/screens/home/ProofHistoryList.tsxapp/src/navigation/home.tsapp/src/stores/database.tsapp/src/screens/prove/ProveScreen.tsxapp/src/stores/proofHistoryStore.ts
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/stores/proof-types.tsapp/src/screens/misc/SplashScreen.tsxapp/src/components/NavBar/IdDetailsNavBar.tsxapp/src/components/NavBar/HomeNavBar.tsxapp/src/components/NavBar/index.tsapp/src/navigation/index.tsxapp/src/screens/home/IdDetailsScreen.tsxapp/src/components/homeScreen/idCard.tsxapp/src/screens/home/HomeScreen.tsxapp/src/utils/colors.tsapp/src/utils/fonts.tsapp/src/screens/home/ProofHistoryList.tsxapp/src/navigation/home.tsapp/src/stores/database.tsapp/src/screens/prove/ProveScreen.tsxapp/src/stores/proofHistoryStore.ts
app/package.json
📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)
Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests
Files:
app/package.json
🧠 Learnings (8)
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to src/navigation/**/*.{ts,tsx} : Set platform-specific initial routes: web → Home, mobile → Splash
Applied to files:
app/src/screens/misc/SplashScreen.tsxapp/src/navigation/home.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to src/navigation/**/*.{ts,tsx} : Use react-navigation/native with createStaticNavigation for type-safe navigation
Applied to files:
app/src/components/NavBar/IdDetailsNavBar.tsxapp/src/navigation/index.tsxapp/src/navigation/home.ts
📚 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}} : Test isPassportDataValid() with realistic synthetic passport data (never real user data)
Applied to files:
app/src/components/homeScreen/idCard.tsx
📚 Learning: 2025-08-29T15:30:12.210Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-08-29T15:30:12.210Z
Learning: Document complex native module changes for AI review
Applied to files:
app/src/screens/home/HomeScreen.tsx
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to src/**/*.{ios.ts,ios.tsx,android.ts,android.tsx} : Use platform-specific files (.ios.tsx/.android.tsx etc.) for differing implementations
Applied to files:
app/src/utils/fonts.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to src/**/*.{ts,tsx} : Use Platform.OS checks before platform-specific code paths
Applied to files:
app/src/utils/fonts.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Applies to src/**/screens/**/*.{ts,tsx} : Lazy load screens using React.lazy()
Applied to files:
app/src/navigation/home.ts
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Organize screens by feature modules (passport, home, settings, etc.)
Applied to files:
app/src/navigation/home.ts
🧬 Code graph analysis (11)
app/src/components/NavBar/IdDetailsNavBar.tsx (6)
app/src/components/NavBar/index.ts (1)
IdDetailsNavBar(7-7)app/src/mocks/react-native-safe-area-context.js (1)
useSafeAreaInsets(44-46)app/src/components/NavBar/BaseNavBar.tsx (1)
NavBar(128-133)app/src/utils/colors.ts (3)
slate50(41-41)charcoal(15-15)black(8-8)app/src/utils/constants.ts (1)
extraYPadding(5-5)app/src/utils/haptic/index.ts (1)
buttonTap(18-18)
app/src/components/NavBar/HomeNavBar.tsx (4)
app/src/utils/colors.ts (3)
slate50(41-41)charcoal(15-15)black(8-8)app/src/utils/constants.ts (1)
extraYPadding(5-5)app/src/components/NavBar/BaseNavBar.tsx (1)
NavBar(128-133)app/src/utils/haptic/index.ts (1)
buttonTap(18-18)
app/src/navigation/index.tsx (1)
app/src/stores/proof-types.ts (1)
ProofHistory(32-46)
app/src/screens/home/IdDetailsScreen.tsx (3)
app/src/providers/passportDataProvider.tsx (5)
usePassport(885-887)DocumentCatalog(95-98)getAllDocuments(355-371)loadDocumentCatalog(502-535)setSelectedDocument(799-809)app/src/utils/colors.ts (6)
slate50(41-41)white(59-59)slate300(37-37)slate100(33-33)slate500(43-43)black(8-8)app/src/screens/home/ProofHistoryList.tsx (1)
ProofHistoryList(54-401)
app/src/components/homeScreen/idCard.tsx (2)
app/src/utils/colors.ts (6)
white(59-59)black(8-8)slate300(37-37)slate100(33-33)slate400(39-39)slate500(43-43)app/src/utils/fonts.ts (2)
dinot(8-8)plexMono(9-9)
app/src/screens/home/HomeScreen.tsx (3)
app/src/providers/passportDataProvider.tsx (6)
usePassport(885-887)DocumentCatalog(95-98)DocumentMetadata(86-93)loadDocumentCatalog(502-535)getAllDocuments(355-371)setSelectedDocument(799-809)app/src/utils/colors.ts (1)
slate50(41-41)app/src/utils/constants.ts (1)
extraYPadding(5-5)
app/src/utils/fonts.ts (1)
app/src/screens/MainScreen.tsx (1)
Platform(106-115)
app/src/screens/home/ProofHistoryList.tsx (5)
app/src/stores/proof-types.ts (1)
ProofHistory(32-46)app/src/stores/proofHistoryStore.ts (1)
useProofHistoryStore(35-216)app/src/utils/colors.ts (10)
slate200(35-35)white(59-59)black(8-8)slate400(39-39)blue100(9-9)blue600(10-10)red500(27-27)slate50(41-41)slate500(43-43)slate300(37-37)app/src/components/typography/BodyText.ts (1)
BodyText(9-11)app/src/utils/fonts.ts (2)
plexMono(9-9)dinot(8-8)
app/src/navigation/home.ts (2)
app/src/components/NavBar/IdDetailsNavBar.tsx (1)
IdDetailsNavBar(16-56)app/src/components/NavBar/index.ts (1)
IdDetailsNavBar(7-7)
app/src/stores/database.ts (1)
app/src/stores/database.web.ts (1)
openDatabase(20-51)
app/src/screens/prove/ProveScreen.tsx (1)
app/src/providers/passportDataProvider.tsx (2)
usePassport(885-887)loadDocumentCatalog(502-535)
⏰ 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). (6)
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: test
- GitHub Check: e2e-ios
- GitHub Check: analyze-ios
- GitHub Check: analyze-android
🔇 Additional comments (9)
app/src/screens/misc/SplashScreen.tsx (1)
23-27: No-op import refactor — OKFormatting-only change; behavior and tree-shaking remain unchanged.
app/package.json (1)
78-78: BlurView dependency addition looks goodVersion ^4.4.1 is compatible with RN 0.75.x in practice. Ensure CI runs a full native build so Pod/Gradle integration failures surface early.
app/src/stores/proof-types.ts (1)
45-45: New required field documentId: ensure it’s non-empty across migrationsDB migration sets DEFAULT '' for legacy rows. If downstream logic assumes a truthy documentId, empty strings will behave like “missing” and can break UX flows (e.g., deep-linking to an ID). Verify call sites either backfill or guard against ''.
app/src/utils/fonts.ts (1)
5-9: Verify Android font family name matches bundled asset'IBMPlexMono-Regular' must match the actual Android fontFamily exposed by your TTF/OTF. A mismatch silently falls back to the system font and will degrade code/mono UI surfaces.
app/src/stores/database.ts (1)
100-102: Schema requires documentId but migration seeds '' — confirm downstream behaviorNew installs enforce NOT NULL; legacy rows receive ''. If documentId is semantically required for ProofHistory flows, consider a backfill strategy or ensure UI/API paths treat '' as “legacy/no-doc” explicitly.
app/src/components/NavBar/index.ts (1)
7-7: LGTM: barrel export for IdDetailsNavBarRe-export looks correct and keeps the NavBar API consistent.
app/src/stores/proofHistoryStore.ts (1)
169-187: Guard documentId and verify DB migration/backfill
- In
app/src/stores/proofHistoryStore.ts, use a safe fallback to prevent undefined at runtime:documentId: row.documentId ?? '',- Confirm in your backend schema that existing rows have
document_idbackfilled and that the column is declaredNOT NULL.- (Medium-priority) Consider adding an index on
proof_history(document_id)if you’ll frequently filter or paginate by it.app/src/components/NavBar/HomeNavBar.tsx (1)
74-77: Verify status bar contrast with new light backgroundbackgroundColor is slate50 (very light) while barStyle is 'light' (typically light-content), which can reduce readability on iOS/Android. Confirm BaseNavBar maps 'light' to dark-content for light backgrounds; otherwise switch to 'dark'.
app/src/navigation/home.ts (1)
51-58: LGTM: route and custom header wiring look correctThe IdDetails route is lazily loaded, uses the custom header, and hides the default back button. Navigation typing should already cover the
{ documentId: string }param.
Comment on lines
+66
to
+71
| // Get screen dimensions for adaptive sizing | ||
| const { width: screenWidth, height: screenHeight } = Dimensions.get('window'); | ||
|
|
||
| // Calculate adaptive sizes based on screen dimensions | ||
| // Reduce width slightly to account for horizontal margins (8px each side = 16px total) | ||
| const cardWidth = screenWidth * 0.95 - 16; // 90% of screen width minus margin space |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Recompute layout on orientation/size changes.
Using Dimensions.get('window') freezes sizes at mount. Switch to useWindowDimensions to react to rotation and multi-window.
-import { Dimensions, Pressable } from 'react-native';
+import { useWindowDimensions, Pressable } from 'react-native';
...
- const { width: screenWidth, height: screenHeight } = Dimensions.get('window');
+ const { width: screenWidth, height: screenHeight } = useWindowDimensions();🤖 Prompt for AI Agents
In app/src/components/homeScreen/idCard.tsx around lines 66 to 71, the code uses
Dimensions.get('window') which only captures sizes at mount and won't update on
rotation or multi-window; replace it with the useWindowDimensions hook: import
useWindowDimensions from 'react-native' (or from 'react-native' named import),
call const { width: screenWidth, height: screenHeight } = useWindowDimensions()
inside the component so sizes recompute on orientation/size changes, then
compute cardWidth = screenWidth * 0.95 - 16 as before; also remove or update any
unused Dimensions import to avoid lint warnings.
Comment on lines
+206
to
+346
| height={imageSize.height} | ||
| > | ||
| <XStack flex={1} gap={padding * 0.3}> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="TYPE" | ||
| value={ | ||
| idDocument.documentCategory === 'passport' | ||
| ? 'PASSPORT' | ||
| : 'ID CARD' | ||
| } | ||
| maskValue={ | ||
| idDocument.documentCategory === 'passport' | ||
| ? 'PASSPORT' | ||
| : 'ID CARD' | ||
| } | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="CODE" | ||
| value={idDocument.mock ? 'SELF DEV' : 'SELF ID'} | ||
| maskValue={idDocument.mock ? 'SELF DEV' : 'SELF ID'} | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="DOC NO." | ||
| value={ | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).passNoSlice | ||
| } | ||
| maskValue="XX-XXXXXXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| </XStack> | ||
| <XStack flex={1} gap={padding * 0.3}> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="SURNAME" | ||
| value={getNameAndSurname( | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).nameSlice, | ||
| ).surname.join(' ')} | ||
| maskValue="XXXXXXXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="NAME" | ||
| value={getNameAndSurname( | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).nameSlice, | ||
| ).names.join(' ')} | ||
| maskValue="XXXXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="SEX" | ||
| value={ | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).sexSlice | ||
| } | ||
| maskValue="X" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| </XStack> | ||
| <XStack flex={1} gap={padding * 0.3}> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="NATIONALITY" | ||
| value={ | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).nationalitySlice | ||
| } | ||
| maskValue="XXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="DOB" | ||
| value={formatDateFromYYMMDD( | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).dobSlice, | ||
| )} | ||
| maskValue="XX/XX/XXXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="EXPIRY DATE" | ||
| value={formatDateFromYYMMDD( | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).expiryDateSlice, | ||
| true, | ||
| )} | ||
| maskValue="XX/XX/XXXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| </XStack> | ||
| <XStack flex={1} gap={padding * 0.3}> | ||
| <YStack flex={1}> | ||
| <IdAttribute | ||
| name="AUTHORITY" | ||
| value={ | ||
| getPassportAttributes( | ||
| idDocument.mrz, | ||
| idDocument.documentCategory, | ||
| ).issuingStateSlice | ||
| } | ||
| maskValue="XXX" | ||
| hidden={hidden} | ||
| /> | ||
| </YStack> | ||
| <YStack flex={1} /> | ||
| <YStack flex={1} /> | ||
| </XStack> |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid repeated MRZ parsing: compute once and reuse.
getPassportAttributes(...) is called many times per render, multiplying string slicing work. Compute once and pass slices around.
-const IdCardLayout: React.FC<IdCardLayoutAttributes> = ({
+const IdCardLayout: React.FC<IdCardLayoutAttributes> = ({
idDocument,
selected,
hidden,
}) => {
+ const attrs = getPassportAttributes(
+ idDocument.mrz,
+ idDocument.documentCategory,
+ );
...
- value={
- getPassportAttributes(
- idDocument.mrz,
- idDocument.documentCategory,
- ).passNoSlice
- }
+ value={attrs.passNoSlice}
...
- value={getNameAndSurname(
- getPassportAttributes(
- idDocument.mrz,
- idDocument.documentCategory,
- ).nameSlice,
- ).surname.join(' ')}
+ value={getNameAndSurname(attrs.nameSlice).surname.join(' ')}
...
- value={
- getPassportAttributes(
- idDocument.mrz,
- idDocument.documentCategory,
- ).sexSlice
- }
+ value={attrs.sexSlice}Apply the same substitution for nationalitySlice, dobSlice, expiryDateSlice, issuingStateSlice, and names. If needed, memoize attrs with useMemo keyed by mrz and documentCategory.
🤖 Prompt for AI Agents
In app/src/components/homeScreen/idCard.tsx around lines 206 to 346, the
component repeatedly calls getPassportAttributes(idDocument.mrz,
idDocument.documentCategory) (and nested getNameAndSurname calls) for each
IdAttribute; compute the parsed attributes once and reuse them to avoid repeated
MRZ parsing: add a const attrs = useMemo(() =>
getPassportAttributes(idDocument.mrz, idDocument.documentCategory),
[idDocument.mrz, idDocument.documentCategory]) (and optionally const nameParts =
useMemo(() => getNameAndSurname(attrs.nameSlice), [attrs.nameSlice]) near the
top of the component) and then replace all occurrences with attrs.passNoSlice,
attrs.nationalitySlice, attrs.dobSlice, attrs.expiryDateSlice,
attrs.issuingStateSlice and use nameParts.names / nameParts.surname for the
NAME/SURNAME fields; memoize as suggested if needed to further avoid
re-computation.
Comment on lines
+82
to
+99
| <XStack alignItems="center"> | ||
| <Button | ||
| size={'$3'} | ||
| unstyled | ||
| icon={<ScanIcon width={'24'} height={'100%'} color={charcoal} />} | ||
| onPress={() => { | ||
| buttonTap(); | ||
| props.navigation.navigate('QRCodeViewFinder'); | ||
| }} | ||
| /> | ||
| <Button | ||
| size={'$3'} | ||
| unstyled | ||
| marginTop={10} | ||
| icon={<ClipboardIcon size={24} color={charcoal} />} | ||
| onPress={handleConsumeToken} | ||
| /> | ||
| </XStack> |
Contributor
There was a problem hiding this comment.
Harden consume-token flow: no token logging, POST + timeout, config-driven base URL
Currently handleConsumeToken logs clipboard contents and uses a GET with the token in the URL, which can leak in proxies/logs and has no timeout. Switch to a POST with body, remove sensitive logs, add AbortController with a reasonable timeout, and read the API base URL from configuration.
Apply this minimal change to wire a hardened handler:
- onPress={handleConsumeToken}
+ onPress={consumeDeferredTokenSecurely}Add the secure handler (outside this hunk):
// Prefer env/config source, not a hardcoded URL
const API_BASE = process.env.EXPO_PUBLIC_API_BASE ?? 'https://api.self.xyz';
async function consumeDeferredTokenSecurely() {
buttonTap();
const content = await Clipboard.getString();
const uuidRegex =
/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
if (!content || !uuidRegex.test(content)) {
props.navigation.navigate('DeferredLinkingInfo');
return;
}
const ac = new AbortController();
const t = setTimeout(() => ac.abort(), 10000); // 10s
try {
const resp = await fetch(`${API_BASE}/consume-deferred-linking-token`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ token: content }),
signal: ac.signal,
});
if (!resp.ok) throw new Error(`HTTP ${resp.status}`);
const result = await resp.json();
if (result.status !== 'success') {
throw new Error(result.message || 'Failed to consume token');
}
const selfApp: SelfApp = JSON.parse(result.data.self_app);
useSelfAppStore.getState().setSelfApp(selfApp);
useSelfAppStore.getState().startAppListener(selfApp.sessionId);
try { Clipboard.setString(''); } catch {}
props.navigation.navigate('ProveScreen');
} catch (err) {
// Do not log token or full response; keep logs minimal
console.error('Error consuming token', err);
if (err instanceof Error && err.message.includes('Token not found or expired')) {
try { Clipboard.setString(''); } catch {}
props.navigation.navigate('DeferredLinkingInfo');
}
} finally {
clearTimeout(t);
}
}🤖 Prompt for AI Agents
In app/src/components/NavBar/HomeNavBar.tsx around lines 82 to 99, replace the
existing handleConsumeToken usage with a new secure handler defined outside this
hunk: stop logging clipboard contents, read the API base from config (e.g.
process.env.EXPO_PUBLIC_API_BASE with a fallback), POST the token in a JSON body
to /consume-deferred-linking-token instead of sending it in the URL, validate
the clipboard value with a UUID regex and navigate to DeferredLinkingInfo if
invalid, use an AbortController with a ~10s timeout to cancel slow requests,
parse and check resp.ok and result.status, update the SelfApp store and start
the app listener on success, clear the clipboard after success or known "token
not found/expired" failures, and only emit minimal error logs (no token or full
response content); ensure clearTimeout is called in finally.
Comment on lines
+21
to
+26
| <NavBar.Container | ||
| backgroundColor={slate50} | ||
| barStyle={'light'} | ||
| justifyContent="space-between" | ||
| paddingTop={Math.max(insets.top, 15) + extraYPadding} | ||
| > |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Status bar contrast: use dark status bar on light header background
Header background is light (slate50) but barStyle="light" yields light text/icons on a light background → poor contrast. Switch to a dark status bar style to meet accessibility and readability.
- <NavBar.Container
- backgroundColor={slate50}
- barStyle={'light'}
+ <NavBar.Container
+ backgroundColor={slate50}
+ barStyle={'dark'} // or 'dark-content' based on NavBar.Container prop contract🏁 Script executed:
#!/bin/bash
rg -n "export const Container" -C3 app/src/components/NavBar
rg -n "interface ContainerProps" -C3 app/src/components/NavBar
rg -n "barStyle" -C5 app/src/components/NavBarLength of output: 5355
🏁 Script executed:
#!/bin/bash
rg -n "SystemBars" -C3 app/src/components/NavBar
rg -n "SystemBarStyle" -C3 .Length of output: 2368
Use dark system bar style on light header
<NavBar.Container> in IdDetailsNavBar.tsx uses barStyle="light", which renders light icons/text on a light slate50 background → poor contrast. Change to barStyle="dark" to ensure dark icons/text on the light header for accessibility (github.com)
- <NavBar.Container
- backgroundColor={slate50}
- barStyle="light"
+ <NavBar.Container
+ backgroundColor={slate50}
+ barStyle="dark"📝 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
| <NavBar.Container | |
| backgroundColor={slate50} | |
| barStyle={'light'} | |
| justifyContent="space-between" | |
| paddingTop={Math.max(insets.top, 15) + extraYPadding} | |
| > | |
| <NavBar.Container | |
| backgroundColor={slate50} | |
| barStyle="dark" | |
| justifyContent="space-between" | |
| paddingTop={Math.max(insets.top, 15) + extraYPadding} | |
| > |
🤖 Prompt for AI Agents
In app/src/components/NavBar/IdDetailsNavBar.tsx around lines 21 to 26, the
NavBar.Container is using barStyle="light" which places light icons/text on a
light slate50 background; change barStyle to "dark" so the system bar uses dark
icons/text for proper contrast on the light header. Update the prop to
barStyle="dark" and keep other props unchanged.
Comment on lines
+53
to
+57
| export type RootStackParamList = StaticParamList<typeof AppNavigation> & { | ||
| IdDetails: { documentId: string }; | ||
| ProofHistoryDetail: { data: ProofHistory }; | ||
| ProofRequestStatusScreen: undefined; | ||
| }; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid passing large/PII-rich objects in route params
ProofHistory may include userId and base64 logo, which increases memory footprint and risk of accidental logging/analytics leakage in navigation state. Prefer passing a stable identifier and loading the item from the store on the destination screen.
Apply:
-export type RootStackParamList = StaticParamList<typeof AppNavigation> & {
- IdDetails: { documentId: string };
- ProofHistoryDetail: { data: ProofHistory };
- ProofRequestStatusScreen: undefined;
-};
+export type RootStackParamList = StaticParamList<typeof AppNavigation> & {
+ IdDetails: { documentId: string };
+ ProofHistoryDetail: { id: string };
+ ProofRequestStatusScreen: undefined;
+};Follow-up: update navigators and ProofHistoryList to pass id only, and hydrate detail data from the store.
📝 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
| export type RootStackParamList = StaticParamList<typeof AppNavigation> & { | |
| IdDetails: { documentId: string }; | |
| ProofHistoryDetail: { data: ProofHistory }; | |
| ProofRequestStatusScreen: undefined; | |
| }; | |
| export type RootStackParamList = StaticParamList<typeof AppNavigation> & { | |
| IdDetails: { documentId: string }; | |
| ProofHistoryDetail: { id: string }; | |
| ProofRequestStatusScreen: undefined; | |
| }; |
🤖 Prompt for AI Agents
In app/src/navigation/index.tsx around lines 53 to 57, the ProofHistoryDetail
route is typed to accept a ProofHistory object (which can include PII and large
base64 data); change the route param to accept only a stable identifier (e.g., {
id: string }) and update all navigator usages and the ProofHistoryList to pass
only that id. On the destination screen, hydrate the full ProofHistory by
selecting it from the store (or loading it by id) instead of relying on
navigation params; ensure any existing call sites and tests are updated to the
new param shape and remove any direct passing of the full object.
Comment on lines
+156
to
+165
| Array.from(monthGroups) | ||
| .sort( | ||
| (a, b) => | ||
| new Date(groups[b][0].timestamp).getMonth() - | ||
| new Date(groups[a][0].timestamp).getMonth(), | ||
| ) | ||
| .forEach(month => { | ||
| sections.push({ title: month, data: groups[month] }); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix month section sorting to account for year.
Sorting by getMonth() ignores the year, misordering sections across years. Sort by the representative timestamp directly.
- Array.from(monthGroups)
- .sort(
- (a, b) =>
- new Date(groups[b][0].timestamp).getMonth() -
- new Date(groups[a][0].timestamp).getMonth(),
- )
+ Array.from(monthGroups)
+ .sort((a, b) => groups[b][0].timestamp - groups[a][0].timestamp)
.forEach(month => {
sections.push({ title: month, data: groups[month] });
});📝 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
| Array.from(monthGroups) | |
| .sort( | |
| (a, b) => | |
| new Date(groups[b][0].timestamp).getMonth() - | |
| new Date(groups[a][0].timestamp).getMonth(), | |
| ) | |
| .forEach(month => { | |
| sections.push({ title: month, data: groups[month] }); | |
| }); | |
| Array.from(monthGroups) | |
| .sort((a, b) => groups[b][0].timestamp - groups[a][0].timestamp) | |
| .forEach(month => { | |
| sections.push({ title: month, data: groups[month] }); | |
| }); |
🤖 Prompt for AI Agents
In app/src/screens/home/ProofHistoryList.tsx around lines 156 to 165, the sort
currently compares only getMonth(), which ignores year and misorders sections
across years; update the comparator to parse the representative timestamps and
compare their full time values (e.g., use Date.parse(...) or new
Date(...).getTime()) for groups[b][0].timestamp versus groups[a][0].timestamp so
sections are sorted by actual datetime (newest/oldest as intended) before
pushing to sections.
Comment on lines
72
to
+92
| useEffect(() => { | ||
| if (provingStore.uuid && selectedApp) { | ||
| addProofHistory({ | ||
| appName: selectedApp.appName, | ||
| sessionId: provingStore.uuid!, | ||
| userId: selectedApp.userId, | ||
| userIdType: selectedApp.userIdType, | ||
| endpointType: selectedApp.endpointType, | ||
| status: ProofStatus.PENDING, | ||
| logoBase64: selectedApp.logoBase64, | ||
| disclosures: JSON.stringify(selectedApp.disclosures), | ||
| }); | ||
| } | ||
| }, [addProofHistory, provingStore.uuid, selectedApp]); | ||
| const addHistory = async () => { | ||
| if (provingStore.uuid && selectedApp) { | ||
| const catalog = await loadDocumentCatalog(); | ||
| const selectedDocumentId = catalog.selectedDocumentId; | ||
|
|
||
| addProofHistory({ | ||
| appName: selectedApp.appName, | ||
| sessionId: provingStore.uuid!, | ||
| userId: selectedApp.userId, | ||
| userIdType: selectedApp.userIdType, | ||
| endpointType: selectedApp.endpointType, | ||
| status: ProofStatus.PENDING, | ||
| logoBase64: selectedApp.logoBase64, | ||
| disclosures: JSON.stringify(selectedApp.disclosures), | ||
| documentId: selectedDocumentId || '', // Fallback to empty if none selected | ||
| }); | ||
| } | ||
| }; | ||
| addHistory(); | ||
| }, [addProofHistory, provingStore.uuid, selectedApp, loadDocumentCatalog]); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent duplicate PENDING history entries per session
This effect can re-run (prop changes, function identity changes), creating multiple PENDING entries for the same sessionId. Guard by remembering the last added sessionId.
@@
const selectedAppRef = useRef<typeof selectedApp>(null);
+ const lastAddedSessionIdRef = useRef<string | null>(null);
@@
- useEffect(() => {
- const addHistory = async () => {
- if (provingStore.uuid && selectedApp) {
- const catalog = await loadDocumentCatalog();
- const selectedDocumentId = catalog.selectedDocumentId;
-
- addProofHistory({
- appName: selectedApp.appName,
- sessionId: provingStore.uuid!,
- userId: selectedApp.userId,
- userIdType: selectedApp.userIdType,
- endpointType: selectedApp.endpointType,
- status: ProofStatus.PENDING,
- logoBase64: selectedApp.logoBase64,
- disclosures: JSON.stringify(selectedApp.disclosures),
- documentId: selectedDocumentId || '', // Fallback to empty if none selected
- });
- }
- };
- addHistory();
- }, [addProofHistory, provingStore.uuid, selectedApp, loadDocumentCatalog]);
+ useEffect(() => {
+ const addHistory = async () => {
+ if (!provingStore.uuid || !selectedApp) return;
+ if (lastAddedSessionIdRef.current === provingStore.uuid) return;
+ const catalog = await loadDocumentCatalog();
+ const selectedDocumentId = catalog.selectedDocumentId || '';
+ addProofHistory({
+ appName: selectedApp.appName,
+ sessionId: provingStore.uuid,
+ userId: selectedApp.userId,
+ userIdType: selectedApp.userIdType,
+ endpointType: selectedApp.endpointType,
+ status: ProofStatus.PENDING,
+ logoBase64: selectedApp.logoBase64,
+ disclosures: JSON.stringify(selectedApp.disclosures),
+ documentId: selectedDocumentId,
+ });
+ lastAddedSessionIdRef.current = provingStore.uuid;
+ };
+ addHistory();
+ }, [addProofHistory, provingStore.uuid, selectedApp, loadDocumentCatalog]);📝 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
| useEffect(() => { | |
| if (provingStore.uuid && selectedApp) { | |
| addProofHistory({ | |
| appName: selectedApp.appName, | |
| sessionId: provingStore.uuid!, | |
| userId: selectedApp.userId, | |
| userIdType: selectedApp.userIdType, | |
| endpointType: selectedApp.endpointType, | |
| status: ProofStatus.PENDING, | |
| logoBase64: selectedApp.logoBase64, | |
| disclosures: JSON.stringify(selectedApp.disclosures), | |
| }); | |
| } | |
| }, [addProofHistory, provingStore.uuid, selectedApp]); | |
| const addHistory = async () => { | |
| if (provingStore.uuid && selectedApp) { | |
| const catalog = await loadDocumentCatalog(); | |
| const selectedDocumentId = catalog.selectedDocumentId; | |
| addProofHistory({ | |
| appName: selectedApp.appName, | |
| sessionId: provingStore.uuid!, | |
| userId: selectedApp.userId, | |
| userIdType: selectedApp.userIdType, | |
| endpointType: selectedApp.endpointType, | |
| status: ProofStatus.PENDING, | |
| logoBase64: selectedApp.logoBase64, | |
| disclosures: JSON.stringify(selectedApp.disclosures), | |
| documentId: selectedDocumentId || '', // Fallback to empty if none selected | |
| }); | |
| } | |
| }; | |
| addHistory(); | |
| }, [addProofHistory, provingStore.uuid, selectedApp, loadDocumentCatalog]); | |
| const selectedAppRef = useRef<typeof selectedApp>(null); | |
| const lastAddedSessionIdRef = useRef<string | null>(null); | |
| useEffect(() => { | |
| const addHistory = async () => { | |
| // Bail out if we don't have a session or app yet | |
| if (!provingStore.uuid || !selectedApp) return; | |
| // Already recorded this session? Prevent duplicate PENDING entries. | |
| if (lastAddedSessionIdRef.current === provingStore.uuid) return; | |
| const catalog = await loadDocumentCatalog(); | |
| const selectedDocumentId = catalog.selectedDocumentId || ''; | |
| addProofHistory({ | |
| appName: selectedApp.appName, | |
| sessionId: provingStore.uuid, | |
| userId: selectedApp.userId, | |
| userIdType: selectedApp.userIdType, | |
| endpointType: selectedApp.endpointType, | |
| status: ProofStatus.PENDING, | |
| logoBase64: selectedApp.logoBase64, | |
| disclosures: JSON.stringify(selectedApp.disclosures), | |
| documentId: selectedDocumentId, | |
| }); | |
| // Mark this session as recorded | |
| lastAddedSessionIdRef.current = provingStore.uuid; | |
| }; | |
| addHistory(); | |
| }, [addProofHistory, provingStore.uuid, selectedApp, loadDocumentCatalog]); |
🤖 Prompt for AI Agents
In app/src/screens/prove/ProveScreen.tsx around lines 72 to 92, the useEffect
can run multiple times and repeatedly add a PENDING history entry for the same
provingStore.uuid; fix by tracking the last sessionId you added (e.g., use a ref
like lastAddedSessionRef) and check it before calling addProofHistory—if
lastAddedSessionRef.current === provingStore.uuid skip adding, otherwise call
addProofHistory and set lastAddedSessionRef.current = provingStore.uuid; ensure
the guard is checked inside addHistory and the ref is defined outside the effect
so it persists across renders.
Comment on lines
+113
to
+137
| try { | ||
| const [insertResult] = await db.executeSql( | ||
| `INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | ||
| [ | ||
| proof.appName, | ||
| proof.endpointType, | ||
| proof.status, | ||
| proof.errorCode || null, | ||
| proof.errorReason || null, | ||
| timestamp, | ||
| proof.disclosures, | ||
| proof.logoBase64 || null, | ||
| proof.userId, | ||
| proof.userIdType, | ||
| proof.sessionId, | ||
| proof.documentId, | ||
| ], | ||
| ); | ||
| return { | ||
| id: insertResult.insertId.toString(), | ||
| timestamp, | ||
| proof.disclosures, | ||
| proof.logoBase64 || null, | ||
| proof.userId, | ||
| proof.userIdType, | ||
| proof.sessionId, | ||
| ], | ||
| ); | ||
| return { | ||
| id: insertResult.insertId.toString(), | ||
| timestamp, | ||
| rowsAffected: insertResult.rowsAffected, | ||
| }; | ||
| rowsAffected: insertResult.rowsAffected, | ||
| }; | ||
| } catch (error) { |
Contributor
There was a problem hiding this comment.
INSERT OR IGNORE + insertId.toString() can crash and hide duplicates
If sessionId already exists, rowsAffected will be 0 and insertId may be undefined; calling toString() will throw. Also, returning a fresh Date.now() timestamp for a duplicate is misleading. Guard duplicates and fetch the existing row’s id/timestamp.
Apply this diff:
@@
- try {
- const [insertResult] = await db.executeSql(
+ try {
+ const [insertResult] = await db.executeSql(
`INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
[
proof.appName,
proof.endpointType,
proof.status,
proof.errorCode || null,
proof.errorReason || null,
timestamp,
proof.disclosures,
proof.logoBase64 || null,
proof.userId,
proof.userIdType,
proof.sessionId,
proof.documentId,
],
);
- return {
- id: insertResult.insertId.toString(),
- timestamp,
- rowsAffected: insertResult.rowsAffected,
- };
+ if (insertResult.rowsAffected === 0) {
+ // Duplicate sessionId: return existing row
+ const [existing] = await db.executeSql(
+ `SELECT id, timestamp FROM ${TABLE_NAME} WHERE sessionId = ? LIMIT 1`,
+ [proof.sessionId],
+ );
+ const row = existing.rows.item(0);
+ return {
+ id: String(row?.id ?? 0),
+ timestamp: row?.timestamp ?? timestamp,
+ rowsAffected: 0,
+ };
+ }
+ return {
+ id: String(insertResult.insertId),
+ timestamp,
+ rowsAffected: insertResult.rowsAffected,
+ };📝 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
| try { | |
| const [insertResult] = await db.executeSql( | |
| `INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId) | |
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | |
| [ | |
| proof.appName, | |
| proof.endpointType, | |
| proof.status, | |
| proof.errorCode || null, | |
| proof.errorReason || null, | |
| timestamp, | |
| proof.disclosures, | |
| proof.logoBase64 || null, | |
| proof.userId, | |
| proof.userIdType, | |
| proof.sessionId, | |
| proof.documentId, | |
| ], | |
| ); | |
| return { | |
| id: insertResult.insertId.toString(), | |
| timestamp, | |
| proof.disclosures, | |
| proof.logoBase64 || null, | |
| proof.userId, | |
| proof.userIdType, | |
| proof.sessionId, | |
| ], | |
| ); | |
| return { | |
| id: insertResult.insertId.toString(), | |
| timestamp, | |
| rowsAffected: insertResult.rowsAffected, | |
| }; | |
| rowsAffected: insertResult.rowsAffected, | |
| }; | |
| } catch (error) { | |
| try { | |
| const [insertResult] = await db.executeSql( | |
| `INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId) | |
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | |
| [ | |
| proof.appName, | |
| proof.endpointType, | |
| proof.status, | |
| proof.errorCode || null, | |
| proof.errorReason || null, | |
| timestamp, | |
| proof.disclosures, | |
| proof.logoBase64 || null, | |
| proof.userId, | |
| proof.userIdType, | |
| proof.sessionId, | |
| proof.documentId, | |
| ], | |
| ); | |
| if (insertResult.rowsAffected === 0) { | |
| // Duplicate sessionId: return existing row | |
| const [existing] = await db.executeSql( | |
| `SELECT id, timestamp FROM ${TABLE_NAME} WHERE sessionId = ? LIMIT 1`, | |
| [proof.sessionId], | |
| ); | |
| const row = existing.rows.item(0); | |
| return { | |
| id: String(row?.id ?? 0), | |
| timestamp: row?.timestamp ?? timestamp, | |
| rowsAffected: 0, | |
| }; | |
| } | |
| return { | |
| id: String(insertResult.insertId), | |
| timestamp, | |
| rowsAffected: insertResult.rowsAffected, | |
| }; | |
| } catch (error) { | |
| // ... existing error handling ... | |
| } |
🤖 Prompt for AI Agents
In app/src/stores/database.ts around lines 113 to 137, the code assumes
insertResult.insertId always exists after an INSERT OR IGNORE; when the insert
is ignored (rowsAffected === 0) insertId can be undefined which causes
insertId.toString() to throw and returning a fresh timestamp is misleading. Fix
by checking insertResult.rowsAffected: if > 0 return insertId and the timestamp
used for insert; if === 0 query the table for the existing row using the unique
key(s) (e.g., sessionId or combination used to dedupe) to retrieve the existing
id and timestamp and return those instead; avoid calling toString() on undefined
and do not overwrite the original timestamp for duplicates.
Comment on lines
+138
to
+167
| if ((error as Error).message.includes('no column named documentId')) { | ||
| await addDocumentIdColumn(); | ||
| // Then retry the insert (copy the executeSql call here) | ||
| const [insertResult] = await db.executeSql( | ||
| `INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | ||
| [ | ||
| proof.appName, | ||
| proof.endpointType, | ||
| proof.status, | ||
| proof.errorCode || null, | ||
| proof.errorReason || null, | ||
| timestamp, | ||
| proof.disclosures, | ||
| proof.logoBase64 || null, | ||
| proof.userId, | ||
| proof.userIdType, | ||
| proof.sessionId, | ||
| proof.documentId, | ||
| ], | ||
| ); | ||
| return { | ||
| id: insertResult.insertId.toString(), | ||
| timestamp, | ||
| rowsAffected: insertResult.rowsAffected, | ||
| }; | ||
| } else { | ||
| throw error; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Migration retry is not concurrency-safe; also duplicate handling needed on retry
If another thread adds the column between your error check and ALTER, this call will throw “duplicate column name” and fail the insert. Make the migration idempotent and keep the duplicate-session fallback in the retry path.
Apply this diff:
@@
- if ((error as Error).message.includes('no column named documentId')) {
- await addDocumentIdColumn();
+ if ((error as Error).message.includes('no column named documentId')) {
+ await addDocumentIdColumn();
// Then retry the insert (copy the executeSql call here)
const [insertResult] = await db.executeSql(
`INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
[
proof.appName,
proof.endpointType,
proof.status,
proof.errorCode || null,
proof.errorReason || null,
timestamp,
proof.disclosures,
proof.logoBase64 || null,
proof.userId,
proof.userIdType,
proof.sessionId,
proof.documentId,
],
);
- return {
- id: insertResult.insertId.toString(),
- timestamp,
- rowsAffected: insertResult.rowsAffected,
- };
+ if (insertResult.rowsAffected === 0) {
+ const [existing] = await db.executeSql(
+ `SELECT id, timestamp FROM ${TABLE_NAME} WHERE sessionId = ? LIMIT 1`,
+ [proof.sessionId],
+ );
+ const row = existing.rows.item(0);
+ return {
+ id: String(row?.id ?? 0),
+ timestamp: row?.timestamp ?? timestamp,
+ rowsAffected: 0,
+ };
+ }
+ return {
+ id: String(insertResult.insertId),
+ timestamp,
+ rowsAffected: insertResult.rowsAffected,
+ };
} else {
throw error;
}📝 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
| if ((error as Error).message.includes('no column named documentId')) { | |
| await addDocumentIdColumn(); | |
| // Then retry the insert (copy the executeSql call here) | |
| const [insertResult] = await db.executeSql( | |
| `INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId) | |
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | |
| [ | |
| proof.appName, | |
| proof.endpointType, | |
| proof.status, | |
| proof.errorCode || null, | |
| proof.errorReason || null, | |
| timestamp, | |
| proof.disclosures, | |
| proof.logoBase64 || null, | |
| proof.userId, | |
| proof.userIdType, | |
| proof.sessionId, | |
| proof.documentId, | |
| ], | |
| ); | |
| return { | |
| id: insertResult.insertId.toString(), | |
| timestamp, | |
| rowsAffected: insertResult.rowsAffected, | |
| }; | |
| } else { | |
| throw error; | |
| } | |
| } | |
| if ((error as Error).message.includes('no column named documentId')) { | |
| await addDocumentIdColumn(); | |
| // Then retry the insert (copy the executeSql call here) | |
| const [insertResult] = await db.executeSql( | |
| `INSERT OR IGNORE INTO ${TABLE_NAME} (appName, endpointType, status, errorCode, errorReason, timestamp, disclosures, logoBase64, userId, userIdType, sessionId, documentId) | |
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, | |
| [ | |
| proof.appName, | |
| proof.endpointType, | |
| proof.status, | |
| proof.errorCode || null, | |
| proof.errorReason || null, | |
| timestamp, | |
| proof.disclosures, | |
| proof.logoBase64 || null, | |
| proof.userId, | |
| proof.userIdType, | |
| proof.sessionId, | |
| proof.documentId, | |
| ], | |
| ); | |
| if (insertResult.rowsAffected === 0) { | |
| const [existing] = await db.executeSql( | |
| `SELECT id, timestamp FROM ${TABLE_NAME} WHERE sessionId = ? LIMIT 1`, | |
| [proof.sessionId], | |
| ); | |
| const row = existing.rows.item(0); | |
| return { | |
| id: String(row?.id ?? 0), | |
| timestamp: row?.timestamp ?? timestamp, | |
| rowsAffected: 0, | |
| }; | |
| } | |
| return { | |
| id: String(insertResult.insertId), | |
| timestamp, | |
| rowsAffected: insertResult.rowsAffected, | |
| }; | |
| } else { | |
| throw error; | |
| } | |
| } |
Comment on lines
+185
to
+190
| async function addDocumentIdColumn() { | ||
| const db = await openDatabase(); | ||
| await db.executeSql( | ||
| `ALTER TABLE ${TABLE_NAME} ADD COLUMN documentId TEXT NOT NULL DEFAULT ''`, | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make addDocumentIdColumn idempotent to avoid “duplicate column name” races
Check PRAGMA table_info first and swallow duplicate-column errors. This prevents failures under concurrent inserts during rollout.
Apply this diff:
async function addDocumentIdColumn() {
const db = await openDatabase();
- await db.executeSql(
- `ALTER TABLE ${TABLE_NAME} ADD COLUMN documentId TEXT NOT NULL DEFAULT ''`,
- );
+ // No-op if column already exists
+ const [info] = await db.executeSql(`PRAGMA table_info(${TABLE_NAME})`);
+ for (let i = 0; i < info.rows.length; i++) {
+ if (info.rows.item(i).name === 'documentId') {
+ return;
+ }
+ }
+ try {
+ await db.executeSql(
+ `ALTER TABLE ${TABLE_NAME} ADD COLUMN documentId TEXT NOT NULL DEFAULT ''`,
+ );
+ } catch (e) {
+ if (!(e as Error).message.includes('duplicate column name')) {
+ throw e;
+ }
+ }
}📝 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
| async function addDocumentIdColumn() { | |
| const db = await openDatabase(); | |
| await db.executeSql( | |
| `ALTER TABLE ${TABLE_NAME} ADD COLUMN documentId TEXT NOT NULL DEFAULT ''`, | |
| ); | |
| } | |
| async function addDocumentIdColumn() { | |
| const db = await openDatabase(); | |
| // No-op if column already exists | |
| const [info] = await db.executeSql(`PRAGMA table_info(${TABLE_NAME})`); | |
| for (let i = 0; i < info.rows.length; i++) { | |
| if (info.rows.item(i).name === 'documentId') { | |
| return; | |
| } | |
| } | |
| try { | |
| await db.executeSql( | |
| `ALTER TABLE ${TABLE_NAME} ADD COLUMN documentId TEXT NOT NULL DEFAULT ''`, | |
| ); | |
| } catch (e) { | |
| // Swallow “duplicate column name” errors, rethrow others | |
| if (!(e as Error).message.includes('duplicate column name')) { | |
| throw e; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/stores/database.ts around lines 185 to 190, make addDocumentIdColumn
idempotent by first querying PRAGMA table_info(TABLE_NAME) and checking whether
a column named "documentId" already exists; if it exists, return early. If it
does not, run the ALTER TABLE to add the column but wrap the executeSql call in
a try/catch and swallow only errors indicating a duplicate column (or check
error message/code) so concurrent migrations won't fail; keep other errors
propagated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores