Details

Reviewers

• jacek
tomek
atul
• abosh
ashoat
• jon

Commits

rCOMMb1ca4f27f09f: [lib, web, native] extract getting the keyserver admin

Summary

This common logic has been exctracted to lib, because I need to use this in yet another place
More context on this part of code: ENG-1707
Related issue: ENG-1700

Test Plan

Checked that web and native launch and name 'ashoat' is displayed in the chat-thread-ancestors component

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

inka created this revision.Aug 26 2022, 7:39 AM

Herald added subscribers: • abosh, ashoat. · View Herald TranscriptAug 26 2022, 7:39 AM

Harbormaster completed remote builds in B11637: Diff 16003.Aug 26 2022, 7:49 AM

inka requested review of this revision.Aug 26 2022, 7:49 AM

inka edited the test plan for this revision. (Show Details)Aug 26 2022, 7:54 AM

inka edited the summary of this revision. (Show Details)Aug 26 2022, 8:02 AM

inka edited the summary of this revision. (Show Details)

One inline question.

lib/shared/user-utils.js
40–46 ↗	(On Diff #16003)	Up to you if you'd like to use this, it looks cleaner, but performance matters more.
native/components/community-pill.react.js
23 ↗	(On Diff #16003)	It looks like whenever we use `getKeyserverAdmin` (here and in `ChatThreadAncestors`), we are using it to get the `username` field out. Could we just return the `username` in `getKeyserverAdmin`? This hack only works as long as there is only one admin, so returning one string for a username shouldn't be an issue until we revert it, right?

This revision now requires changes to proceed.Aug 26 2022, 9:54 AM

• abosh added inline comments.Aug 26 2022, 9:56 AM

lib/shared/user-utils.js
40–46 ↗	(On Diff #16003)	Meant to write `userInfos[admin.id]` instead of `userInfos[member.id]`.

Thanks for extracting this!

native/components/community-pill.react.js
23 ↗	(On Diff #16003)	`() => { return something; }` can just be `() => something` I think returning `?UserInfo` is better. The repeated `?.username` isn't that bad
web/chat/chat-thread-ancestors.react.js
44 ↗	(On Diff #16003)	`() => { return something; }` can just be `() => something`