Page MenuHomePhabricator
Differential D10901

[web] introduce threadMembersSelectorForThread
AbandonedPublic
Actions

Authored by ginsu on Jan 31 2024, 1:49 PM.
Tags
None
Referenced Files
F2143530: D10901.diff
Sat, Jun 29, 5:16 PM
Unknown Object (File)
Fri, Jun 28, 4:16 AM
Unknown Object (File)
Sun, Jun 23, 11:24 PM
Unknown Object (File)
Sun, Jun 23, 11:09 PM
Unknown Object (File)
Thu, Jun 20, 12:47 AM
Unknown Object (File)
Tue, Jun 11, 4:07 PM
Unknown Object (File)
Tue, Jun 11, 10:07 AM
Unknown Object (File)
Apr 8 2024, 11:44 PM
View All 18 Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
inka
Summary

Our new chat member list sidebar organizes the list of members in a chat by their role.

This diff introduces a new selector called threadMembersSelectorForThread and given a threadID, threadMembersSelectorForThread will select all the chat members for that thread and return them in a map where the key is the roleID and the value pair is an array of all the chat members that are part of that role.

The one special case we needed to consider is GENESIS chats. GENESIS chats don't have the list of members populated in the community root, so in these cases we should just use the current thread info to select those thread members

Linear task: https://linear.app/comm/issue/ENG-5977/group-each-member-in-the-community-by-role

Depends on D10900

Test Plan

Confirmed that the members were getting correctly grouped into their role for both community chats and GENESIS chats (the code for the UI will come in a subsequent diff)

Community chat:

Screenshot 2024-01-31 at 4.52.14 PM.png (2×3 px, 1 MB)

GENESIS chat:

Screenshot 2024-01-31 at 4.52.32 PM.png (2×3 px, 954 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Jan 31 2024, 1:49 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJan 31 2024, 1:49 PM
ginsu edited the summary of this revision. (Show Details)Jan 31 2024, 2:03 PM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, inka.
ginsu added a parent revision: D10900: [web] introduce member list sidebar header.
ginsu attached a referenced file: F1103416: Screenshot 2024-01-31 at 4.52.32 PM.png. (Show Details)
ginsu attached a referenced file: F1103415: Screenshot 2024-01-31 at 4.52.14 PM.png. (Show Details)
ginsu edited the test plan for this revision. (Show Details)
ginsu added inline comments.Jan 31 2024, 2:05 PM
lib/selectors/thread-selectors.js
551–558

This part of the selector was inspired by Rohan's useRolesFromCommunityThreadInfo where he does something similar to retrieve the correct role info for each user

https://github.com/CommE2E/comm/blob/master/lib/utils/role-utils.js#L62-L75

Harbormaster completed remote builds in B26381: Diff 36476.Jan 31 2024, 2:08 PM
ginsu requested review of this revision.Jan 31 2024, 2:08 PM
ashoat added inline comments.Jan 31 2024, 2:55 PM
lib/selectors/thread-selectors.js
551–558

Is this is correct for grandchild subchannels of GENESIS? I'm not sure where this is used, but I wonder if you want the parent of the grandchild to be the "base thread" here, instead of the grandchild itself. Or maybe you want to use GENESIS.

Actually, looking at this more, I'm confused why you want to list the members of the community instead of the members of the chat?

ginsu planned changes to this revision.Feb 1 2024, 1:43 AM

There is a flaw in my logic, putting this diff back in my queue until the updates are ready

lib/selectors/thread-selectors.js
551–558

Spent some time looking at this and this is a flaw in my logic. We should be using the members of the chat and not the members of the community.

ginsu abandoned this revision.Feb 1 2024, 3:05 PM

D10919 makes this diff obsolete

ginsu removed a parent revision: D10900: [web] introduce member list sidebar header.Feb 1 2024, 3:05 PM

Revision Contents
Changeset List

PathSize
lib/
selectors/
57 lines

Diff 36476

View Options

lib/selectors/thread-selectors.js

Loading...