Improve Validation Error Messages For Banking Info#482
Improve Validation Error Messages For Banking Info#482Derek Siemens (DerekSiemens) wants to merge 4 commits intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for richer, (optionally) translatable validation error messages for the Tax & Cash “Banking Info” form by mapping Finance/Impact API validation errors into frontend-friendly error codes and formatting per-field ICU templates.
Changes:
- Map API validation error
fieldnames to form field names and map API errormessagestrings to short frontend error codes. - Extend the form error shape to carry an
errorCode, and plumb it through to validation help text rendering. - Introduce per-field ICU
selecttemplates (as component props) to render more specific error messages.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/useBankingInfoForm.tsx | Adds mapping from API field/message to form field + errorCode and attaches errorCode onto per-field errors. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx | Adds per-field ICU error message props + uses them in getValidationErrorMessage when errorCode is available. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form-view.tsx | Extends the form error typing to include optional errorCode and adds errorMessages to text props. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx | Passes errorCode + fieldName into the validation message formatter for each input. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (errorTemplate) { | ||
| return intl.formatMessage( | ||
| { | ||
| id: `fieldError-${fieldName}-${errorCode}`, |
There was a problem hiding this comment.
In getValidationErrorMessage, the intl.formatMessage id is built from both fieldName and errorCode. Since errorCode can be the raw API message when it’s not mapped, this creates effectively unbounded/dynamic message IDs (including spaces/punctuation), which makes translation management and caching unpredictable. Consider using a stable id per field (e.g. fieldError-${fieldName}) and rely on the ICU select in defaultMessage to vary by errorCode instead of encoding errorCode into the id.
| id: `fieldError-${fieldName}-${errorCode}`, | |
| id: `fieldError-${fieldName}`, |
| // ────────────────────────────────────────────────────────────────── | ||
| // Per-field validation error messages | ||
| // Each prop uses ICU select on {errorCode} to pick the right message. | ||
| // Error codes are short frontend keys mapped from the API error codes. |
There was a problem hiding this comment.
The banner comment says these frontend error codes are “mapped from the API error codes”, but in useBankingInfoForm they’re derived from (and keyed by) the Impact API message strings. Tweaking this wording to “mapped from API error messages” would better reflect the actual behavior and avoid confusion for future maintainers.
| // Error codes are short frontend keys mapped from the API error codes. | |
| // Error codes are short frontend keys mapped from API error messages. |
| * If a message doesn't match any key here, it passes through as-is to the ICU select's | ||
| * `other` branch, which displays it directly via `{errorCode}`. | ||
| */ | ||
| const API_MESSAGE_TO_FRONTEND: Record<string, string> = { |
There was a problem hiding this comment.
I do not think this is a good approach but its the best the AI can do with the current backend implementation.
The graphql API returns the field and the error message. But we need the error message to be translatable, I got it to setup this map so the ICU message can be nicer but this is so fragile... If the payments squad changes their text then our frontend would default to our fallback in the ICU message.
I think a better approach would be to return the error path from the graphql API. EX, this is coming back from the Finance API withdrawal.settings.error.patronymicName.alphanumeric. We can then use this as a much more stable data point for the ICU message translatability. The backend of course can remove some elements out of that path as needed
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If we have a specific error code from the API, try to use | ||
| // the per-field ICU error message template for a rich message | ||
| if (type === "invalid" && errorCode && fieldName) { | ||
| const errorTemplate = props.text.errorMessages?.[fieldName]; | ||
| if (errorTemplate) { | ||
| return intl.formatMessage( | ||
| { | ||
| id: `fieldError-${fieldName}-${errorCode}`, | ||
| defaultMessage: errorTemplate, | ||
| }, | ||
| { | ||
| errorCode, | ||
| fieldName: label, | ||
| } |
There was a problem hiding this comment.
intl.formatMessage message id is derived from errorCode (fieldError-${fieldName}-${errorCode}). If errorCode can ever be a raw API message (as implied by the templates’ other branch), this produces unstable/possibly invalid ids (spaces/punctuation) and can create unbounded ids at runtime. Prefer a stable id that does not include the raw error string (e.g., keyed only by fieldName) and keep errorCode purely as a select discriminator/value.
| const mappedValidationErrors = validationErrors?.reduce( | ||
| (agg, error) => { | ||
| const formField = | ||
| API_FIELD_TO_FORM_FIELD[error.field] || error.field; | ||
| const errorCode = | ||
| API_ERROR_PATH_TO_FRONTEND[error.errorPath] || error.errorPath; | ||
| return { | ||
| ...agg, | ||
|
|
||
| [error.field]: { | ||
| [formField]: { | ||
| type: "invalid", | ||
| errorCode, | ||
| }, |
There was a problem hiding this comment.
When mapping backend validation errors, the fallback for an unmapped errorPath sets errorCode to error.errorPath. Downstream, the ICU templates’ other branch is documented as showing the raw API message, so this fallback will display a technical path rather than a user-facing message. Consider using error.message as the fallback display text (and keep errorCode as the mapped short code), or store both errorCode and message separately in inputErrors.
| helpText: getValidationErrorMessage({ | ||
| type: errors?.inputErrors?.beneficiaryTaxPayerId?.type, | ||
| label: props.text.taxPayerIdLabel, | ||
| errorCode: errors?.inputErrors?.taxPayerId?.errorCode, |
There was a problem hiding this comment.
For the beneficiaryTaxPayerId input, errorCode is being read from errors.inputErrors.taxPayerId, while the error state guard/type uses errors.inputErrors.beneficiaryTaxPayerId. This mismatch will typically drop the API errorCode (so you fall back to the generic invalid message). Use the same key consistently for type and errorCode (and only use fieldName: "taxPayerId" if you intentionally want to select the tax-payer-id template).
| errorCode: errors?.inputErrors?.taxPayerId?.errorCode, | |
| errorCode: errors?.inputErrors?.beneficiaryTaxPayerId?.errorCode, |
Description of the change
Add in translatable error messages from the Finance API for banking information.
Type of change
Links
Checklists
Development
Paperwork
Code review