-
Notifications
You must be signed in to change notification settings - Fork 7
feat(customers): split BusinessInfo response schema (taxId/incorporatedOn optional) #524
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||||||||||||||||||
| type: object | ||||||||||||||||||||||
| description: | | ||||||||||||||||||||||
| Business information returned on a customer. `taxId` and `incorporatedOn` are | ||||||||||||||||||||||
| required on creation but may be absent on legacy customers that pre-date the | ||||||||||||||||||||||
| requirement, so both are optional in responses. | ||||||||||||||||||||||
| required: | ||||||||||||||||||||||
| - legalName | ||||||||||||||||||||||
| properties: | ||||||||||||||||||||||
| legalName: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: Legal name of the business | ||||||||||||||||||||||
| example: Acme Corporation, Inc. | ||||||||||||||||||||||
| doingBusinessAs: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: Trade name or DBA name of the business, if different from the legal name | ||||||||||||||||||||||
| example: Acme | ||||||||||||||||||||||
| country: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: Country of incorporation or registration (ISO 3166-1 alpha-2) | ||||||||||||||||||||||
| example: US | ||||||||||||||||||||||
| registrationNumber: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: Business registration number | ||||||||||||||||||||||
| example: '5523041' | ||||||||||||||||||||||
| incorporatedOn: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| format: date | ||||||||||||||||||||||
| description: Date of incorporation in ISO 8601 format (YYYY-MM-DD) | ||||||||||||||||||||||
| example: '2018-03-14' | ||||||||||||||||||||||
| entityType: | ||||||||||||||||||||||
| $ref: ./EntityType.yaml | ||||||||||||||||||||||
| taxId: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: Tax identification number | ||||||||||||||||||||||
| example: 47-1234567 | ||||||||||||||||||||||
|
Comment on lines
+32
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The PR description notes that legacy DB rows have
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: openapi/components/schemas/customers/BusinessInfoResponse.yaml
Line: 32-35
Comment:
**Nullable vs. absent — `taxId` may need `type: 'null'` in OAS 3.1**
The PR description notes that legacy DB rows have `null` values for `tax_id` and `incorporated_on`. Removing these fields from `required` allows them to be *absent*, but in OpenAPI 3.1.0 (which this spec uses) an absent field is not the same as a `null` value — `null` is only valid when the schema explicitly permits it. If the API serialises legacy rows by writing `taxId: null` rather than omitting the key, any strictly-conforming OAS 3.1 client or validator will still reject the response. The existing codebase pattern (see `Card.yaml` `stateReason`) uses `oneOf: [{...}, {type: 'null'}]` for exactly this case.
```suggestion
taxId:
oneOf:
- type: string
description: Tax identification number
example: 47-1234567
- type: 'null'
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||
| countriesOfOperation: | ||||||||||||||||||||||
| type: array | ||||||||||||||||||||||
| items: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: List of countries where the business operates (ISO 3166-1 alpha-2) | ||||||||||||||||||||||
| example: | ||||||||||||||||||||||
| - US | ||||||||||||||||||||||
| businessType: | ||||||||||||||||||||||
| $ref: ./BusinessType.yaml | ||||||||||||||||||||||
| purposeOfAccount: | ||||||||||||||||||||||
| $ref: ./PurposeOfAccount.yaml | ||||||||||||||||||||||
| sourceOfFunds: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: The primary source of funds for the business | ||||||||||||||||||||||
| example: Funds derived from customer payments for software services | ||||||||||||||||||||||
| expectedMonthlyTransactionCount: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| enum: | ||||||||||||||||||||||
| - COUNT_UNDER_10 | ||||||||||||||||||||||
| - COUNT_10_TO_100 | ||||||||||||||||||||||
| - COUNT_100_TO_500 | ||||||||||||||||||||||
| - COUNT_500_TO_1000 | ||||||||||||||||||||||
| - COUNT_OVER_1000 | ||||||||||||||||||||||
| description: Expected number of transactions per month | ||||||||||||||||||||||
| example: COUNT_100_TO_500 | ||||||||||||||||||||||
| expectedMonthlyTransactionVolume: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| enum: | ||||||||||||||||||||||
| - VOLUME_UNDER_10K | ||||||||||||||||||||||
| - VOLUME_10K_TO_100K | ||||||||||||||||||||||
| - VOLUME_100K_TO_1M | ||||||||||||||||||||||
| - VOLUME_1M_TO_10M | ||||||||||||||||||||||
| - VOLUME_OVER_10M | ||||||||||||||||||||||
| description: Expected total transaction volume per month in USD equivalent | ||||||||||||||||||||||
| example: VOLUME_100K_TO_1M | ||||||||||||||||||||||
| expectedRecipientJurisdictions: | ||||||||||||||||||||||
| type: array | ||||||||||||||||||||||
| items: | ||||||||||||||||||||||
| type: string | ||||||||||||||||||||||
| description: List of countries where the business expects to send payments (ISO 3166-1 alpha-2) | ||||||||||||||||||||||
| example: | ||||||||||||||||||||||
| - US | ||||||||||||||||||||||
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.
incorporatedOnalso needs nullable annotationSame issue as
taxId: if legacy records serialise this field asnullrather than omitting it, OAS 3.1 validators and generated clients will reject the response becausenullis not a validdate-formatted string.Prompt To Fix With AI