Page MenuHomePhabricator

[lib] introduce useMembersGroupedByRole
ClosedPublic

Authored by ginsu on Feb 1 2024, 2:36 PM.
Tags
None
Referenced Files
F3551024: D10919.id36822.diff
Thu, Dec 26, 7:09 PM
F3551023: D10919.id36819.diff
Thu, Dec 26, 7:09 PM
F3551020: D10919.id36557.diff
Thu, Dec 26, 7:09 PM
F3551019: D10919.id36556.diff
Thu, Dec 26, 7:09 PM
F3551018: D10919.id36756.diff
Thu, Dec 26, 7:09 PM
Unknown Object (File)
Sun, Dec 15, 11:50 AM
Unknown Object (File)
Mon, Dec 2, 6:15 AM
Unknown Object (File)
Sat, Nov 30, 1:49 AM
Subscribers

Details

Summary

Our new chat member list sidebar organizes the list of members in a chat by their role. This diff introduces a new hook that give a threadInfo will group all the members of that thread by their role and return an array of grouped role infos.

Thankfully rohan created useRolesFromCommunityThreadInfo which handles all the heavy lifting of figuring out the role info for each member in a thread. We can use that to group our members by their role, then sort the roles alphabetically by their role name and return them as an array of GroupedRoleInfos to be consumed later.

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

Depends on D10978

Test Plan

Confirmed that the roles + members for threads matched exaclty the same values that we had in our old thread members modal

Community parent channel:
Old members list modal:

Screenshot 2024-02-01 at 5.57.22 PM.png (2×3 px, 896 KB)

New member list sidebar:

Screenshot 2024-02-01 at 5.59.33 PM.png (2×3 px, 1003 KB)

Community sub-channel:
Old members list modal:

Screenshot 2024-02-01 at 5.57.50 PM.png (2×3 px, 857 KB)

New member list sidebar:

Screenshot 2024-02-01 at 6.00.03 PM.png (2×3 px, 926 KB)

Community sidebar:
Old members list modal:

Screenshot 2024-02-01 at 5.57.37 PM.png (2×3 px, 875 KB)

New member list sidebar:

Screenshot 2024-02-01 at 6.00.24 PM.png (2×3 px, 961 KB)

GENESIS Chat:
Old members list modal:

Screenshot 2024-02-01 at 5.58.00 PM.png (2×3 px, 869 KB)

New member list sidebar:

Screenshot 2024-02-01 at 6.00.49 PM.png (2×3 px, 925 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Feb 1 2024, 2:57 PM
atul requested changes to this revision.EditedFeb 5 2024, 10:45 AM

I would guess that a function named useMembersGroupedByRole would return something like Map<string, $ReadOnlyArray<RelativeMemberInfo>>

I'd expect something like that to be easier to work with. For example, if we had logic that only cared about one of the roles (eg admin), we could avoiding going through every element of the array and just index into the Map.

Could you quickly explain the reasoning for this array of arrays approach? Definitely let me know if I'm missing something here

This revision now requires changes to proceed.Feb 5 2024, 10:45 AM
ginsu edited the summary of this revision. (Show Details)

address comments

Looks good, including suggestion from ChatGPT with simpler syntax inline that you can choose to adopt/ignore depending on what you prefer

lib/utils/role-utils.js
116–124 ↗(On Diff #36756)

Would something like below work?

return React.useMemo(() => {
  const sortedRoles = new Map([...groupByRoleName.entries()].sort((a, b) => a[0].localeCompare(b[0])));
  return sortedRoles;
}, [groupByRoleName]);

(generated with ChatGPT)

This revision is now accepted and ready to land.Feb 7 2024, 11:50 AM
lib/utils/role-utils.js
116–124 ↗(On Diff #36756)

Tried the suggestion, but was getting errors + to me what I had originally is easier to read

This revision was landed with ongoing or failed builds.Feb 7 2024, 4:42 PM
This revision was automatically updated to reflect the committed changes.