Page MenuHomePhabricator

[native] Use search users in chat composer
ClosedPublic

Authored by michal on May 29 2023, 6:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 8:06 PM
Unknown Object (File)
Sun, Nov 10, 4:05 AM
Unknown Object (File)
Oct 30 2024, 3:24 PM
Unknown Object (File)
Oct 30 2024, 3:24 PM
Unknown Object (File)
Oct 30 2024, 3:24 PM
Unknown Object (File)
Oct 30 2024, 3:24 PM
Unknown Object (File)
Oct 30 2024, 3:24 PM
Unknown Object (File)
Oct 30 2024, 3:24 PM
Subscribers

Details

Summary

Uses search users endpoint in native chat composer. To handle that we might now have to have these users in the userStore now user-list returns whole user info instead of just an id. In other places where this is used I just extract the user id.

Depends on D8009

Test Plan

Check if when searching in chat composer, there are users not in userStore displayed. Try clicking them and check if they are added correctly. Try selecting someone else and then selecting someone not in user store, and check if the "you have to friend them first" alert appears.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Looks good but check comments from D8013 related to shared code

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

Extracted the shared code (here and not in D8013, because that's how I have my git stack set up).

kamil requested changes to this revision.Jun 5 2023, 2:55 AM

Two questions about handling fetching results form the backend.
(Sorry for not asking about this in the first review cycle)

lib/shared/search-utils.js
263–280 ↗(On Diff #27393)

Wondering, if we do not have debouncing or throttling, we might create multiple promises in a short period of time, what if they will be resolved in a different order than started? Is it okay, or should we create/use some mechanism to ensure that the result in the state will always be the result of the last promise?

268 ↗(On Diff #27393)

What if this will throw? It looks like failed calls have no reflection in the reducers' logic. Wondering what will be the best handling of this, maybe setting serverSearchResults to empty array?

This revision now requires changes to proceed.Jun 5 2023, 2:55 AM

Catch errors

lib/shared/search-utils.js
263–280 ↗(On Diff #27393)

That's how it works in other places, and is also mentioned/ tracked in the https://linear.app/comm/issue/ENG-1327, so it's probably fine here for now.

This revision is now accepted and ready to land.Jun 14 2023, 3:40 AM