Paths

Table of Contentst

[lib] introduce threadInfosSelectorForThreadType selector
ClosedPublic
Actions

Authored by ginsu on Sep 14 2023, 10:29 AM.

Details

Reviewers

atul
inka
ashoat

Commits

rCOMM9e4fe445626b: [lib] introduce threadInfosSelectorForThreadType selector

Summary

The next step for user profiles is to select the threadInfo of the thread between the viewer and the user who's profile is being viewed. This is so we can navigate to this thread, get the relationship prompt, etc.

For user profiles there are two types of thread types we need to consider: PERSONAL (the DM thread between two users) and PRIVATE (thread with only yourself). Since these two thread types will be relevant for us, I created this selector so we could easily only grab those relevant threadInfos

Depends on D9235

Test Plan

Ran the following code in the UserProfileBottomSheet component and confirmed I was returning the correct information

const privateThreadInfosSelector = threadInfosSelectorForThreadType(
  threadTypes.PRIVATE,
);
const privateThreadInfos = useSelector(privateThreadInfosSelector);

const personalThreadInfosSelctor = threadInfosSelectorForThreadType(
  threadTypes.PERSONAL,
);
const personalThreadInfos = useSelector(personalThreadInfosSelctor);

console.log('privateThreadInfos');
console.log(privateThreadInfos);

console.log('personalThreadInfos');
console.log(personalThreadInfos);

Screenshot 2023-09-14 at 2.04.17 PM.png (2×3 px, 1 MB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Sep 14 2023, 10:29 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptSep 14 2023, 10:29 AM

Harbormaster completed remote builds in B22637: Diff 31147.Sep 14 2023, 10:48 AM

ginsu requested review of this revision.Sep 14 2023, 10:48 AM

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

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, inka.

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

ginsu attached a referenced file: F759614: Screenshot 2023-09-14 at 2.04.17 PM.png. (Show Details)

How does the code for search results currently handle this? I think there's a similar need there for getting a "pending or realized" thread for the viewer and a given user. The new selector here makes me wonder if you're reusing as much of that code as possible.

How does the code for search results currently handle this?

Currently the search uses useGlobalThreadSearchIndex hook to create a search index and adds new entries for all the thread infos in the thread store. The "keywords" the search index uses to tokenize the thread infos is based on the thread info name, description, and the usernames of the members of a thread.

Then based off the value of the updatedSearchText that gets passed to the onChangeSearchText it will use that to query the relevant results (which takes the form of a set of thread ids). Then this is used to create the relevant ChatThreadItem in the getThreadListSearchResults function.

So the difference here is that we are not trying to build a ChatThreadItem. The only thing we need for user profiles is a threadInfo and an optional pendingPersonalThreadUserInfo (if the user we are looking at is a user where we don't already have an existing realized thread). Both of these are needed for the navigateToThread hook (for navigating to the thread between the viewer and the user in the profile) and the useRelationshipPrompt hook (for doing relationship actions like friending, unfriending, blocking etc. between the viewer and the user in the profile).

To get this threadInfo needed for the user profiles we could just select all the threadInfos from the thread store and do the necessary steps to find the correct threadInfo and forgo this new selector, but I thought it would be better if we preemptively found the relevant thread infos first (only private and personal thread info types) and did the necessary steps later on a much smaller and focused list (the purpose of this selector).

Once I am unblocked by this nix issue, I have diffs ready for review and they should offer more context and better highlight the differences between what we need for searching for a thread and what we need for user profiles. I hope this helps and lmk if there are any other questions.

There's code that exists for the search experience that handles finding a thread for a given user if it already exists, OR creating a "pending" thread if that thread doesn't exist yet.

We need to do the same thing here.

The fact that you're not touching the existing code is concerning. You should either be leveraging it directly here, or you should be factoring out the shared logic into a helper that is used in both parts.

Barring that, you're almost certainly doing some reimplementation or copy-paste. Can you please update this diff to avoid that? Or otherwise explain what I'm missing here.

This revision now requires changes to proceed.Sep 18 2023, 6:49 AM

ginsu added a child revision: 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: D9235: [lib] introduce createPendingPersonalThread function.

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

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

D9235 factors out the logic in the search experience for creating a "pending" thread if that thread doesn't exist yet. This logic will be shared between the search experience and the user profiles experience.

The logic for finding an existing thread for a given user in the user profiles experience is pretty different from the search experience. The TLDR is the search experience will try and find an existing thread based on a thread id whereas the user profile experience will try and find an existing thread based on a user id

The logic for finding an existing thread for a given user in the search experience goes through all the chatListData, and checks if the theadInfo.id from the chatListData exists in the threadSearchResults set (which comes from the threadSearchIndex and is populated based on the searchText). It then returns a list of all the relevant ChatThreadItems to be rendered.
On the other hand, the logic for finding an existing thread for a given user in the search experience only needs to return a singular UserProfileThreadInfo (which contains a single threadID in the type), and again we find this based on the user id.

The logic for finding an exiting thread for a given user in the search experience lives in the getThreadListSearchResults (https://github.com/CommE2E/comm/blob/master/lib/shared/thread-utils.js#L1339-L1379)
D9247 is a diff where I introduce a hook that handles finding a thread for a given user if it already exists, OR creating a "pending" thread if that thread doesn't exist yet (again the creating a pending thread logic has been factored out) and this diff should offer a lot more context into my approach and should show why we still should introduce this selector.

ginsu requested review of this revision.Sep 20 2023, 12:12 PM

Thanks for explaining!

This revision is now accepted and ready to land.Sep 20 2023, 1:30 PM

rebase before landing

This revision was landed with ongoing or failed builds.Sep 21 2023, 1:45 PM

Closed by commit rCOMM9e4fe445626b: [lib] introduce threadInfosSelectorForThreadType selector (authored by ginsu). · Explain Why

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMM9e4fe445626b: [lib] introduce threadInfosSelectorForThreadType selector.

Harbormaster completed remote builds in B22790: Diff 31354.Sep 21 2023, 2:02 PM