Page MenuHomePhabricator
Differential D10105

[lib] Add optional specialRole field to RoleInfo
ClosedPublic
Actions

Authored by atul on Nov 30 2023, 1:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 5, 2:55 AM
Unknown Object (File)
Wed, Apr 2, 3:44 AM
Unknown Object (File)
Tue, Apr 1, 5:16 PM
Unknown Object (File)
Sun, Mar 23, 12:04 AM
Unknown Object (File)
Wed, Mar 19, 10:58 PM
Unknown Object (File)
Wed, Mar 12, 4:55 AM
Unknown Object (File)
Mar 4 2025, 5:45 AM
Unknown Object (File)
Feb 20 2025, 4:38 AM
View All Files
Subscribers
ashoat

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
rohan updated this revision to Diff 34107.Dec 1 2023, 6:04 AM

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 ↗(On Diff #34107)

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 ↗(On Diff #34107)

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 ↗(On Diff #34107)

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
atul updated this revision to Diff 36719.Feb 6 2024, 10:06 AM

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
atul added a comment.EditedFeb 6 2024, 10:33 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:

  1. Introduce ServerLegacyRoleInfo to be used by keyserver to handle fetching from database, etc
  2. 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.

atul mentioned this in D10968: Rename `LegacyRoleInfo` to `ClientLegacyRoleInfo`.Feb 6 2024, 10:55 AM
atul mentioned this in D10969: [lib] Introduce `ServerLegacyRoleInfo`.Feb 6 2024, 11:08 AM
atul removed a child revision: D10106: [keyserver] Update `fetchRoles(threadID: string)` to fetch special_role.Feb 6 2024, 11:55 AM
atul mentioned this in rCOMM552bf09bda6c: Rename `LegacyRoleInfo` to `ClientLegacyRoleInfo`.Feb 7 2024, 12:36 PM
atul mentioned this in rCOMM1cb9081f4e66: [lib] Introduce `ServerLegacyRoleInfo`.Feb 7 2024, 12:38 PM

Revision Contents
Changeset List

PathSize
lib/
types/
2 lines

Diff 34064

View Options

lib/types/thread-types.js

// @flow // @flow
import * as React from 'react'; import * as React from 'react';
import { assetMetaData } from './asset-meta-data.js'; import { assetMetaData } from './asset-meta-data.js';
import HeroContent from './hero-content.react.js'; import HeroContent from './hero-content.react.js';
import InfoBlock from './info-block.react.js';
ginsuAuthorUnsubmitted
Done
Inline Actions

Even though, there are currently no uses of InfoBlock we don't want to completely remove the component since the plan is to repurpose the component in a subsequent diff

ginsu: Even though, there are currently no uses of `InfoBlock` we don't want to completely remove the…
import css from './landing.css'; import css from './landing.css';
import Picture from './Picture.react.js'; import Picture from './Picture.react.js';
import usePreloadAssets from './use-pre-load-assets.react.js'; import usePreloadAssets from './use-pre-load-assets.react.js';
function AppLanding(): React.Node { function AppLanding(): React.Node {
usePreloadAssets(assetMetaData); usePreloadAssets(assetMetaData);
const [ const [hero] = assetMetaData;
hero,
federated,
customizable,
encrypted,
sovereign,
openSource,
lessNoisy,
] = assetMetaData;
return ( return (
<main className={css.wrapper}> <main className={css.wrapper}>
<div className={hero.imageStyle}> <div className={hero.imageStyle}>
<Picture url={hero.url} alt={hero.alt} /> <Picture url={hero.url} alt={hero.alt} />
</div> </div>
<HeroContent /> <HeroContent />
<InfoBlock {...federated} />
<InfoBlock {...customizable} />
<InfoBlock {...encrypted} />
<InfoBlock {...sovereign} />
<InfoBlock {...openSource} />
<InfoBlock {...lessNoisy} />
</main> </main>
); );
} }
export default AppLanding; export default AppLanding;