Conversation
🦋 Changeset detectedLatest commit: c3098bd 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 |
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 improves the robustness and observability of the Manteca integration. It addresses potential user friction during onboarding by providing a more resilient flow for invalid CUITs and ensures that critical user actions, such as onramp completions and account activations, are properly tracked. Additionally, it includes a practical fallback for missing document images, contributing to a smoother overall user experience. Highlights
Changelog
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
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds INVALID_LEGAL_ID handling to Manteca onboarding (maps API legalId errors to a surfaced error, supports inquiry resume/create flow), introduces Onramp and RampAccount analytics events, normalizes Manteca hook event names and credential source queries, and adds related tests and changesets. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant API as "ramp.ts (POST /onboarding)"
participant Manteca as "mantecaOnboarding\n(server/utils/ramps/manteca.ts)"
participant Persona as "persona (inquiry service)\n(server/utils/persona)"
participant Response as Response
Client->>API: POST /onboarding (payload)
API->>Manteca: initiateOnboarding()
Manteca->>Manteca: call external Manteca API
Manteca-->>API: throw INVALID_LEGAL_ID
API->>Persona: getInquiry(user)
alt existing & resumable (created/pending/expired)
Persona-->>API: return existing inquiryId
else missing or non-resumable
API->>Persona: createInquiry(...)
Persona-->>API: return new inquiryId
end
API->>Persona: resumeInquiry(inquiryId)
Persona-->>API: return sessionToken
API-->>Response: 400 { code: INVALID_LEGAL_ID, inquiryId, sessionToken }
Response-->>Client: error with resume data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/manteca.ts (1)
776-789: 🛠️ Refactor suggestion | 🟠 MajorNew entries added at the end of both objects — move to sorted/middle positions.
INVALID_CUITis appended at the end ofErrorCodes, andINVALID_LEGAL_IDat the end ofMantecaApiErrorCodes. Both should be placed alphabetically or at least not last.Proposed fix
export const ErrorCodes = { + INVALID_CUIT: "invalid cuit", + INVALID_ORDER_SIZE: "invalid order size", + MANTECA_USER_INACTIVE: "manteca user inactive", NOT_SUPPORTED_CHAIN_ID: "not supported chain id", NOT_SUPPORTED_CURRENCY: "not supported currency", - MANTECA_USER_INACTIVE: "manteca user inactive", - INVALID_ORDER_SIZE: "invalid order size", NO_PERSONA_ACCOUNT: "no persona account", NO_DOCUMENT: "no document", - INVALID_CUIT: "invalid cuit", }; const MantecaApiErrorCodes = { + INVALID_LEGAL_ID: "legalId has wrong value", INVALID_ORDER_SIZE: "MIN_SIZE", USER_NOT_FOUND: "USER_NF", - INVALID_LEGAL_ID: "legalId has wrong value", } as const;As per coding guidelines: "avoid adding items at the end of json/array lists. add in the middle or sorted position."
🤖 Fix all issues with AI agents
In `@server/test/hooks/manteca.test.ts`:
- Line 323: Tests currently spy on segment.track without stubbing its behaviour,
allowing the real track() to run and potentially send events; update the tests
that call vi.spyOn(segment, "track") (including the occurrences around the
current lines) to stub the implementation to a no-op instead of executing the
real method (e.g., after spying, set a mock implementation or replace with a
vi.fn that returns nothing) so segment.track cannot call out to analytics during
test runs.
In `@server/utils/ramps/manteca.ts`:
- Line 342: Replace the non-compliant comment marker "HACK" in the expression
using backDocumentURL ?? frontDocumentURL with a permitted TODO or FIXME marker;
change the inline comment to follow the exact format "TODO <lowercase
explanation>" (uppercase tag, single space, then a lowercase description) — e.g.
replace "HACK: onboarding starts only when two images are uploaded" with "TODO
onboarding starts only when two images are uploaded".
![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 introduces several improvements for the Manteca integration. It adds a user-friendly flow to handle invalid CUIT during onboarding by leveraging Persona inquiries, implements a fallback for the back document image to prevent onboarding failures, and adds Segment tracking for key onramp events. The changes are well-tested and improve both robustness and observability. I have one suggestion to refactor a piece of logic for better clarity and type safety.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #756 +/- ##
==========================================
+ Coverage 67.73% 67.81% +0.08%
==========================================
Files 206 206
Lines 6940 7032 +92
Branches 2175 2210 +35
==========================================
+ Hits 4701 4769 +68
- Misses 2049 2063 +14
- Partials 190 200 +10
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:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@server/api/ramp.ts`:
- Around line 211-222: The inquiry flow under the
MantecaErrorCodes.INVALID_LEGAL_ID branch (calls to getInquiry, createInquiry,
resumeInquiry) can throw and currently escapes the outer catch; wrap that whole
sequence in its own try/catch, and in the catch: capture the error to Sentry (or
the existing logger) with onboarding context (include credentialId,
error.message, and original error code), then return a controlled HTTP error
response (e.g., 502 or a 4xx with a clear message) instead of letting it bubble
as a 500. Ensure you keep the existing resume path when successful and reference
the existing symbols getInquiry, createInquiry, resumeInquiry, and
MantecaErrorCodes.INVALID_LEGAL_ID so reviewers can find the change.
In `@server/hooks/manteca.ts`:
- Line 42: The debug namespace was changed and may break existing filtering;
revert or add backward-compatibility so existing DEBUG=exa:manteca-hook
continues to capture logs: update the createDebug call (the createDebug(...)
used to initialize the const debug) to use the original "exa:manteca-hook"
namespace or create and export/assign both namespaces (e.g.,
createDebug("exa:manteca") and createDebug("exa:manteca-hook")) so callers using
the const debug still emit under the old name; adjust the const debug
initialization accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/test/api/ramp.test.ts`:
- Around line 250-335: Add positive assertions that persona.createInquiry was
called in the two INVALID_LEGAL_ID tests where a new inquiry should be created:
the "returns 400 with new inquiry for invalid legal id when no existing inquiry"
and the "creates new inquiry for invalid legal id when existing inquiry is not
resumable" tests. Concretely, after the response assertions in each test, add
expect(persona.createInquiry).toHaveBeenCalled() (or toHaveBeenCalledTimes(1) if
you prefer stricter checks) to ensure createInquiry is invoked; keep the
existing negative assertion expect(createInquirySpy).not.toHaveBeenCalled() in
the resumable test unchanged.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/api/ramp.ts`:
- Around line 211-222: TypeScript can't guarantee existing isn't undefined when
you access existing.id after computing resumable, so either narrow the type or
assert non-null; update the code around getInquiry/resumable to ensure a
definite id — simplest fix: replace existing.id with existing!.id when building
{ data: { id: existing.id } } (or alternatively add an explicit return type for
getInquiry such as Promise<InquiryType | undefined> and handle the undefined
case before using existing.id), ensuring the symbols involved are getInquiry,
existing, resumable, createInquiry, and resumeInquiry.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/test/hooks/manteca.test.ts`:
- Line 323: The spy uses mockResolvedValue() for the synchronous function
segment.track which returns void; replace mockResolvedValue() with a synchronous
mock such as vi.spyOn(segment, "track").mockImplementation(() => {}) or
.mockReturnValue(undefined) to match the void return type; apply the same change
to the other occurrence noted (around the second instance at the later test).
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores