♻️ app: use new kyc flow #653
Conversation
🦋 Changeset detectedLatest commit: 86d01f7 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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 |
Summary of ChangesHello @franm91, 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 overhauls the application's Know Your Customer (KYC) integration. The primary goal is to modernize and simplify how users interact with the KYC process, from initial verification to status checks. By introducing a new unified function for starting KYC and standardizing API responses, the changes aim to improve the robustness and maintainability of the identity verification system. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the KYC flow to use a new, unified startKYC function, which simplifies the client-side logic across several components. The changes are largely consistent and improve maintainability by centralizing the KYC initiation process. I've identified a critical regression in the web platform logic that affects mini-app functionality, and a medium-severity issue concerning redundant error reporting in one component. Addressing these points will ensure the new flow is both robust and correct.
| const embeddingContext = queryClient.getQueryData<EmbeddingContext>(["embedding-context"]); | ||
| if (embeddingContext && !embeddingContext.endsWith("-web")) { | ||
| window.location.replace(url); | ||
| window.location.replace(otl); | ||
| return; | ||
| } | ||
| window.open(url); | ||
| return; | ||
| } | ||
|
|
||
| const { Inquiry } = await import("react-native-persona"); | ||
| Inquiry.fromTemplate(KYC_TEMPLATE_ID) | ||
| .environment(environment) | ||
| .referenceId(credential.credentialId) | ||
| .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(); | ||
| } | ||
|
|
||
| export async function resumeInquiry(inquiryId: string, sessionToken: string) { | ||
| if (Platform.OS === "web") { | ||
| const url = await getKYCLink(KYC_TEMPLATE_ID); | ||
| if (await sdk.isInMiniApp()) { | ||
| await sdk.actions.openUrl(url); | ||
| return; | ||
| } | ||
| const embeddingContext = queryClient.getQueryData<EmbeddingContext>(["embedding-context"]); | ||
| if (embeddingContext && !embeddingContext.endsWith("-web")) { | ||
| window.location.replace(url); | ||
| await sdk.actions.openUrl(otl); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The order of checks for the web platform is incorrect and introduces a regression for mini-apps. The check for embeddingContext now comes before the sdk.isInMiniApp() check. In a mini-app environment, embeddingContext can be a value like "farcaster", which doesn't end with "-web". This would cause window.location.replace(otl) to be called, which is incorrect for mini-apps. The correct behavior is to use sdk.actions.openUrl(otl).
To fix this, the sdk.isInMiniApp() check should be performed before the embeddingContext check, as it was in the previous implementation.
| const embeddingContext = queryClient.getQueryData<EmbeddingContext>(["embedding-context"]); | |
| if (embeddingContext && !embeddingContext.endsWith("-web")) { | |
| window.location.replace(url); | |
| window.location.replace(otl); | |
| return; | |
| } | |
| window.open(url); | |
| return; | |
| } | |
| const { Inquiry } = await import("react-native-persona"); | |
| Inquiry.fromTemplate(KYC_TEMPLATE_ID) | |
| .environment(environment) | |
| .referenceId(credential.credentialId) | |
| .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(); | |
| } | |
| export async function resumeInquiry(inquiryId: string, sessionToken: string) { | |
| if (Platform.OS === "web") { | |
| const url = await getKYCLink(KYC_TEMPLATE_ID); | |
| if (await sdk.isInMiniApp()) { | |
| await sdk.actions.openUrl(url); | |
| return; | |
| } | |
| const embeddingContext = queryClient.getQueryData<EmbeddingContext>(["embedding-context"]); | |
| if (embeddingContext && !embeddingContext.endsWith("-web")) { | |
| window.location.replace(url); | |
| await sdk.actions.openUrl(otl); | |
| return; | |
| } | |
| if (await sdk.isInMiniApp()) { | |
| await sdk.actions.openUrl(otl); | |
| return; | |
| } | |
| const embeddingContext = queryClient.getQueryData<EmbeddingContext>(["embedding-context"]); | |
| if (embeddingContext && !embeddingContext.endsWith("-web")) { | |
| window.location.replace(otl); | |
| return; | |
| } |
| } catch (error) { | ||
| if (!(error instanceof APIError)) { | ||
| reportError(error); | ||
| return; | ||
| throw error; | ||
| } | ||
| if (error.text === "kyc required" || error.text === "kyc not found" || error.text === "kyc not started") { | ||
| await createInquiry(credential); | ||
| return; | ||
| if (error.text !== "not started" && error.text !== "no kyc") { | ||
| reportError(error); | ||
| throw error; | ||
| } | ||
| reportError(error); | ||
| } |
There was a problem hiding this comment.
The reportError calls within the catch block of the mutationFn are redundant. Since the errors are re-thrown, they will be handled by the onError callback of the useMutation hook, which also calls reportError. This results in double reporting of the same error.
To fix this, you should remove the reportError calls from within the mutationFn's catch block.
} catch (error) {
if (!(error instanceof APIError)) {
throw error;
}
if (error.text !== "not started" && error.text !== "no kyc") {
throw error;
}
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## kyc #653 +/- ##
==========================================
- Coverage 68.20% 67.33% -0.87%
==========================================
Files 41 30 -11
Lines 1346 1292 -54
Branches 337 332 -5
==========================================
- Hits 918 870 -48
+ Misses 276 271 -5
+ Partials 152 151 -1
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:
|
No description provided.