Page MenuHomePhabricator

[lib] Remove usage of `memberHasAdminPowers` in `useKeyserverAdmin`
ClosedPublic

Authored by atul on Mar 6 2024, 3:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 2:38 AM
Unknown Object (File)
Tue, Oct 15, 8:04 PM
Unknown Object (File)
Tue, Oct 15, 8:04 PM
Unknown Object (File)
Tue, Oct 15, 8:04 PM
Unknown Object (File)
Tue, Oct 15, 8:04 PM
Unknown Object (File)
Tue, Oct 15, 8:04 PM
Unknown Object (File)
Sun, Oct 13, 8:03 AM
Unknown Object (File)
Sep 4 2024, 3:06 PM
Subscribers

Details

Summary

As part of https://linear.app/comm/issue/ENG-6933/update-memberhasadminpower-logic-to-rely-on-specialrole-field we're going to update memberHasAdminPowers so it determines whether a member is an admin not based on derived permissions, but whether they have an admin role in the community thread.

This will require us to change the signature of memberHasAdminPowers to include additional info (eg ThreadInfo). Before making that change, I wanted to go through all of the usages of memberHasAdminPowers to see how things would need to change at the callsite.

With useKeyserverAdmin, there's really no need to use memberHasAdminPowers since we're dealing with a community thread and can just look for member that has admin role directly. There's no real benefit to using memberHasAdminPowers or a similar utility here, so opted to just use roleHasAdminRole directly on each member role.


Depends on D11262

Test Plan

6957eb.png (1×1 px, 1 MB)

Made sure that keyserver admin appeared as expected before the change (navigating to thread settings on native app + logging): (left)
Made sure that keyserver admin appeared as expected after the change: (right)

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D11263 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul requested review of this revision.Mar 6 2024, 3:32 PM
tomek added inline comments.
lib/shared/user-utils.js
36–38 ↗(On Diff #37907)

Is this comment still valid?

41 ↗(On Diff #37907)

It would be more convenient for member.role to be a RoleInfo instead of a string

This revision is now accepted and ready to land.Mar 7 2024, 1:26 AM
lib/shared/user-utils.js
36–38 ↗(On Diff #37907)

Yes the comment is still valid because we're retrieving the first member that meets members.find(...) criteria. If there are multiple members that meet the criteria, the first one that gets "found" will not necessarily be keyserver admin.

41 ↗(On Diff #37907)

Yeah, agree it would be more convenient in this situation to avoid having to "query" roles with memberID.