Page MenuHomePhabricator
Differential D9513

[web] Call useRoleNamesFromCommunityThreadInfo to help filter Admin members
ClosedPublic
Actions

Authored by rohan on Oct 17 2023, 8:45 AM.
Tags
None
Referenced Files
F2902514: D9513.id32269.diff
Sat, Oct 5, 5:54 PM
F2902510: D9513.id32085.diff
Sat, Oct 5, 5:52 PM
Unknown Object (File)
Sun, Sep 29, 5:46 PM
Unknown Object (File)
Thu, Sep 19, 2:51 AM
Unknown Object (File)
Tue, Sep 17, 5:40 AM
Unknown Object (File)
Tue, Sep 17, 5:40 AM
Unknown Object (File)
Tue, Sep 17, 5:40 AM
Unknown Object (File)
Tue, Sep 17, 5:40 AM
View All Files
Subscribers
tomek
will

Details

Reviewers
ashoat
atul
ginsu
Commits
rCOMM4c1aa7dcb4bf: [web] Call useRoleNamesFromCommunityThreadInfo to help filter Admin members
Summary

In an effort to unify the role name checking logic, I'm using useRoleNamesFromCommunityThreadInfo instead of memberIsAdmin(member, threadInfo) || memberHasAdminPowers(member), and then filtering out all of the role names that are not 'Admins'. This is the only way we have to check for Admins right now, but the isAdminRole work that's coming soon will help make these checks a lot more concrete.

Resolves https://linear.app/comm/issue/ENG-5178/ashoat-shows-up-under-admins-column-but-with-members-pill

Depends on D9430

Test Plan

Confirmed that all Admins show up when filtering out the members in both community roots and subchannels, and that GENESIS doesn't crash

Diff Detail

Repository
rCOMM Comm
Branch
ENG-5178
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan created this revision.Oct 17 2023, 8:45 AM
Herald added subscribers: will, tomek. · View Herald TranscriptOct 17 2023, 8:45 AM
rohan updated this revision to Diff 32085.Oct 17 2023, 8:46 AM

Rename adminsMap to roleNames to be consistent

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 17 2023, 8:57 AM
Harbormaster failed remote builds in B23290: Diff 32084!
Harbormaster failed remote builds in B23291: Diff 32085!
rohan updated this revision to Diff 32086.Oct 17 2023, 9:00 AM

Flow?

Harbormaster completed remote builds in B23292: Diff 32086.Oct 17 2023, 9:21 AM
rohan requested review of this revision.Oct 17 2023, 9:21 AM
atul accepted this revision.Oct 17 2023, 9:30 AM

This is the only way we have to check for Admins right now, but the isAdminRole work that's coming soon will help make these checks a lot more concrete.

Bookmarked this diff to make sure it's updated following isAdminRole work.

This revision is now accepted and ready to land.Oct 17 2023, 9:30 AM
rohan added a comment.Oct 17 2023, 9:43 AM
In D9513#278861, @atul wrote:

Bookmarked this diff to make sure it's updated following isAdminRole work.

Thank you!

ashoat added inline comments.Oct 18 2023, 1:57 PM
web/modals/threads/members/members-modal.react.js
52 ↗(On Diff #32086)

Please use the roleIsAdminRole utility here. By doing this comparison in a "one-off" way you're making it harder to refactor later

rohan updated this revision to Diff 32269.Oct 20 2023, 9:02 AM

Use roleIsAdminRole

This revision was landed with ongoing or failed builds.Oct 20 2023, 9:05 AM
Closed by commit rCOMM4c1aa7dcb4bf: [web] Call useRoleNamesFromCommunityThreadInfo to help filter Admin members (authored by rohan). · Explain Why
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM4c1aa7dcb4bf: [web] Call useRoleNamesFromCommunityThreadInfo to help filter Admin members.
Harbormaster completed remote builds in B23404: Diff 32269.Oct 20 2023, 9:20 AM

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
members/
16 lines

Diff 32084

View Options

web/modals/threads/members/members-modal.react.js

Loading...