[lib] Include searchUsers results in getPotentialMemberItems
ClosedPublic
Actions

Authored by michal on May 29 2023, 6:04 AM.

Details

Reviewers

kamil
inka

Commits

rCOMM623f06c7a828: [lib] Include searchUsers results in getPotentialMemberItems

Summary

We will want to include results from user search endpoint in web and native chat composers. This diff adds an option to getPotentialMemberItems to include additional users from the endpoint.

Test Plan

Tested if the user are returned correctly (that are not in user store) are returned correctly

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

michal created this revision.May 29 2023, 6:04 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptMay 29 2023, 6:04 AM

Harbormaster completed remote builds in B19823: Diff 27154.May 29 2023, 6:23 AM

michal requested review of this revision.May 29 2023, 6:23 AM

michal added a child revision: D8010: [native] Use search users in chat composer.May 29 2023, 6:41 AM

michal added a child revision: D8011: [web] Change navInfo.selectedUserList to contains user infos.May 29 2023, 6:52 AM

ashoat added inline comments.May 29 2023, 6:58 AM

lib/shared/search-utils.js
35–42 ↗	(On Diff #27154)	At this point we probably should refactor this function to take an object of params instead of a list of them

Looks good but address @ashoat's comment before landing

lib/shared/search-utils.js
120–123 ↗	(On Diff #27154)	shouldn't you modify this condition to return true when `!containingThreadInfo && !userInfo.relationshipStatus`? (`relationshipStatus` is not defined for `GlobalAccountUserInfo`)

This revision is now accepted and ready to land.May 30 2023, 9:41 AM

Change the function to take a object of params, fix

lib/shared/search-utils.js
120–123 ↗	(On Diff #27154)	Fair point, not sure why flow is returning an error here (flow playground). In any case this should never happen because `GlobalAccountUserInfo` will only exist when `text !=== ''` (there's a search). I've changed it to `userInfo?.relationshipStatus`.