Skip to content

Make umaAddress optional in create user endpoint#249

Closed
pengying wants to merge 1 commit intomainfrom
claude/slack-session-Ka8XL
Closed

Make umaAddress optional in create user endpoint#249
pengying wants to merge 1 commit intomainfrom
claude/slack-session-Ka8XL

Conversation

@pengying
Copy link
Copy Markdown
Contributor

This allows users to be created without an UMA address, which will be auto-generated by the system. Needed for SoFi's FX discovery flow where users don't provide an UMA address upfront.

https://claude.ai/code/session_01S9UjiuUhTGRtySPbk9Nyon

This allows users to be created without an UMA address, which will
be auto-generated by the system. Needed for SoFi's FX discovery flow
where users don't provide an UMA address upfront.

https://claude.ai/code/session_01S9UjiuUhTGRtySPbk9Nyon
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 11, 2026

Greptile Summary

This PR removes umaAddress from the required list in User.yaml to support user creation without an upfront UMA address (auto-generated by the system), needed for SoFi's FX discovery flow.

Two minor follow-up items worth noting: (1) the POST /users endpoint description in users.yaml still reads "with UMA address and bank account information" and should be updated to reflect the field is now optional; (2) the 409 error description ("Conflict - User with the UMA address already exists") may need revisiting for the auto-generation case where conflict detection would fall back to platformUserId.

Confidence Score: 4/5

Safe to merge after considering the response schema accuracy concern — auto-generated clients will treat umaAddress as absent-able in responses.

All findings are P2, but the response schema concern has a mild correctness angle (generated clients may not expect umaAddress in every response when the system always populates it), warranting a 4 rather than 5 until the team confirms the behaviour is intentional or the schemas are split.

openapi/components/schemas/users/User.yaml — shared request/response schema now marks umaAddress optional in both contexts.

Important Files Changed

Filename Overview
openapi/components/schemas/users/User.yaml Removes umaAddress from required, making it optional in both request and response schemas; missing auto-generation documentation on the property description.

Sequence Diagram

sequenceDiagram
    participant Client
    participant API as POST /users
    participant System

    alt umaAddress provided
        Client->>API: { platformUserId, userType, umaAddress, ... }
        API->>System: Create user with supplied umaAddress
        System-->>API: User record (umaAddress = supplied value)
        API-->>Client: 201 { umaAddress, platformUserId, ... }
    else umaAddress omitted (new behaviour)
        Client->>API: { platformUserId, userType, ... }
        API->>System: Create user without umaAddress
        System-->>API: User record (umaAddress = auto-generated)
        API-->>Client: 201 { umaAddress?, platformUserId, ... }
        Note over API,Client: Schema marks umaAddress optional in response too
    end
Loading

Comments Outside Diff (1)

  1. openapi/components/schemas/users/User.yaml, line 11-14 (link)

    P2 Missing auto-generation documentation

    The umaAddress description doesn't explain what happens when the field is omitted. Since the PR purpose is to support auto-generation (per the PR description), API consumers reading the spec won't know a UMA address is automatically assigned when none is provided.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: openapi/components/schemas/users/User.yaml
    Line: 11-14
    
    Comment:
    **Missing auto-generation documentation**
    
    The `umaAddress` description doesn't explain what happens when the field is omitted. Since the PR purpose is to support auto-generation (per the PR description), API consumers reading the spec won't know a UMA address is automatically assigned when none is provided.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Claude Code

Fix All in Claude Code

Prompt To Fix All With AI
This is a comment left during a code review.
Path: openapi/components/schemas/users/User.yaml
Line: 11-14

Comment:
**Missing auto-generation documentation**

The `umaAddress` description doesn't explain what happens when the field is omitted. Since the PR purpose is to support auto-generation (per the PR description), API consumers reading the spec won't know a UMA address is automatically assigned when none is provided.

```suggestion
  umaAddress:
    type: string
    description: Full UMA address. If not provided on creation, a UMA address will be auto-generated by the system.
    example: $john.doe@uma.domain.com
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: openapi/components/schemas/users/User.yaml
Line: 2-4

Comment:
**Response schema also marks `umaAddress` as optional**

`User.yaml` is used for both the `POST /users` request body and the `201` response (via `IndividualUser.yaml` / `BusinessUser.yaml`). Removing `umaAddress` from `required` here means the 201 response schema now also declares it optional. If the system always returns an auto-generated address in the response, the response schema should still mark it required — otherwise generated clients will defensively treat `umaAddress` as potentially absent from every response.

Consider splitting into separate request/response schemas (e.g. `CreateUserRequest.yaml` with `umaAddress` optional, and the existing `User.yaml` keeping `umaAddress` required in `required`), or at minimum add a note in the response schema that `umaAddress` is always populated.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Make umaAddress optional in create user ..." | Re-trigger Greptile

Comment on lines 2 to 4
required:
- umaAddress
- platformUserId
- userType
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Response schema also marks umaAddress as optional

User.yaml is used for both the POST /users request body and the 201 response (via IndividualUser.yaml / BusinessUser.yaml). Removing umaAddress from required here means the 201 response schema now also declares it optional. If the system always returns an auto-generated address in the response, the response schema should still mark it required — otherwise generated clients will defensively treat umaAddress as potentially absent from every response.

Consider splitting into separate request/response schemas (e.g. CreateUserRequest.yaml with umaAddress optional, and the existing User.yaml keeping umaAddress required in required), or at minimum add a note in the response schema that umaAddress is always populated.

Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/users/User.yaml
Line: 2-4

Comment:
**Response schema also marks `umaAddress` as optional**

`User.yaml` is used for both the `POST /users` request body and the `201` response (via `IndividualUser.yaml` / `BusinessUser.yaml`). Removing `umaAddress` from `required` here means the 201 response schema now also declares it optional. If the system always returns an auto-generated address in the response, the response schema should still mark it required — otherwise generated clients will defensively treat `umaAddress` as potentially absent from every response.

Consider splitting into separate request/response schemas (e.g. `CreateUserRequest.yaml` with `umaAddress` optional, and the existing `User.yaml` keeping `umaAddress` required in `required`), or at minimum add a note in the response schema that `umaAddress` is always populated.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code

@pengying pengying closed this Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants