Page MenuHomePhabricator
Differential D14462

[lib] Introduce delete message permissions
ClosedPublic
Actions

Authored by tomek on Mar 19 2025, 7:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 9, 4:21 AM
Unknown Object (File)
Fri, Apr 4, 12:27 PM
Unknown Object (File)
Fri, Apr 4, 9:47 AM
Unknown Object (File)
Wed, Apr 2, 4:32 AM
Unknown Object (File)
Tue, Apr 1, 10:03 PM
Unknown Object (File)
Mon, Mar 31, 12:30 PM
Unknown Object (File)
Mon, Mar 31, 11:22 AM
Unknown Object (File)
Fri, Mar 28, 7:53 PM
View All 20 Files
Subscribers
None

Details

Reviewers
kamil
ashoat
Commits
rCOMM7e031e9590fa: [lib] Introduce delete message permissions
Summary

Add new permissions

https://linear.app/comm/issue/ENG-10321/create-constants-and-update-default-permissions
https://linear.app/comm/issue/ENG-10406/allow-managing-new-permissions-in-the-ui

Test Plan

Tested in combination with the next diff. Checked if the permissions appear in the manage role UI.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek created this revision.Mar 19 2025, 7:46 AM
tomek added a child revision: D14463: [lib] Add a migration that adds message delete permissions.Mar 19 2025, 7:52 AM
Harbormaster completed remote builds in B33768: Diff 47460.Mar 19 2025, 8:05 AM
tomek requested review of this revision.Mar 19 2025, 8:05 AM
ashoat accepted this revision.Mar 19 2025, 9:38 AM

I don't remember all the places where we need to add a new permission. Wondering if you modelled this on another diff that introduced a different permission – if so, would be helpful to share a link to that diff.

This revision is now accepted and ready to land.Mar 19 2025, 9:38 AM
ashoat added inline comments.Mar 19 2025, 9:43 AM
lib/types/thread-permission-types.js
448 ↗(On Diff #47460)

Typo: capital L

tomek updated this revision to Diff 47574.Wed, Mar 26, 11:52 AM

Fix typo and stop filtering out the permission

tomek added a comment.Wed, Mar 26, 11:52 AM
In D14462#401685, @ashoat wrote:

I don't remember all the places where we need to add a new permission. Wondering if you modelled this on another diff that introduced a different permission – if so, would be helpful to share a link to that diff.

Based it partially on https://phab.comm.dev/D12062, but also checked all the places where threadPermissions.EDIT_MESSAGE was used, as the new permissions should behave quite similarly. There is one place I'm not sure of: threadPermissionsRemovedForGenesisMembers - we include threadPermissions.EDIT_MESSAGE there, but I'm not sure why. In the most recent revision I didn't add the new permissions there.

Harbormaster failed remote builds in B33846: Diff 47574!Wed, Mar 26, 12:00 PM
Harbormaster completed remote builds in B33846: Diff 47574.Wed, Mar 26, 12:18 PM
ashoat added a comment.Wed, Mar 26, 9:10 PM
In D14462#402560, @tomek wrote:

There is one place I'm not sure of: threadPermissionsRemovedForGenesisMembers - we include threadPermissions.EDIT_MESSAGE there, but I'm not sure why. In the most recent revision I didn't add the new permissions there.

Interesting. I guess the thinking was if you can't send a message, you shouldn't be able to edit a message? I think it doesn't really matter, since you can only edit your own messages anyways. Same thing goes for deleting messages, so it's okay to omit.

tomek retitled this revision from [lib] Introduce new permissions to [lib] Introduce delete message permissions.Thu, Apr 3, 8:12 AM
tomek added a parent revision: D14565: [keyserver] Introduce checkThreadsOr.Thu, Apr 10, 8:05 AM
Closed by commit rCOMM7e031e9590fa: [lib] Introduce delete message permissions (authored by tomek). · Explain WhyTue, Apr 15, 2:11 AM
This revision was automatically updated to reflect the committed changes.
tomek added a commit: rCOMM7e031e9590fa: [lib] Introduce delete message permissions.

Revision Contents
Changeset List

Diff 47777

View Options

lib/permissions/minimally-encoded-thread-permissions-test-data.js

Show First 20 Lines • Show All 104 Lines • ▼ Show 20 Linestype Props = {
+decompressSocketMessage: CompressedData => string, +decompressSocketMessage: CompressedData => string,
// Redux dispatch functions // Redux dispatch functions
+dispatch: Dispatch, +dispatch: Dispatch,
+dispatchActionPromise: DispatchActionPromise, +dispatchActionPromise: DispatchActionPromise,
// async functions that hit server APIs // async functions that hit server APIs
+socketCrashLoopRecovery?: () => Promise<void>, +socketCrashLoopRecovery?: () => Promise<void>,
// keyserver olm sessions specific props // keyserver olm sessions specific props
+getInitialNotificationsEncryptedMessage?: ( +getInitialNotificationsEncryptedMessage?: (
keyserverID: string,
options?: ?InitialNotifMessageOptions, options?: ?InitialNotifMessageOptions,
) => Promise<string>, ) => Promise<string>,
}; };
type State = { type State = {
+inflightRequests: ?InflightRequests, +inflightRequests: ?InflightRequests,
}; };
class Socket extends React.PureComponent<Props, State> { class Socket extends React.PureComponent<Props, State> {
state: State = { state: State = {
▲ Show 20 Lines • Show All 700 Lines • Show Last 20 Lines
View Options

lib/permissions/minimally-encoded-thread-permissions.js

Show First 20 Lines • Show All 104 Lines • ▼ Show 20 Linestype Props = {
+decompressSocketMessage: CompressedData => string, +decompressSocketMessage: CompressedData => string,
// Redux dispatch functions // Redux dispatch functions
+dispatch: Dispatch, +dispatch: Dispatch,
+dispatchActionPromise: DispatchActionPromise, +dispatchActionPromise: DispatchActionPromise,
// async functions that hit server APIs // async functions that hit server APIs
+socketCrashLoopRecovery?: () => Promise<void>, +socketCrashLoopRecovery?: () => Promise<void>,
// keyserver olm sessions specific props // keyserver olm sessions specific props
+getInitialNotificationsEncryptedMessage?: ( +getInitialNotificationsEncryptedMessage?: (
keyserverID: string,
options?: ?InitialNotifMessageOptions, options?: ?InitialNotifMessageOptions,
) => Promise<string>, ) => Promise<string>,
}; };
type State = { type State = {
+inflightRequests: ?InflightRequests, +inflightRequests: ?InflightRequests,
}; };
class Socket extends React.PureComponent<Props, State> { class Socket extends React.PureComponent<Props, State> {
state: State = { state: State = {
▲ Show 20 Lines • Show All 700 Lines • Show Last 20 Lines
View Options

lib/permissions/minimally-encoded-thread-permissions.test.js

Show First 20 Lines • Show All 104 Lines • ▼ Show 20 Linestype Props = {
+decompressSocketMessage: CompressedData => string, +decompressSocketMessage: CompressedData => string,
// Redux dispatch functions // Redux dispatch functions
+dispatch: Dispatch, +dispatch: Dispatch,
+dispatchActionPromise: DispatchActionPromise, +dispatchActionPromise: DispatchActionPromise,
// async functions that hit server APIs // async functions that hit server APIs
+socketCrashLoopRecovery?: () => Promise<void>, +socketCrashLoopRecovery?: () => Promise<void>,
// keyserver olm sessions specific props // keyserver olm sessions specific props
+getInitialNotificationsEncryptedMessage?: ( +getInitialNotificationsEncryptedMessage?: (
keyserverID: string,
options?: ?InitialNotifMessageOptions, options?: ?InitialNotifMessageOptions,
) => Promise<string>, ) => Promise<string>,
}; };
type State = { type State = {
+inflightRequests: ?InflightRequests, +inflightRequests: ?InflightRequests,
}; };
class Socket extends React.PureComponent<Props, State> { class Socket extends React.PureComponent<Props, State> {
state: State = { state: State = {
▲ Show 20 Lines • Show All 700 Lines • Show Last 20 Lines
View Options

lib/permissions/thread-permissions.js

Show First 20 Lines • Show All 104 Lines • ▼ Show 20 Linestype Props = {
+decompressSocketMessage: CompressedData => string, +decompressSocketMessage: CompressedData => string,
// Redux dispatch functions // Redux dispatch functions
+dispatch: Dispatch, +dispatch: Dispatch,
+dispatchActionPromise: DispatchActionPromise, +dispatchActionPromise: DispatchActionPromise,
// async functions that hit server APIs // async functions that hit server APIs
+socketCrashLoopRecovery?: () => Promise<void>, +socketCrashLoopRecovery?: () => Promise<void>,
// keyserver olm sessions specific props // keyserver olm sessions specific props
+getInitialNotificationsEncryptedMessage?: ( +getInitialNotificationsEncryptedMessage?: (
keyserverID: string,
options?: ?InitialNotifMessageOptions, options?: ?InitialNotifMessageOptions,
) => Promise<string>, ) => Promise<string>,
}; };
type State = { type State = {
+inflightRequests: ?InflightRequests, +inflightRequests: ?InflightRequests,
}; };
class Socket extends React.PureComponent<Props, State> { class Socket extends React.PureComponent<Props, State> {
state: State = { state: State = {
▲ Show 20 Lines • Show All 700 Lines • Show Last 20 Lines
View Options

lib/types/thread-permission-types.js

Loading...