Cash reward redemption#490
Open
Sam Beveridge (00salmon) wants to merge 103 commits into
Open
Conversation
…o cash-reward-redemption
Contributor
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.
| 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).
Suggested change
| 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
…Errors + help text not showing on first submission.
This commit was generated by GitHub Actions CI
This commit was generated by GitHub Actions CI
This commit was generated by GitHub Actions CI
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 66 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- packages/mint-components/package-lock.json: Generated file
Comments suppressed due to low confidence (6)
packages/mint-components/src/components/tax-and-cash/sqm-tax-and-cash/useTaxAndCash.tsx:1
routeAfterMaybeCompleting()refetches, but then computescurrentStepusing the staleuserobject captured beforerefetch(), so routing can be wrong (e.g., still routes as STARTED/invalid even after completing). Use the refetch result (or recompute fromdataafter refetch) when callinggetCurrentStep, and consider handling/void-ing the async call to avoid unhandled promise rejections.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/useBankingInfoForm.tsx:1- The fallback
errorCodeuses the raw APIerror.codewhen there’s no mapping. Downstream ICU templates treatotheras a human-readable fallback, buterror.codeis typically not user-facing. Consider storing both a short mapped code and the originalerror.message(or usingerror.messageas the fallback when no mapping exists) so the “other” branch can display a readable message.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx:1 classificationCodeErroris keyed asbeneficiaryClassification, but the new field introduced informDefinitions.tsxuseserrors.inputErrors.classificationCode/name="/classificationCode". This mismatch prevents the per-field ICU template from being found. Rename the key toclassificationCode(or align the input error key tobeneficiaryClassification) sogetValidationErrorMessagecan pick up the right template.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1- The
errorCodelookup useserrors?.inputErrors?.taxPayerId?.errorCode, but the field key for this input isbeneficiaryTaxPayerId(as used fortypeon line 242). This makeserrorCodeundefined even when the API returned one. Useerrors?.inputErrors?.beneficiaryTaxPayerId?.errorCodeinstead.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1 - These new fields don’t pass
errorCodeandfieldNameintogetValidationErrorMessage, so the new per-field ICU error templates (and mapped API error codes) won’t be used forbranchName/classificationCode. PasserrorCode: errors?.inputErrors?.branchName?.errorCodewithfieldName: "branchName", and similarly forclassificationCode.
packages/mint-components/src/components/sqm-referral-table/cells/sqm-referral-table-rewards-cell.tsx:379 - This appears intended to render the “Not available”/fallback status when there is no matching state, but it only triggers for an empty-string state. If
stateisundefined/null(a more common “no state” sentinel), this block won’t render—so users may see no status text at all. Prefer a falsy check (e.g.,!state) or ensurestateis normalized to""in one place.
{state === "" && (
<div>
<TextSpanView type="p">{statusText}</TextSpanView>
</div>
)}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 66 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- packages/mint-components/package-lock.json: Generated file
Comments suppressed due to low confidence (8)
packages/mint-components/src/components/tax-and-cash/sqm-tax-and-cash/useTaxAndCash.tsx:1
currentStepis computed using the pre-refetchuserobject, so routing can ignore the newly completed connection status / updated publisher fields. ComputecurrentStepfrom the refetched data (e.g., use the returned refetch result or derive from the latest query state) before callingsetStep.
packages/mint-components/src/components/tax-and-cash/data.ts:1connectionStatusis typed as always-present, but the updated UI logic treats it as possibly missing for legacy connections (andsaasquatch.d.tsmakes it optional). If the API can return null/undefined for legacy users, this type should allow that (e.g.,connectionStatus?: ...orconnectionStatus: ... | null) to avoid incorrect assumptions and potential runtime/type mismatches.
packages/mint-components/src/components/tax-and-cash/sqm-user-info-form/sqm-user-info-form-view.tsx:1- Two concrete runtime hazards here: (1)
valueonly coercesnullto""but notundefined, sovalue={undefined}can leak to the input; consider using a nullish coalesce (formState.phoneNumber ?? "") to normalize both. (2)formDatais typed optional (formData?: FormState) but is dereferenced unguarded (formData.phoneNumberCountryCode); useformData?.phoneNumberCountryCode(and handle the undefined case) to prevent crashes if the validator is invoked withoutformData.
packages/mint-components/src/components/tax-and-cash/sqm-user-info-form/sqm-user-info-form-view.tsx:1 - Two concrete runtime hazards here: (1)
valueonly coercesnullto""but notundefined, sovalue={undefined}can leak to the input; consider using a nullish coalesce (formState.phoneNumber ?? "") to normalize both. (2)formDatais typed optional (formData?: FormState) but is dereferenced unguarded (formData.phoneNumberCountryCode); useformData?.phoneNumberCountryCode(and handle the undefined case) to prevent crashes if the validator is invoked withoutformData.
packages/mint-components/src/components/tax-and-cash/sqm-user-info-form/sqm-user-info-form-view.tsx:1 onSl-inputfrom Shoelace is dispatched by the<sl-input>custom element, soe.targetis not anHTMLInputElement. Casting toHTMLInputElementis misleading and can break if consumers expect DOM input semantics; use the Shoelace element type (or a safeany/property access) and update its.valueproperty directly.
packages/mint-components/src/components/tax-and-cash/utils.ts:1- Returning
""for missing input can unintentionally overwrite a previously stored phone number (or trigger backend validation) when the caller always includesphoneNumberin the mutation payload. Consider returningundefined(and omitting the field) or otherwise letting callers decide whether to send the field wheninputis empty.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1 - These new inputs don’t pass
errorCodeandfieldNameintogetValidationErrorMessage, so the richer ICU templates added insqm-banking-info-form.tsxwon’t be used for API-derived validation errors onbranchName/classificationCode. For consistency with the other fields, passerrorCode: errors?.inputErrors?.<field>?.errorCodeandfieldName: "<field>"so per-field messaging works.
packages/mint-components/src/components/sqm-widget-verification/sqm-code-verification/sqm-code-verification.feature:1 - Corrected spelling of 'recieved' to 'received'.
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 66 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- packages/mint-components/package-lock.json: Generated file
Comments suppressed due to low confidence (6)
packages/mint-components/src/components/tax-and-cash/sqm-tax-and-cash/useTaxAndCash.tsx:1
getCurrentStep(user)is computed using the staleuservalue from beforerefetch(), so routing can be wrong (e.g., still seeing STARTED / missing fields). Capture the result ofrefetch()(or callgetCurrentStepwith the refreshed user object) and handle errors from the async call (e.g.,void routeAfterMaybeCompleting().catch(...)) to avoid unhandled promise rejections in the effect.
packages/mint-components/src/components/tax-and-cash/utils.ts:1parsePhoneNumberFromStringcan throw if the provided country code is not recognized at runtime (you’re casting an arbitrarystringtoCountryCode). Wrap parsing in a try/catch and fall back to the digits-only sanitizer (or returnfalsein validators) to prevent the form from crashing on unexpected backend values.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx:1classificationCodeErroris being stored underbeneficiaryClassification, but the new input is namedclassificationCode(and backend mapping also appears to useclassificationCode). This mismatch will prevent the per-field ICU template lookup from working for classification-code errors; align the key (classificationCode) with the form field/errors shape.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1- These new fields don’t pass
errorCodeandfieldNameintogetValidationErrorMessage, so API-derived error-code mapping won’t surface the intended rich ICU messages (it will fall back to the generic invalid text). PasserrorCode: errors?.inputErrors?.branchName?.errorCode+fieldName: "branchName"and similarly forclassificationCode.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1 - These new fields don’t pass
errorCodeandfieldNameintogetValidationErrorMessage, so API-derived error-code mapping won’t surface the intended rich ICU messages (it will fall back to the generic invalid text). PasserrorCode: errors?.inputErrors?.branchName?.errorCode+fieldName: "branchName"and similarly forclassificationCode.
packages/mint-components/src/components/sqm-referral-table/cells/sqm-referral-table-rewards-cell.tsx:379 - Rendering a dedicated branch for
state === ""is fragile and likely unintended (most “no match” cases areundefined/null, and an empty string will also force the ICUselectinto theotherbranch). Prefer returning an explicit, meaningful status from the resolver (or render based on!state) to avoid inconsistent display between “no rule” vs “unknown rule”.
{state === "" && (
<div>
<TextSpanView type="p">{statusText}</TextSpanView>
</div>
)}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 66 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- packages/mint-components/package-lock.json: Generated file
Comments suppressed due to low confidence (6)
packages/mint-components/src/components/tax-and-cash/sqm-tax-and-cash/useTaxAndCash.tsx:1
routeAfterMaybeCompleting()computescurrentStepusing the pre-refetchuserobject even after callingrefetch(), so routing can be wrong (e.g., still seeingSTARTED/ incomplete status). Also,completeConnection()buildsvarsusing outerdata?.userrather than theuserargument, which can cause inconsistent IDs/names if the closure is stale. Recommended fix: (1) makerouteAfterMaybeCompleting()use the refetch result (e.g.,const refreshed = await refetch(); const refreshedUser = refreshed?.data?.user ?? user;then callgetCurrentStep(refreshedUser)), (2) buildvarsconsistently from theuserparameter and itspublisher, and (3) explicitly handle the async call withvoid routeAfterMaybeCompleting()and internal try/catch to avoid unhandled promise rejections insideuseEffect.
packages/mint-components/src/components/tax-and-cash/data.ts:1connectionStatusis typed as always present and non-null, but multiple callsites and specs in this PR treat legacy users as havingconnectionStatusnull/undefined. To align the type with expected backend behavior (and avoid runtime/type drift), makeconnectionStatusoptional/nullable (e.g.,connectionStatus?: "NOT_STARTED" | "STARTED" | "COMPLETED" | null;) and update any dependent logic that assumes it’s always set.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1- The new
branchNameandclassificationCodeinputs don’t passerrorCodeandfieldNameintogetValidationErrorMessage, so they can’t use the new per-field ICU error templates even though the rest of the form is wired for that. Suggested fix: passerrorCode: errors?.inputErrors?.branchName?.errorCode+fieldName: "branchName"(and similarly forclassificationCode) to keep behavior consistent and enable the richer messages.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx:1 - The per-field error template map keys should match the
fieldNamepassed togetValidationErrorMessage()and the keys used inerrors.inputErrors. For the newly addedclassificationCodefield, the map currently usesbeneficiaryClassification, which meansclassificationCodewill fall back to the generic invalid message even when an APIerrorCodeis present. Suggested fix: add aclassificationCode: props.classificationCodeErrorentry (and keepbeneficiaryClassificationonly if it’s still a real input key elsewhere), then ensure the classification-code input passesfieldName: "classificationCode".
packages/mint-components/src/components/tax-and-cash/utils.ts:1 parsePhoneNumberFromString()can throw for invalid/unsupportedCountryCodevalues (e.g., unexpected strings coming from data or the select). Since this helper is used in submit paths, an exception here would break saving rather than cleanly falling back to digits-only. Suggested fix: wrap the parse call in a try/catch and return the digits-only fallback (or empty string) when parsing fails; apply the same defensive pattern inisValidI18nPhoneNumber()so it returnsfalseinstead of throwing.
packages/mint-components/src/components/sqm-widget-verification/sqm-widget-verification.tsx:1- Correct grammar in this comment to make the intent clear (e.g., “Any updates must be reflected in …”).
Coleton (Locrian24)
left a comment
Coleton (Locrian24)
left a comment
Contributor
There was a problem hiding this comment.
Will approve once hygiene and stencil-docs-target is bumped
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 67 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- packages/mint-components/package-lock.json: Generated file
Comments suppressed due to low confidence (7)
packages/mint-components/src/components/tax-and-cash/data.ts:1
connectionStatusis modeled here as always present whenimpactConnectionis non-null, but other changes in this PR (andsaasquatch.d.ts) treat it as optional and sometimes null/undefined (legacy). This mismatch can lead to unsafe assumptions and incorrect narrowing in the UI logic. Update theUserQuerytype to reflect observed API behavior (e.g.,connectionStatus?: ... | null).
packages/mint-components/src/components/tax-and-cash/sqm-tax-and-cash/useTaxAndCash.tsx:1- After calling
refetch(), routing still uses the pre-refetchuserobject, sogetCurrentStep(user)can compute the wrong step (especially ifconnectionStatuschanges to COMPLETED). Also, the async call isn't awaited/handled, so errors become unhandled promise rejections. Use the refetch result (or updateddata.user) to compute the step, and invoke the async function with explicit error handling (e.g.,void routeAfterMaybeCompleting().catch(...)).
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx:1 classificationCodeErroris wired underbeneficiaryClassification, which appears to be a different field (and will overwrite the classification template). As a result, classification code (KBE) errors likely won't map to the correct per-field ICU template. Rename/add the correct key for the field that receivesclassificationCodevalidation errors (e.g.,classificationCode) and keepbeneficiaryClassificationmapped to the classification select error template.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1- These new inputs don't pass
errorCodeandfieldNameintogetValidationErrorMessage, so they can't take advantage of the new API error-code mapping + per-field ICU templates introduced in this PR. Pass througherrors?.inputErrors?.<field>?.errorCodeand the corresponding fieldName keys so branch name / classification code get the richer messages.
packages/mint-components/src/components/tax-and-cash/sqm-user-info-form/sqm-user-info-form-view.tsx:1 - The comment indicates the placeholder is the pair
phoneNumber === \"0000000\"andphoneNumberCountryCode === \"DZ\", but the unlock logic checks onlyphoneNumber. This can incorrectly unlock fields if a real value ever matches the placeholder number (or if the backend changes one side only). Match on both fields (phone number + country code) to align with the documented invariant.
packages/mint-components/src/components/sqm-referral-table/cells/sqm-referral-table-rewards-cell.tsx:379 - This adds a new render branch for
state === \"\"that duplicates thePAYOUT_APPROVEDblock and changes the previous behavior. If\"\"represents an unknown/unmatched state, it should likely be handled explicitly (e.g., a dedicated fallback UI or no status section), not by reusing the payout-approved layout. Consider reverting the originalPAYOUT_APPROVEDcondition and adding a deliberate, separate fallback for empty/unknown state.
{state === "" && (
<div>
<TextSpanView type="p">{statusText}</TextSpanView>
</div>
)}
packages/mint-components/src/components/sqm-widget-verification/sqm-code-verification/sqm-code-verification.feature:1
- Correct spelling of 'recieved' to 'received'.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 67 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- packages/mint-components/package-lock.json: Generated file
Comments suppressed due to low confidence (4)
packages/mint-components/src/components/tax-and-cash/sqm-tax-and-cash/useTaxAndCash.tsx:1
getCurrentStep(user)uses the pre-refetchuserobject even afterawait refetch(). IfcompleteImpactConnectionupdatesconnectionStatusand/or publisher billing fields, routing can still compute the wrong step (e.g., staying on "/1" because it’s evaluating stale data). Use the refetch result (or re-readdata.userafter refetch) to computecurrentStepfrom the refreshed user instead of the stale closure value.
packages/mint-components/src/components/tax-and-cash/sqm-user-info-form/sqm-user-info-form-view.tsx:1formDatais typed as optional inValidationErrorFunction(formData?: FormState), but this handler dereferencesformData.phoneNumberCountryCodewithout a guard. This can fail TypeScript compilation (or throw at runtime ifformDatais ever omitted). Use optional chaining or a default (e.g.,formData?.phoneNumberCountryCode) and decide how validation should behave when it’s missing.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/formDefinitions.tsx:1- The new fields
branchName(and similarlyclassificationCodebelow) don’t passerrorCodeandfieldNameintogetValidationErrorMessage, while other inputs do. This prevents the new API error-code mapping from producing the richer per-field ICU messages you introduced. Update these new inputs to passerrorCode: errors?.inputErrors?.branchName?.errorCodeandfieldName: "branchName"(and the equivalent forclassificationCode) to keep behavior consistent.
packages/mint-components/src/components/tax-and-cash/sqm-banking-info-form/sqm-banking-info-form.tsx:1 classificationCodeErroris wired under thebeneficiaryClassificationkey, but the new KBE field introduced inuseBankingInfoForm/formDefinitions.tsxis namedclassificationCode. As written, API validation errors forclassificationCodewon’t be able to find the per-field template, and may fall back to generic messaging. Wireprops.classificationCodeErrorunder the matchingclassificationCodekey (and keepbeneficiaryClassificationseparate if it represents the “Classification” select field).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the change
Type of change
Links
Checklists
Development
Paperwork
Code review