Page MenuHomePhabricator
Differential D14463

[lib] Add a migration that adds new permissions
AcceptedPublic
Actions

Authored by tomek on Wed, Mar 19, 7:52 AM.
Tags
None
Referenced Files
F5062177: D14463.id.diff
Wed, Mar 26, 7:46 PM
Unknown Object (File)
Wed, Mar 26, 2:44 PM
Unknown Object (File)
Wed, Mar 26, 10:21 AM
Unknown Object (File)
Wed, Mar 26, 7:00 AM
Unknown Object (File)
Wed, Mar 26, 4:47 AM
Unknown Object (File)
Tue, Mar 25, 10:20 PM
Unknown Object (File)
Tue, Mar 25, 6:02 PM
Unknown Object (File)
Tue, Mar 25, 1:26 AM
View All 10 Files
Subscribers
None

Details

Reviewers
kamil
ashoat
Summary

All the admins should have both new permissions. All the members should have only DELETE_OWN_MESSAGES permission.

https://linear.app/comm/issue/ENG-10323/add-keyserver-migrations

Depends on D14462

Test Plan

Run the migrations and checked if the permissions were updated in the DB. Also modified roles in the UI and made sure the DB was updated.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Wed, Mar 19, 7:52 AM
Harbormaster completed remote builds in B33769: Diff 47461.Wed, Mar 19, 8:10 AM
tomek requested review of this revision.Wed, Mar 19, 8:10 AM
ashoat accepted this revision.Wed, Mar 19, 9:52 AM
ashoat added inline comments.
keyserver/src/database/migration-config.js
994–995 ↗(On Diff #47461)

It looks like this migration was initially added for a special situation, where we had accidentally removed a permission. I guess that adding a new permission from scratch requires the same stuff as fixing an accidentally removed permission?

keyserver/src/fetchers/thread-fetchers.js
337–338 ↗(On Diff #47461)

If we land this with NEXT_CODE_VERSION, we need to make sure that we land a client migration at the same time. Otherwise you can consider switching to FUTURE_CODE_VERSION

lib/shared/thread-utils.js
836–841 ↗(On Diff #47461)

Does the threadPermissionMembershipPrefixes.MEMBER prefix get stripped before we get there?

This revision is now accepted and ready to land.Wed, Mar 19, 9:52 AM
tomek updated this revision to Diff 47575.Wed, Mar 26, 11:56 AM

Updated permissions filtering logic

keyserver/src/database/migration-config.js
994–995 ↗(On Diff #47461)

Yes, exactly

keyserver/src/fetchers/thread-fetchers.js
337–338 ↗(On Diff #47461)

The client migrations are introduced in the next diff, and I'm going to make sure both diffs land simultaneously.

lib/shared/thread-utils.js
836–841 ↗(On Diff #47461)

After my testing in the next diff I realized that it wasn't stripped. Updated the approach to filter out all the permissions based on the suffix which should work regardless of the prefixes.

tomek added a child revision: D14505: [lib] Migrate permissions on clients.Wed, Mar 26, 12:06 PM
Harbormaster completed remote builds in B33847: Diff 47575.Wed, Mar 26, 12:31 PM

Revision Contents
Changeset List

PathSize
keyserver/
src/
database/
121 lines
fetchers/
13 lines
lib/
shared/
7 lines

Diff 47575

View Options

keyserver/src/database/migration-config.js

Show First 20 Lines • Show All 119 Lines • ▼ Show 20 LinesversionBroadcast.onmessage = (event: MessageEvent) => {
} }
}; };
// Start initializing the shared worker immediately // Start initializing the shared worker immediately
void getCommSharedWorker(); void getCommSharedWorker();
const queryClient = new QueryClient(); const queryClient = new QueryClient();
const wagmiConfig = getWagmiConfig({ includeInjectedWallet: true }); const wagmiConfig = getWagmiConfig([
'injected',
'rainbow',
'metamask',
'coinbase',
'walletconnect',
]);
type BaseProps = { type BaseProps = {
+location: { +location: {
+pathname: string, +pathname: string,
... ...
}, },
}; };
type Props = { type Props = {
▲ Show 20 Lines • Show All 432 Lines • Show Last 20 Lines
View Options

keyserver/src/fetchers/thread-fetchers.js

Show First 20 Lines • Show All 119 Lines • ▼ Show 20 LinesversionBroadcast.onmessage = (event: MessageEvent) => {
} }
}; };
// Start initializing the shared worker immediately // Start initializing the shared worker immediately
void getCommSharedWorker(); void getCommSharedWorker();
const queryClient = new QueryClient(); const queryClient = new QueryClient();
const wagmiConfig = getWagmiConfig({ includeInjectedWallet: true }); const wagmiConfig = getWagmiConfig([
'injected',
'rainbow',
'metamask',
'coinbase',
'walletconnect',
]);
type BaseProps = { type BaseProps = {
+location: { +location: {
+pathname: string, +pathname: string,
... ...
}, },
}; };
type Props = { type Props = {
▲ Show 20 Lines • Show All 432 Lines • Show Last 20 Lines
View Options

lib/shared/thread-utils.js

Show First 20 Lines • Show All 119 Lines • ▼ Show 20 LinesversionBroadcast.onmessage = (event: MessageEvent) => {
} }
}; };
// Start initializing the shared worker immediately // Start initializing the shared worker immediately
void getCommSharedWorker(); void getCommSharedWorker();
const queryClient = new QueryClient(); const queryClient = new QueryClient();
const wagmiConfig = getWagmiConfig({ includeInjectedWallet: true }); const wagmiConfig = getWagmiConfig([
'injected',
'rainbow',
'metamask',
'coinbase',
'walletconnect',
]);
type BaseProps = { type BaseProps = {
+location: { +location: {
+pathname: string, +pathname: string,
... ...
}, },
}; };
type Props = { type Props = {
▲ Show 20 Lines • Show All 432 Lines • Show Last 20 Lines