Fix waysToContact not displaying when API returns a string value#412
Conversation
nadavosa
left a comment
There was a problem hiding this comment.
PR #412 Review: Fix waysToContact not displaying when API returns a string
The core fix is correct — wrapping a bare string in an array so it renders. A few issues worth addressing:
1. No enum validation before the cast (correctness)
The typeof raw === "string" && raw guard only checks non-empty, not that the value is a valid PreferredCommunicationType. An unexpected value (wrong casing, deprecated key) gets cast and silently reaches the form or API. Consider:
const isValid = (v: string): v is PreferredCommunicationType =>
Object.values(PreferredCommunicationType).includes(v as PreferredCommunicationType);
const waysToContact = Array.isArray(raw)
? raw.filter(isValid)
: isValid(raw) ? [raw] : [];This would protect the array branch too.
2. Explicit field listing may drift from schema
The old spread (...opportunity.contact) included all contact fields automatically. The new explicit list (name, phone, email, waysToContact) will silently miss any new fields added to OpportunityContactDetailsFormData. A comment noting the list must stay in sync with the schema would help.
3. createOpportunityContactDetailsSchema(t) called outside useMemo (pre-existing)
This creates a new schema object on every render, causing the useForm resolver to change each render. Not introduced by this PR but it's in the touched area — worth fixing in a follow-up.
nadavosa
left a comment
There was a problem hiding this comment.
Both issues addressed correctly:
- Enum validation now uses
new Set<string>(COMMUNICATION_TYPES)— covers both the string and array branches cleanly - Sync comment added to the explicit field list
Approving.
Closes #399
The API returns
waysToContactas a plain string (e.g."mobilePhone") instead of an array. The existing guard only checkedArray.isArray, so a string value was silently dropped and the field always displayed "–" even when data existed.Changes:
OpportunityContactDetails.tsx: handlewaysToContactas either aPreferredCommunicationType[]or a single string, wrapping the string case in a one-element arrayname,phone,email) explicitly default to""instead of relying on the spread to avoid undefined values slipping through