Details

Reviewers

atul
inka
ashoat
tomek

Commits

rCOMM37a9f269f73e: [lib] introduce createPendingPersonalThread function

Summary

This diff introduces a helper function called createPendingPersonalThread. At the moment we are going to use this function if we search for a user that we don't have a preexisting shared personal thread yet, but we will also need to do the same thing for user profiles (if we view a user profile that we don't have a preexisting shared personal thread yet).

Depends on D9120

Test Plan

flow and confirmed that there are no regressions with the search experience when I try and search for a user that I don't already have a thread with yet

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Sep 19 2023, 3:09 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 19 2023, 3:09 PM

Harbormaster completed remote builds in B22745: Diff 31288.Sep 19 2023, 3:27 PM

ginsu requested review of this revision.Sep 19 2023, 3:27 PM

No reviewers on this

ginsu edited the summary of this revision. (Show Details)Sep 19 2023, 3:30 PM

ginsu added reviewers: atul, inka.

simplify

Harbormaster completed remote builds in B22746: Diff 31289.Sep 19 2023, 3:55 PM

remove export

Harbormaster completed remote builds in B22761: Diff 31314.Sep 20 2023, 9:39 AM

ginsu mentioned this in D9247: [lib] introduce useUserProfileThreadInfo hook.Sep 20 2023, 9:43 AM

ginsu edited the summary of this revision. (Show Details)Sep 20 2023, 10:07 AM

ginsu added a parent revision: D9120: [native] update UserProfileBottomSheet to get a userInfo from a userID.

ginsu added a child revision: D9204: [lib] introduce threadInfosSelectorForThreadType selector.

ginsu mentioned this in D9204: [lib] introduce threadInfosSelectorForThreadType selector.Sep 20 2023, 12:12 PM

Adding @ashoat and @tomek here since I don't have a ton of context on pending threads

ashoat requested changes to this revision.Sep 20 2023, 2:28 PM

ashoat added inline comments.

lib/shared/thread-utils.js
420 ↗	(On Diff #31314)	There's something subtly different here: you're stripping any properties other than `id` and `username` from the `pendingPersonalThreadUserInfo` that you pass in as part of `members` to `createPendingThread`. Previously, the `UserIDAndUsername` would be passed in whole-sale. Can you explain / justify this change?

This revision now requires changes to proceed.Sep 20 2023, 2:28 PM

ginsu requested review of this revision.Sep 21 2023, 10:41 AM

ginsu added inline comments.

lib/shared/thread-utils.js
420 ↗	(On Diff #31314)	Sorry I can see how this is concerning/I should have called this out prior to review... Anyways, in the `createPendingThread` function for `members` we only care about the `id` and `username` fields since we recreate `userInfos` object within that function using only those fields. In other places where we use `members` in `createPendingThread` we only grab the `id`: https://github.com/CommE2E/comm/blob/master/lib/shared/thread-utils.js#L333 https://github.com/CommE2E/comm/blob/master/lib/shared/thread-utils.js#L339 https://github.com/CommE2E/comm/blob/master/lib/shared/thread-utils.js#L370 Since we aren't accessing any of the other fields for `UserInfo`, it is a personal preference of mine to be very explicit and only pass in what is needed instead of passing in an entire object. I feel like it also makes the function more clear that these are the exact parameters that are needed for the function to work. However, if we prefer to use the `UserIDAndUsername` type I can update this diff to fix this