[lib] Add optional specialRole field to RoleInfo
ClosedPublic
Actions

Authored by atul on Nov 30 2023, 1:01 PM.

Details

Reviewers

ginsu
tomek
rohan

Commits

rCOMM51398b30f3e5: [lib] Add optional specialRole field to RoleInfo

Summary

We want to introduce a specialRole field to the RoleInfo/LegacyRoleInfo type so we can include it when fetching threads/roles, and going forward perform checks on role.specialRole instead of relying on role.isDefault since this won't extend to checking for other special roles (like Admins).

This diff introduces this field as specialRole?: ?number to prevent flow errors. Towards the end of this stack, I plan to change this type to specialRole: ?number so we always include specialRole, whether it is a number or null.

Part of ENG-5994

Test Plan

Ran flow

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

rohan created this revision.Nov 30 2023, 1:01 PM

rohan held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptNov 30 2023, 1:01 PM

rohan added a child revision: D10106: [keyserver] Update `fetchRoles(threadID: string)` to fetch special_role.Nov 30 2023, 1:04 PM

Harbormaster completed remote builds in B24651: Diff 34064.Nov 30 2023, 1:22 PM

rohan published this revision for review.Nov 30 2023, 1:32 PM

rohan planned changes to this revision.Nov 30 2023, 1:58 PM

Use more specific type for specialRole field

Harbormaster completed remote builds in B24689: Diff 34107.Dec 1 2023, 6:41 AM

tomek accepted this revision.Dec 1 2023, 7:52 AM

tomek added inline comments.

lib/types/thread-types.js
95	Just to make sure: are the type and validator of minimally-encoded versions also updated?

This revision is now accepted and ready to land.Dec 1 2023, 7:52 AM

rohan added inline comments.Dec 1 2023, 11:56 AM

lib/types/thread-types.js
95	Yes they should be. `MinimallyEncodedRoleInfo` uses `LegacyRoleInfo` export type MinimallyEncodedRoleInfo = $ReadOnly<{ ...LegacyRoleInfo, +minimallyEncoded: true, +permissions: $ReadOnlyArray<string>, }>; (cc @atul in case I missed something)

atul added inline comments.Dec 1 2023, 12:37 PM

lib/types/thread-types.js
95	Yup, looks right! Since `MinimallyEncodedRoleInfo` is "derived" from `LegacyRoleInfo`, any changes to `LegacyRoleInfo` should be reflected in `MinimallyEncodedRoleInfo`.

atul commandeered this revision.Feb 5 2024, 12:35 PM

atul edited reviewers, added: rohan; removed: atul.

atul retitled this revision from [lib] Add optional speciaRole field to RoleInfo to [lib] Add optional specialRole field to RoleInfo.Feb 5 2024, 12:48 PM

update before landing

This revision was landed with ongoing or failed builds.Feb 6 2024, 10:17 AM

Closed by commit rCOMM51398b30f3e5: [lib] Add optional specialRole field to RoleInfo (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM51398b30f3e5: [lib] Add optional specialRole field to RoleInfo.

Harbormaster completed remote builds in B26572: Diff 36719.Feb 6 2024, 10:22 AM

I modified this diff before landing to only modify RoleInfo (formerly MinimallyEncodedRoleInfo) since I figured we could ignore LegacyRoleInfo going forward...

However, as I previously realized and then forgot, we DO need the specialRole field LegacyRoleInfo because on the keyserver we're still constructing Legacy "everything" and then POSSIBLY minimally encoding for new clients. So we'll need specialRole in the LegacyRoleInfo that's being initially constructed so it's available when being converted to RoleInfo.

That said, we will never want a LegacyRoleInfo with specialRole on ANY client because that would lead to issues with validation. So what we'll want to do is:

Introduce ServerLegacyRoleInfo to be used by keyserver to handle fetching from database, etc
Based on codeVersion check either

Old clients: strip out specialRole field to go from ServerLegacyRoleInfo -> ClientLegacyRoleInfo
New clients: minimallyEncode* ServerLegacyRoleInfo -> RoleInfo WHILE RETAINING specialRole field.