Fix crash when attendee email is undefined in getPersonalDetailByEmail#87955
Conversation
|
@Krishna2323 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28dd257f5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@Krishna2323 I have addressed all the feedbacks and lint errors, could you please take another look. thanks |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| return getSplitAuthor(tr, splits); | ||
| } | ||
|
|
||
| return att.email ? getPersonalDetailByEmail(att.email)?.accountID : undefined; |
There was a problem hiding this comment.
getPersonalDetailByEmail already handles undefined email now. The att.email ? check is redundant.
| // If the transaction is a split, then attendees are not present as a property so we need to use a helper function. | ||
| ?.flatMap<number | undefined>((tr) => | ||
| tr.comment?.attendees?.map?.((att) => (tr.comment?.source === CONST.IOU.TYPE.SPLIT ? getSplitAuthor(tr, splits) : getPersonalDetailByEmail(att.email)?.accountID)), | ||
| normalizeAttendees(tr.comment?.attendees).map((att) => { |
There was a problem hiding this comment.
The normalizeAttendees call here is redundant since getPersonalDetailByEmail already handles undefined email. We can simplify this back to the original one-liner and remove the normalizeAttendees import from this file.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariMonosnap.screencast.2026-04-19.23-54-07.mp4 |
|
@marufsharifi left a few minor comments—otherwise LGTM. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
🚧 @chuckdries has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
Explanation of Change
This change makes attendee data safer and more consistent when older expenses contain incomplete attendee info.
Before, some historical attendees could exist without an email, but parts of the app still assumed
emailwas always present. That could crash report previews, break attendee selection, and create inconsistent recent-attendee data. Now we normalize attendee data into one canonical shape as soon as it enters app logic, preserve name-only attendees safely, and keep email lookups guarded as a fallback.Fixed Issues
$ #86781
PROPOSAL: #86781 (comment)
Tests
emailfield.Offline tests
Same as Tests.
QA Steps
Same as Tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Not applicable.
Android: mWeb Chrome
Not applicable.
iOS: Native
Not applicable.
iOS: mWeb Safari
Not applicable.
MacOS: Chrome / Safari
Screen.Recording.2026-04-15.at.1.02.23.PM.mp4