Conversation
…o cash-reward-redemption
There was a problem hiding this comment.
Pull request overview
This PR enhances the Tax & Cash banking info form by introducing structured, per-field validation error handling based on Impact API validation metadata, and adds a new “Partner Info Modal” component to the Mint component library/stencilbook.
Changes:
- Map backend validation errors (
field+errorPath) to form field names and frontend-friendly error codes. - Add configurable per-field ICU error-message templates and wire them into form validation rendering.
- Introduce a new
sqm-partner-info-modalcomponent (view + stories) and register it in the stencilbook; bump package version.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/useBankingInfoForm.tsx | Adds API→form field/error-code mapping and includes errorPath in GraphQL validationErrors. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx | Adds per-field error message props + ICU templates and renders richer invalid messages when an API errorCode is present. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form-view.tsx | Extends form error shape to include errorCode and adds text.errorMessages. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx | Passes errorCode + fieldName into validation message builder for each form input. |
| packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/readme.md | Documents the newly added per-field error-message props. |
| packages/mint-components/src/components/sqm-stencilbook/sqm-stencilbook.tsx | Registers Partner Info Modal stories in the stencilbook. |
| packages/mint-components/src/components/sqm-partner-info-modal/sqm-partner-info-modal.tsx | Adds new Partner Info Modal component (currently demo-hook driven). |
| packages/mint-components/src/components/sqm-partner-info-modal/sqm-partner-info-modal-view.tsx | Implements the modal UI (dialog + selects + submit button). |
| packages/mint-components/src/components/sqm-partner-info-modal/PartnerInfoModal.stories.tsx | Adds Storybook stories for the Partner Info Modal. |
| packages/mint-components/src/components.d.ts | Updates generated component typings for new/updated props and new component. |
| packages/mint-components/package.json | Bumps package version to 2.1.8-0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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.
errorCode falls back to error.errorPath, but the UI templates’ other branch is documented to display the raw API message. Falling back to the dot-delimited path will surface non-user-friendly strings like withdrawal.settings.error.routingcode in the UI. Consider using error.message as the fallback display value (or pass both a stable code and a human message separately).
| helpText: getValidationErrorMessage({ | ||
| type: errors?.inputErrors?.beneficiaryTaxPayerId?.type, | ||
| label: props.text.taxPayerIdLabel, | ||
| errorCode: errors?.inputErrors?.taxPayerId?.errorCode, |
There was a problem hiding this comment.
The Tax Payer ID field’s error helper reads errors?.inputErrors?.taxPayerId?.errorCode, but the error being checked is errors?.inputErrors?.beneficiaryTaxPayerId. This prevents the specific API errorCode from being shown for this field. Use the same key (beneficiaryTaxPayerId) when reading errorCode (while still using fieldName: "taxPayerId" if you intend to select the taxPayerId template).
| errorCode: errors?.inputErrors?.taxPayerId?.errorCode, | |
| errorCode: errors?.inputErrors?.beneficiaryTaxPayerId?.errorCode, |
…eUserInfoForm and useTaxAndCash step logic. Handle Impact API sending back DZ and 000000 for phone number data
…h/program-tools into cash-reward-redemption
… partner creation in to widget verificaiton flow. Some view refactors for all 3 components
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 23 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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.
The fallback for unknown API error codes uses error.errorPath as the displayed errorCode. Even after fixing the field name, the error “code/path” strings aren’t user-friendly, while the API already provides a human-readable message. Consider storing both a short mapped errorCode (for ICU select) and a separate errorMessage fallback (or use message as the other branch value) so the UI doesn’t show internal code strings to end users.
| // Early partner creation does not collect these fields | ||
| if ( | ||
| !billingAddress || | ||
| !billingCity || | ||
| !billingPostalCode || | ||
| !billingState || | ||
| !phoneNumber | ||
| ) { | ||
| return "/1"; | ||
| } |
There was a problem hiding this comment.
getCurrentStep treats any non-empty phoneNumber as “present”, but elsewhere in this PR the Impact placeholder value ("0000000") is treated as “missing” and cleared for the form. As written, a placeholder phone number will bypass the “missing fields” redirect and advance the flow incorrectly. Consider treating the placeholder value as missing here as well (and ideally centralize this sentinel handling so it’s consistent across the flow).
| ? null | ||
| : user.impactConnection.publisher.phoneNumberCountryCode, | ||
| phoneNumber: | ||
| user.impactConnection.publisher.phoneNumber === "0000000" | ||
| ? null |
There was a problem hiding this comment.
setUserFormContext sets phoneNumber / phoneNumberCountryCode to null when the placeholder is detected. In this codebase, the form view props for these fields are typed as string | undefined, and web components like sl-input/sl-select typically expect a string value. Use an empty string or undefined for “no value” to avoid passing null into input value props and to keep downstream payload building consistent.
| ? null | |
| : user.impactConnection.publisher.phoneNumberCountryCode, | |
| phoneNumber: | |
| user.impactConnection.publisher.phoneNumber === "0000000" | |
| ? null | |
| ? undefined | |
| : user.impactConnection.publisher.phoneNumberCountryCode, | |
| phoneNumber: | |
| user.impactConnection.publisher.phoneNumber === "0000000" | |
| ? undefined |
| 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.
validationErrors are queried with a code field (see the GraphQL documents above), but the TypeScript result types and mapping logic expect errorPath (error.errorPath). This means errorCode will be undefined at runtime and the per-field ICU error messages won’t resolve correctly. Update the result types and mapping to use the actual response field name (or query the field you intend to use) and ensure the reducer reads that same property consistently.
| errorCode: errors?.inputErrors?.taxPayerId?.errorCode, | ||
| fieldName: "taxPayerId", |
There was a problem hiding this comment.
For the beneficiaryTaxPayerId field, the error type is read from errors.inputErrors.beneficiaryTaxPayerId, but the new errorCode is read from errors.inputErrors.taxPayerId. This mismatch will prevent the correct per-field error message from being selected. Use the same key consistently (likely beneficiaryTaxPayerId) for both type and errorCode.
| errorCode: errors?.inputErrors?.taxPayerId?.errorCode, | |
| fieldName: "taxPayerId", | |
| errorCode: errors?.inputErrors?.beneficiaryTaxPayerId?.errorCode, | |
| fieldName: "beneficiaryTaxPayerId", |
…ctPartner mutation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 24 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| validationErrors: { | ||
| field: string; | ||
| message: string; | ||
| errorPath: string; | ||
| }[]; |
There was a problem hiding this comment.
validationErrors is typed with an errorPath field, but the GraphQL mutations in this file request validationErrors { code }. This mismatch will cause the new error-code mapping to see undefined at runtime. Update the TypeScript result types to match the actual response shape (or request the correct field).
| const vars = { | ||
| user: { | ||
| id: user.id, | ||
| accountId: user.accountId, | ||
| }, | ||
| firstName: user.firstName, |
There was a problem hiding this comment.
onSubmit constructs vars using user.id/accountId/firstName/lastName without verifying user is defined. Since showModal only checks userLoading (not user), the modal can be interactive even if user is missing, leading to a crash. Add a guard (and surface networkErrorText if user data is unavailable).
| validationErrors { | ||
| field | ||
| message | ||
| code | ||
| } |
There was a problem hiding this comment.
The mutation requests validationErrors { code }, but downstream logic reads errorPath. Either request errorPath from the API (if supported) or switch the mapping logic/types to use code consistently.
| console.log(states, "partner info modal states"); // TEMP | ||
|
|
There was a problem hiding this comment.
Remove the temporary console.log debug statement before merging. Leaving this in a production component will spam logs and may expose internal state.
| console.log(states, "partner info modal states"); // TEMP |
| const formField = | ||
| API_FIELD_TO_FORM_FIELD[error.field] || error.field; | ||
| const errorCode = | ||
| API_ERROR_PATH_TO_FRONTEND[error.errorPath] || error.errorPath; |
There was a problem hiding this comment.
The error mapping uses error.errorPath, but the mutations request validationErrors { code }. As a result errorCode will be undefined and the per-field ICU error messages won’t be used. Use the actual field returned by the API (e.g., error.code) when computing errorCode.
| API_ERROR_PATH_TO_FRONTEND[error.errorPath] || error.errorPath; | |
| API_ERROR_PATH_TO_FRONTEND[error.code] || error.code; |
| // when creating an impact connection without sending phoneNumber data, the impactAPI defaults the value to "000000" and the phoneNumberCountryCode to "DZ" | ||
| phoneNumberCountryCode: | ||
| user.impactConnection.publisher.phoneNumberCountryCode, | ||
| phoneNumber: user.impactConnection.publisher.phoneNumber, | ||
| user.impactConnection.publisher.phoneNumber === "0000000" | ||
| ? null | ||
| : user.impactConnection.publisher.phoneNumberCountryCode, | ||
| phoneNumber: | ||
| user.impactConnection.publisher.phoneNumber === "0000000" | ||
| ? null | ||
| : user.impactConnection.publisher.phoneNumber, |
There was a problem hiding this comment.
The comment says Impact defaults missing phone numbers to "000000", but the code checks for "0000000". This inconsistency likely prevents the placeholder value from being normalized to null as intended. Use the correct placeholder value (and consider centralizing it as a constant so it’s consistent across the codebase).
| ); | ||
| }, [financeNetworkData, currenciesData, countryCode]); | ||
|
|
||
| console.log(countryCode, currency, "initial country and currency state"); // TEMP |
There was a problem hiding this comment.
Remove the temporary console.log debugging statements before merging (e.g., logging initial country/currency and other internal state). These create noisy production logs and can leak user-related information.
| console.log(countryCode, currency, "initial country and currency state"); // TEMP |
…o handle country that has states.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 25 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const errorCode = | ||
| API_ERROR_PATH_TO_FRONTEND[error.errorPath] || error.errorPath; |
There was a problem hiding this comment.
validationErrors from the withdrawal settings mutations are queried with a code field, but the local TS types and mapping logic expect errorPath (and read error.errorPath). As written, errorCode will always be undefined, so the new per-field ICU error messaging won’t work.
Update the result types to use code (or whatever the API actually returns) and map errorCode from that field (with a sensible fallback like error.message for unknown codes).
| const errorCode = | |
| API_ERROR_PATH_TO_FRONTEND[error.errorPath] || error.errorPath; | |
| const apiErrorCode = error.code; | |
| const errorCode = | |
| API_ERROR_PATH_TO_FRONTEND[apiErrorCode] || | |
| apiErrorCode || | |
| error.message; |
| console.log( | ||
| user.impactConnection.publisher, | ||
| "publisher data in useTaxAndCash to determine step", | ||
| ); |
There was a problem hiding this comment.
Leftover debug logging (console.log) will leak publisher data to the browser console and add noise in production. Please remove this log (or gate it behind a debug flag).
| console.log( | |
| user.impactConnection.publisher, | |
| "publisher data in useTaxAndCash to determine step", | |
| ); |
| // Early partner creation does not collect these fields | ||
| const stateRequired = ["ES", "AU", "US", "CA"].includes(countryCode); | ||
| if ( | ||
| !billingAddress || | ||
| !billingCity || | ||
| !billingPostalCode || | ||
| (stateRequired && !billingState) || | ||
| !phoneNumber |
There was a problem hiding this comment.
getCurrentStep treats phoneNumber as present if it’s a non-empty string, but elsewhere in this PR the Impact API’s “missing phone” sentinel is handled explicitly (e.g. setting phone fields to null when the API default is returned). If the API default is something like "000000"/"0000000", this step check will incorrectly skip step 1 even though the participant still needs to enter a real phone number.
Consider treating the sentinel value as “missing” here as well (using the same shared constant).
| typeof props["sqm-partner-info-modal_stateController"] === "string" | ||
| ? parseStates(props["sqm-partner-info-modal_stateController"]) | ||
| : props["sqm-partner-info-modal_stateController"]; | ||
| if (props.showPartnerModal && partnerState?.states?.open === false) { |
There was a problem hiding this comment.
partnerState?.states?.open is read here, but props["sqm-partner-info-modal_stateController"] is a stateController object/string, not the hook result from usePartnerInfoModal (which is what would contain states.open). This check will always be undefined and won’t correctly hide/show the partner step in editor previews.
Either parse the partner modal’s stateController into the expected shape (e.g., look for partnerState?.["sqm-partner-info-modal"]?.states?.open) or remove this conditional.
| if (props.showPartnerModal && partnerState?.states?.open === false) { | |
| const partnerModalOpen = | |
| partnerState?.states?.open ?? | |
| partnerState?.["sqm-partner-info-modal"]?.states?.open; | |
| if (props.showPartnerModal && partnerModalOpen === false) { |
| const renderStepContent = () => { | ||
| if (props.showPartnerModal) { | ||
| return ( | ||
| <sqm-partner-info-modal | ||
| inModal | ||
| {...partnerText} | ||
| stateController={ | ||
| props["sqm-partner-info-modal_stateController"] || "{}" | ||
| } | ||
| ></sqm-partner-info-modal> | ||
| ); |
There was a problem hiding this comment.
When showPartnerModal is true, the widget always renders <sqm-partner-info-modal inModal ...>, but there’s no way for the partner modal to signal completion back to useWidgetVerification (which exposes onPartnerModalComplete) and no code here calls it. This risks trapping the user in the partner-creation step even after the connection is created.
Wire onPartnerModalComplete into the partner modal step (e.g., via an event/callback prop) and/or hide the partner content when the partner modal reports states.open === false/success === true.
| @@ -542,11 +549,11 @@ export const UserInfoFormView = (props: UserInfoFormViewProps) => { | |||
| {text.supportLink} | |||
| </a> | |||
| ), | |||
| } | |||
| }, | |||
| )} | |||
| </p> | |||
| </sqm-form-message> | |||
| )} | |||
| )} */} | |||
There was a problem hiding this comment.
This commented-out JSX block leaves a large chunk of dead UI in the render tree, which makes future maintenance harder (and can easily get out of sync with real behavior).
If this alert is no longer needed, delete the block entirely; if it may come back, consider a feature flag or a prop-driven conditional instead of keeping commented code.
| export function PartnerInfoModalContentView(props: PartnerInfoModalViewProps) { | ||
| const { states, callbacks, text } = props; | ||
| const sheet = createStyleSheet(style); | ||
| const styleString = sheet.toString(); | ||
|
|
||
| const description = states.isExistingPartner ? ( | ||
| <span class={sheet.classes.DescriptionContainer}> | ||
| <p>{text.descriptionExistingPartner}</p> | ||
| <p>{text.supportDescriptionExistingPartner}</p> | ||
| </span> | ||
| ) : ( | ||
| <p class={sheet.classes.DescriptionContainer}> | ||
| {text.descriptionNewPartner} | ||
| </p> | ||
| ); | ||
|
|
||
| const buttonLabel = states.isExistingPartner | ||
| ? text.confirmButtonLabel | ||
| : text.submitButtonLabel; | ||
|
|
||
| return ( | ||
| <div> | ||
| <style type="text/css"> {styleString}</style> | ||
| <div class={sheet.classes.FormFields}> |
There was a problem hiding this comment.
PartnerInfoModalContentView doesn’t respect states.open at all. When this component is embedded via inModal (as it is in sqm-widget-verification), the content will still render even after usePartnerInfoModal sets open to false on success.
Add an early return when !states.open (or when states.success is true), or provide a callback/event so the parent can switch steps once partner creation completes.
| console.log(countryCode, currency, "initial country and currency state"); // TEMP | ||
| const [countrySearch, setCountrySearch] = useState(""); | ||
| const [currencySearch, setCurrencySearch] = useState(""); | ||
| const [filteredCountries, setFilteredCountries] = useState( | ||
| countriesData?.impactPayoutCountries?.data || [], | ||
| ); | ||
| const [filteredCurrencies, setFilteredCurrencies] = useState( | ||
| currencies || [], | ||
| ); | ||
|
|
||
| console.log(userData, "userData partner info modal"); | ||
|
|
There was a problem hiding this comment.
Multiple console.log statements in this hook (marked // TEMP) will leak user/account data to the browser console and add noise in production. Please remove these logs before merge.
| const [success, setSuccess] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| if (userData && user.impactConnection?.publisher) { |
There was a problem hiding this comment.
This effect condition uses user.impactConnection?.publisher, but user can be undefined while data is still loading or if the viewer isn’t a User. Accessing user.impactConnection will throw at runtime.
Change the guard to if (userData && user?.impactConnection?.publisher) { ... } (or just if (user?.impactConnection?.publisher) ...) and consider disabling submit until user is available, since onSubmit also assumes user.id/accountId exist.
| if (userData && user.impactConnection?.publisher) { | |
| if (userData && user?.impactConnection?.publisher) { |
| header-new-partner="Welcome to {brandName} Program!" | ||
| description-new-partner="We just need a bit more information about you before you start earning cash!" |
There was a problem hiding this comment.
<sqm-partner-info-modal> props in this story use attribute names that don’t exist on the component (e.g. header-new-partner). The actual prop/attribute is modalHeader / modal-header.
Update the story to use the real attribute names so the example renders correctly.
| header-new-partner="Welcome to {brandName} Program!" | |
| description-new-partner="We just need a bit more information about you before you start earning cash!" | |
| modal-header="Welcome to {brandName} Program!" | |
| modal-description="We just need a bit more information about you before you start earning cash!" |
Description of the change
Type of change
Links
Checklists
Development
Paperwork
Code review