Add required SignerFields #81602
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,10 @@ const signerDetailsFields = [FULL_NAME, EMAIL, JOB_TITLE, DATE_OF_BIRTH, STREET, | |
| const signerFilesFields = [PROOF_OF_DIRECTORS, ADDRESS_PROOF, COPY_OF_ID, CODICE_FISCALE]; | ||
| const beneficialOwnerFields = [FIRST_NAME, LAST_NAME, DOB, BENEFICIAL_STREET, BENEFICIAL_CITY, BENEFICIAL_STATE, BENEFICIAL_ZIP_CODE]; | ||
|
|
||
| // These fields are required by the backend and must always be included in the request | ||
| // so that proper validation errors can be returned if they're missing | ||
| const requiredSignerFields = new Set<string>([FULL_NAME, DATE_OF_BIRTH, JOB_TITLE]); | ||
|
|
||
| function getSignerDetailsAndSignerFilesForSignerInfo(reimbursementAccountDraft: OnyxEntry<ReimbursementAccountForm>, signerEmail: string, isUserBeneficialOwner: boolean) { | ||
| const signerDetails: Record<string, string | boolean | FileObject[]> = {}; | ||
| const signerFiles: Record<string, string | FileObject | boolean> = {}; | ||
|
|
@@ -32,18 +36,28 @@ function getSignerDetailsAndSignerFilesForSignerInfo(reimbursementAccountDraft: | |
| continue; | ||
| } | ||
|
|
||
| if (fieldName === STREET || fieldName === CITY || fieldName === STATE || fieldName === ZIP_CODE) { | ||
| if (reimbursementAccountDraft?.[fieldName]) { | ||
| signerDetails[ADDRESS] = signerDetails[ADDRESS] | ||
| ? `${SafeString(signerDetails[ADDRESS])}, ${SafeString(reimbursementAccountDraft?.[fieldName])}` | ||
| : SafeString(reimbursementAccountDraft?.[fieldName]); | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| // Preserve type for DOWNLOADED_PDS_AND_FSG since they are boolean values | ||
| if (fieldName === DOWNLOADED_PDS_AND_FSG) { | ||
| if (reimbursementAccountDraft?.[fieldName]) { | ||
| signerDetails[fieldName] = reimbursementAccountDraft[fieldName]; | ||
| } | ||
| continue; | ||
| } | ||
|
|
||
| // Always include required fields (with empty string if not present) so Auth validation can catch them | ||
| // For non-required fields, only include if they have a value | ||
| if (requiredSignerFields.has(fieldName) || reimbursementAccountDraft?.[fieldName]) { | ||
| signerDetails[fieldName] = SafeString(reimbursementAccountDraft?.[fieldName]); | ||
|
Contributor
There was a problem hiding this comment. ❌ CONSISTENCY-2 (docs)The SafeString() function is applied to required fields even when they might be undefined, but there is no documentation explaining what SafeString(undefined) returns. This could lead to unexpected behavior if SafeString(undefined) does not return an empty string as intended. Suggested fix: Add a comment explaining the behavior, or make the intent explicit by using a ternary operator to return an empty string when the field is not present. Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Comment on lines
+56
to
+59
There was a problem hiding this comment.
When Useful? React with 👍 / 👎. |
||
| } | ||
| } | ||
|
|
||
| if (isUserBeneficialOwner) { | ||
|
|
||
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.
❌ CONSISTENCY-6 (docs)
When isUserBeneficialOwner is true, the code explicitly sets FULL_NAME, DATE_OF_BIRTH, and ADDRESS to empty strings (line 50-52 in the original file), but JOB_TITLE is now always required via requiredSignerFields. This may be inconsistent with the intended behavior.
Consider whether JOB_TITLE should be conditionally required based on isUserBeneficialOwner, or if the current behavior is correct. If JOB_TITLE should always be required regardless of beneficial owner status, consider adding it to the explicit initialization block for consistency.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.