Fix attendees selection to allow names instead of email or phone only#77995
Fix attendees selection to allow names instead of email or phone only#77995
Conversation
|
@eVoloshchak 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] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@eVoloshchak Let us know if you can review it 🙌 |
|
Can we add a test or two for |
@pecanoro done |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Makes sense 👍
mountiny
left a comment
There was a problem hiding this comment.
LGTM, can you please make sure this works offline and persists after sign out and sign in
Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>
c5d8ead
Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>
|
@mountiny it seems to work offline and sign in/sign out Screen.Recording.2025-12-18.at.18.49.46.mov |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-12-18.at.23.10.16.movAndroid: mWeb ChromeScreen.Recording.2025-12-18.at.23.11.44.moviOS: HybridAppScreen.Recording.2025-12-18.at.23.03.56.moviOS: mWeb SafariScreen.Recording.2025-12-18.at.23.05.03.movMacOS: Chrome / SafariScreen.Recording.2025-12-18.at.22.57.55.mov |
|
Thanks |
There was a problem hiding this comment.
Pull request overview
This PR fixes the attendees selection feature to allow adding attendees by plain name (e.g., "Jeff Amazon") instead of requiring an email or phone number. The fix enables the shouldAcceptName configuration option for the attendees search context.
Key changes:
- Added
shouldAcceptNameparameter to the options filtering and user invitation logic - Enabled
shouldAcceptName: truefor the ATTENDEES search context - Added comprehensive unit tests for the new behavior
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/libs/OptionsListUtils/types.ts |
Added shouldAcceptName boolean property to GetOptionsConfig type definition |
src/libs/OptionsListUtils/index.ts |
Added shouldAcceptName parameter to getValidOptions function with default value of false, and passed it through to filterUserToInvite |
src/hooks/useSearchSelector.base.ts |
Enabled shouldAcceptName: true for the SEARCH_CONTEXT_ATTENDEES case, allowing plain text names to generate user invite options |
tests/unit/OptionsListUtilsTest.tsx |
Added two test cases verifying that userToInvite is correctly created when shouldAcceptName is true and not created when false for plain text names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.2.85-0 🚀
|
|
It creates a bunch of deploy blockers. The PR itself is not broken, it does allow adding names as attendees. However, it wasn't a complete fix since it uncovered a lot of things that are not working properly after adding an attendee who isn't an email or phone. |
|
I am going to revert since everyone involved is on vacation and we can try again next year 😄 |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.2.85-7 🚀
|
Explanation of Change
Problem: attendees could only be added by email or phone number, not plain names like "Jeff Amazon", despite the design doc and backend supporting it.
Fix: enable
shouldAcceptName: truefor the attendees search context, allowing the invite option to appear for plain text entries.Fixed Issues
$ #77993
PROPOSAL:
Tests
Screen.Recording.2025-12-18.at.16.06.51.mov
Also make sure this works offline and persists after sign out and sign in:
Screen.Recording.2025-12-18.at.18.49.46.mov
Offline tests
See video above
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as in tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari