-
Notifications
You must be signed in to change notification settings - Fork 2
✨ server: validate id document photos for manteca #693
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
Conversation
🦋 Changeset detectedLatest commit: de60df2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #693 +/- ##
==========================================
+ Coverage 63.73% 64.01% +0.28%
==========================================
Files 169 169
Lines 5324 5324
Branches 1501 1501
==========================================
+ Hits 3393 3408 +15
+ Misses 1744 1730 -14
+ Partials 187 186 -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:
|
Summary of ChangesHello @mainqueg, 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 identity document verification process for Manteca users by introducing a more rigorous check for 'id' class documents. it ensures that such documents include both front and back photos, and intelligently falls back to passport documents if the primary ID is incomplete. This change improves the accuracy and reliability of identity verification, streamlining the onboarding experience while maintaining necessary security standards. 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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughMakes persona document-selection and Manteca validation asynchronous: Changes
Sequence Diagram(s)(Skipped — changes are an internal async refactor and tests; no new multi-component runtime sequence diagram produced.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 introduces validation for ID document photos for Manteca onboarding by making evaluateAccount and getDocumentForManteca asynchronous. This allows fetching document details to verify the presence of front and back photos for 'id' type documents, with a fallback mechanism to other document types. The changes are logical, and the accompanying tests are thorough, covering the new asynchronous flows and validation logic. I have one suggestion to refactor a function for improved clarity and efficiency.
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.
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 `@server/test/utils/persona.test.ts`:
- Around line 5-12: The test suite creates a spy named fetchSpy used in the
"evaluateAccount" tests and while it is reset in beforeEach, it is not restored
after the suite finishes; add an afterAll hook for the evaluateAccount suite
that calls fetchSpy.mockRestore() to properly clean up the spy (locate the
evaluateAccount describe block and add afterAll(() => fetchSpy.mockRestore()) so
the spy lifecycle is completed).
In `@server/utils/persona.ts`:
- Around line 473-494: getValidDocumentForManteca currently calls
getDocument(...) for "id" documents but does not handle exceptions, so any
thrown error prevents falling back to other allowedIds; wrap the getDocument
call in a try/catch, and on error treat it like a missing document (log/debug
the error if desired) and continue the loop to allow returning other documents
from documents (referencing getValidDocumentForManteca, getDocument,
AccountBasicFields, IdentityDocument).
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: 1
🤖 Fix all issues with AI agents
In `@server/test/utils/persona.test.ts`:
- Around line 480-485: The "get document for manteca" test suite creates a
fetchSpy via vi.spyOn(globalThis, "fetch") but never restores it; add an
afterAll hook to restore the spy (call fetchSpy.mockRestore()) to avoid leaking
the mocked global fetch across tests, keeping the existing beforeEach mockReset
intact; reference the describe block name "get document for manteca" and the
fetchSpy variable so you can locate and update the suite.
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.