-
Notifications
You must be signed in to change notification settings - Fork 2
✨ app: implement persona web sdk #683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: lifi
Are you sure you want to change the base?
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## lifi #683 +/- ##
==========================================
- Coverage 61.37% 61.31% -0.06%
==========================================
Files 170 170
Lines 5364 5377 +13
Branches 1517 1516 -1
==========================================
+ Hits 3292 3297 +5
- Misses 1900 1908 +8
Partials 172 172
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Know Your Customer (KYC) verification process by integrating the Persona Web SDK. The primary goal is to provide a more seamless and embedded experience for web users, moving away from external redirects. The changes involve a clear separation of concerns for web and native platforms within the KYC initiation logic, ensuring that each environment leverages its optimal Persona SDK implementation. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughThe PR adds a new "persona" dependency and refactors the KYC initialization flow in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request implements the Persona Web SDK for the KYC process, which is a great addition. The implementation correctly handles platform-specific logic for web and native environments. No specific code review comments are being provided at this time, as the suggested refactoring for startKYC might be considered a premature abstraction according to repository guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export const startKYC = ( | ||
| Platform.OS === "web" | ||
| ? () => { | ||
| let activeClient: InstanceType<typeof PersonaWeb.Client> | undefined; | ||
|
|
||
| const { Inquiry } = await import("react-native-persona"); | ||
| Inquiry.fromInquiry(inquiryId) | ||
| .sessionToken(sessionToken) | ||
| .onCanceled(() => { | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| router.replace("/(main)/(home)"); | ||
| }) | ||
| .onComplete(() => { | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| queryClient.setQueryData(["card-upgrade"], 1); | ||
| router.replace("/(main)/(home)"); | ||
| }) | ||
| .onError((error) => reportError(error)) | ||
| .build() | ||
| .start(); | ||
| } | ||
| return async () => { | ||
| const [{ Client }, { inquiryId, sessionToken }] = await Promise.all([ | ||
| import("persona"), | ||
| getKYCTokens("basic", await getRedirectURI()), | ||
| ]); | ||
|
|
||
| activeClient?.destroy(); | ||
|
|
||
| activeClient = new Client({ | ||
| inquiryId, | ||
| sessionToken, | ||
| environment: environment as "production" | "sandbox", // TODO deprecated - use environmentId instead | ||
| // environmentId: "", | ||
| onReady: () => activeClient?.open(), | ||
| onComplete: () => { | ||
| activeClient?.destroy(); | ||
| activeClient = undefined; | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| queryClient.setQueryData(["card-upgrade"], 1); // TODO probably not needed unless the user is upgrading their card | ||
| router.replace("/(main)/(home)"); | ||
| }, | ||
| onCancel: () => { | ||
| activeClient?.destroy(); | ||
| activeClient = undefined; | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| router.replace("/(main)/(home)"); | ||
| }, | ||
| onError: (error) => { | ||
| activeClient?.destroy(); | ||
| activeClient = undefined; | ||
| reportError(error); | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 KYC flow broken for Farcaster mini apps and embedded web contexts
The refactored startKYC function removes special handling for mini apps and embedded contexts that was present in the old implementation.
Click to expand
Previous Behavior
The old implementation at src/utils/persona.ts had explicit handling for different web contexts:
if (Platform.OS === "web") {
if (await sdk.isInMiniApp()) {
await sdk.actions.openUrl(oneTimeLink); // Mini app specific handling
return;
}
const embeddingContext = queryClient.getQueryData<EmbeddingContext>(["embedding-context"]);
if (embeddingContext && !embeddingContext.endsWith("-web")) {
window.location.replace(oneTimeLink); // Embedded context handling
return;
}
window.open(oneTimeLink, "_blank", "noopener,noreferrer");
}New Behavior
The new implementation uses the Persona Web SDK's Client directly for all web contexts without checking if the user is in a mini app or embedded context.
Impact
-
Farcaster mini apps: The Persona Web SDK likely cannot open its modal/iframe properly in mini app environments where popup/iframe capabilities are restricted. The old code used
sdk.actions.openUrl()to properly navigate within the mini app context. -
Non-web embedded contexts (e.g.,
farcaster,base,metamask,phantom): These contexts previously usedwindow.location.replace()to navigate to the one-time link, which is now not handled at all.
Users in these contexts will likely experience a broken KYC flow where the Persona modal fails to open or displays incorrectly.
Was this helpful? React with 👍 or 👎 to provide feedback.
6dfbe9a to
1442060
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 95: The Persona v5 client is being initialized with the deprecated
environment field; update the Persona client construction in
src/utils/persona.ts so that new Persona.Client(...) is passed environmentId
instead of environment. Locate the function or constant that calls new
Persona.Client (look for symbols like PersonaClient, createPersonaClient, or the
file-exported client instance), replace the environment:
process.env.PERSONA_ENVIRONMENT (or similar) with environmentId:
process.env.PERSONA_ENVIRONMENT_ID (or the appropriate env var containing the
env_... token), and remove the deprecated environment property so the client
uses the required environmentId parameter.
In `@src/utils/persona.ts`:
- Around line 37-72: The shared startKYC utility currently mutates
queryClient.setQueryData(["card-upgrade"], 1) in both web and native branches
(see the onComplete handlers in persona.ts), which wrongly affects all callers;
remove those setQueryData calls from persona.ts and instead apply the mutation
only in the card-upgrade flow by adding the call inside the
card-upgrade-specific caller (VerifyIdentity component) after the startKYC
promise resolves (or alternatively add an explicit boolean parameter to startKYC
like applyCardUpgrade that defaults to false and only setQueryData when true),
ensuring other callers (useBeginKYC, Card) are unaffected.
♻️ Duplicate comments (1)
src/utils/persona.ts (1)
14-57: Web KYC flow still lacks miniapp/embedded fallback.The web branch now always instantiates the Persona Web Client; in Farcaster mini apps or embedded contexts this can fail to open due to iframe/popup restrictions. Consider restoring context checks and fallback navigation (e.g., miniapp openUrl or full redirect).
Persona Web SDK support for embedded webviews / Farcaster mini apps and recommended integration when popups/iframes are restricted
| "i18n-iso-countries": "^7.14.0", | ||
| "i18next": "^25.7.4", | ||
| "moti": "^0.30.0", | ||
| "persona": "^5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Persona Web SDK npm v5 Expo React Native compatibility environment vs environmentId init options
💡 Result:
persona(Web/Embedded Flow) v5 (npm i persona): useenvironmentId(anenv_...token) when constructingnew Persona.Client(...). This is the current, non-deprecated way to choose the Persona API environment. Example shown in the Embedded Flow quickstart. [1]environmentoption: this is the deprecated legacy field; it used string values like"sandbox"/"production"and is documented as “Deprecated. UseenvironmentIdinstead.” [2]
Expo / React Native compatibility
- The Web SDK is meant for web (DOM) environments, not a native React Native runtime. For a mobile app, Persona recommends using their native mobile SDKs / React Native SDK instead. [3][4]
- If you must run Persona in an in-app browser, use a WebView/hosted flow and include
is-webview=truein the URL (required for WebView flow). [4]
References
[1] Persona Embedded Flow Quickstart (NPM) – environmentId in Persona.Client init: (docs.withpersona.com)
[2] Persona “Parameters” docs – environment deprecated, use environmentId: (docs.withpersona.com)
[3] Persona React Native Integration Guide (native SDK usage): (docs.withpersona.com)
[4] Persona WebView integration – native SDKs recommended; is-webview=true required: (docs.withpersona.com)
Citations:
- 1: https://docs.withpersona.com/quickstart-embedded-flow?utm_source=openai
- 2: https://docs.withpersona.com/v2022-09-01/docs/parameters?utm_source=openai
- 3: https://docs.withpersona.com/react-native-sdk-v2-integration-guide?utm_source=openai
- 4: https://docs.withpersona.com/webview-integration?utm_source=openai
🏁 Script executed:
# Search for Persona SDK usage in the codebase
rg -A 5 -B 5 "persona" package.json | head -50Repository: exactly/exa
Length of output: 768
🏁 Script executed:
# Look for Persona initialization code
rg -A 10 "new Persona|Persona\(" --type ts --type tsx --type js --type jsxRepository: exactly/exa
Length of output: 82
🏁 Script executed:
# Check for any persona API version constants or configuration
rg "PERSONA_API_VERSION|Persona.*init|persona.*init" -A 5 --type ts --type tsxRepository: exactly/exa
Length of output: 82
🏁 Script executed:
# Search for Persona initialization/client instantiation
rg -i "new Persona|Persona\.Client|import.*persona" -A 3 | head -100Repository: exactly/exa
Length of output: 4274
🏁 Script executed:
# Look for any persona config or environment setup
rg -i "environmentId|environment.*persona|PERSONA" -B 2 -A 2 | head -100Repository: exactly/exa
Length of output: 6081
🏁 Script executed:
# Check what files import persona package (non react-native)
rg "from ['\"]persona['\"]|import.*['\"]persona['\"]" -lRepository: exactly/exa
Length of output: 75
Update Persona v5 initialization: use environmentId instead of deprecated environment field.
The codebase imports Persona v5 but initializes it using the deprecated environment field ("sandbox"/"production"). Persona v5 requires environmentId (an env_... token). Update src/utils/persona.ts to pass environmentId when constructing new Persona.Client(...) to avoid compatibility failures with v5.
🤖 Prompt for AI Agents
In `@package.json` at line 95, The Persona v5 client is being initialized with the
deprecated environment field; update the Persona client construction in
src/utils/persona.ts so that new Persona.Client(...) is passed environmentId
instead of environment. Locate the function or constant that calls new
Persona.Client (look for symbols like PersonaClient, createPersonaClient, or the
file-exported client instance), replace the environment:
process.env.PERSONA_ENVIRONMENT (or similar) with environmentId:
process.env.PERSONA_ENVIRONMENT_ID (or the appropriate env var containing the
env_... token), and remove the deprecated environment property so the client
uses the required environmentId parameter.
| onComplete: () => { | ||
| activeClient?.destroy(); | ||
| activeClient = undefined; | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| queryClient.setQueryData(["card-upgrade"], 1); // TODO probably not needed unless the user is upgrading their card | ||
| router.replace("/(main)/(home)"); | ||
| }, | ||
| onCancel: () => { | ||
| activeClient?.destroy(); | ||
| activeClient = undefined; | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| router.replace("/(main)/(home)"); | ||
| }, | ||
| onError: (error) => { | ||
| activeClient?.destroy(); | ||
| activeClient = undefined; | ||
| reportError(error); | ||
| }, | ||
| }); | ||
| }; | ||
| } | ||
| : () => { | ||
| // eslint-disable-next-line unicorn/consistent-function-scoping | ||
| return async () => { | ||
| const { inquiryId, sessionToken } = await getKYCTokens("basic", await getRedirectURI()); | ||
| const { Inquiry } = await import("react-native-persona"); | ||
| Inquiry.fromInquiry(inquiryId) | ||
| .sessionToken(sessionToken) | ||
| .onCanceled(() => { | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| router.replace("/(main)/(home)"); | ||
| }) | ||
| .onComplete(() => { | ||
| queryClient.invalidateQueries({ queryKey: ["kyc", "status"] }).catch(reportError); | ||
| queryClient.setQueryData(["card-upgrade"], 1); | ||
| router.replace("/(main)/(home)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts --type=tsx 'startKYC\s*\(' -A 3 -B 3Repository: exactly/exa
Length of output: 82
🏁 Script executed:
rg -n 'startKYC\s*\(' -A 3 -B 3Repository: exactly/exa
Length of output: 1494
🏁 Script executed:
cat src/utils/persona.tsRepository: exactly/exa
Length of output: 3642
🏁 Script executed:
cat src/utils/useBeginKYC.tsRepository: exactly/exa
Length of output: 1258
🏁 Script executed:
cat src/components/card/Card.tsx | head -210 | tail -30Repository: exactly/exa
Length of output: 848
🏁 Script executed:
head -50 src/components/card/Card.tsxRepository: exactly/exa
Length of output: 2010
🏁 Script executed:
rg -n "card|Card" src/components/card/Card.tsx | head -5Repository: exactly/exa
Length of output: 351
🏁 Script executed:
rg -n "^export\s+(default\s+)?.*Card|^function Card|^const Card" src/components/card/Card.tsxRepository: exactly/exa
Length of output: 90
🏁 Script executed:
rg -n "from.*card/Card|import.*Card.*from.*card/Card" --type js --type tsRepository: exactly/exa
Length of output: 144
🏁 Script executed:
rg -n "GettingStarted|getting-started" --max-count 20Repository: exactly/exa
Length of output: 973
🏁 Script executed:
cat src/components/home/card-upgrade/VerifyIdentity.tsxRepository: exactly/exa
Length of output: 3084
Move setQueryData(["card-upgrade"], 1) calls out of the shared startKYC utility.
The shared utility unconditionally sets ["card-upgrade"] on completion in both web (line 41) and native (line 71) branches, affecting all callers: useBeginKYC (onboarding), Card (general card KYC), and VerifyIdentity (card-upgrade). This mutation should only apply to card-upgrade flows. The existing TODO comment ("probably not needed unless the user is upgrading their card") reflects this concern. Move these setQueryData calls into the card-upgrade-specific caller (VerifyIdentity.tsx) or accept the mutation through a parameter/context to explicitly gate it to relevant flows.
🤖 Prompt for AI Agents
In `@src/utils/persona.ts` around lines 37 - 72, The shared startKYC utility
currently mutates queryClient.setQueryData(["card-upgrade"], 1) in both web and
native branches (see the onComplete handlers in persona.ts), which wrongly
affects all callers; remove those setQueryData calls from persona.ts and instead
apply the mutation only in the card-upgrade flow by adding the call inside the
card-upgrade-specific caller (VerifyIdentity component) after the startKYC
promise resolves (or alternatively add an explicit boolean parameter to startKYC
like applyCardUpgrade that defaults to false and only setQueryData when true),
ensuring other callers (useBeginKYC, Card) are unaffected.
Summary by CodeRabbit
New Features
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.