Page MenuHomePhabricator
Differential D14462

[lib] Introduce new permissions
AcceptedPublic
Actions

Authored by tomek on Wed, Mar 19, 7:46 AM.
Tags
None
Referenced Files
F5111626: D14462.diff
Wed, Apr 2, 4:32 AM
F5110088: D14462.diff
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
Unknown Object (File)
Fri, Mar 28, 5:03 AM
Unknown Object (File)
Wed, Mar 26, 8:49 PM
Unknown Object (File)
Wed, Mar 26, 2:47 PM
View All 17 Files
Subscribers
None

Details

Reviewers
kamil
ashoat
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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek created this revision.Wed, Mar 19, 7:46 AM
tomek added a child revision: D14463: [lib] Add a migration that adds new permissions.Wed, Mar 19, 7:52 AM
Harbormaster completed remote builds in B33768: Diff 47460.Wed, Mar 19, 8:05 AM
tomek requested review of this revision.Wed, Mar 19, 8:05 AM
ashoat accepted this revision.Wed, Mar 19, 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.Wed, Mar 19, 9:38 AM
ashoat added inline comments.Wed, Mar 19, 9:43 AM
lib/types/thread-permission-types.js
448

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.

Revision Contents
Changeset List

Diff 47460

View Options

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

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines
export type FFmpegStatistics = { export type FFmpegStatistics = {
// seconds of video being processed per second // seconds of video being processed per second
+speed: number, +speed: number,
// total milliseconds of video processed so far // total milliseconds of video processed so far
+time: number, +time: number,
// total result file size in bytes so far // total result file size in bytes so far
+size: number, +size: number,
+videoQuality: number,
+videoFrameNumber: number,
+videoFps: number,
+bitrate: number,
}; };
export type TranscodeVideoMediaMissionStep = { export type TranscodeVideoMediaMissionStep = {
+step: 'video_ffmpeg_transcode', +step: 'video_ffmpeg_transcode',
+success: boolean, +success: boolean,
+exceptionMessage: ?string, +exceptionMessage: ?string,
+time: number, // ms +time: number, // ms
+returnCode: ?number,
+newPath: ?string, +newPath: ?string,
+stats: ?FFmpegStatistics, +stats: ?FFmpegStatistics,
}; };
export type VideoGenerateThumbnailMediaMissionStep = { export type VideoGenerateThumbnailMediaMissionStep = {
+step: 'video_generate_thumbnail', +step: 'video_generate_thumbnail',
+success: boolean, +success: boolean,
+time: number, // ms +time: number, // ms
▲ Show 20 Lines • Show All 698 Lines • Show Last 20 Lines
View Options

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

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines
export type FFmpegStatistics = { export type FFmpegStatistics = {
// seconds of video being processed per second // seconds of video being processed per second
+speed: number, +speed: number,
// total milliseconds of video processed so far // total milliseconds of video processed so far
+time: number, +time: number,
// total result file size in bytes so far // total result file size in bytes so far
+size: number, +size: number,
+videoQuality: number,
+videoFrameNumber: number,
+videoFps: number,
+bitrate: number,
}; };
export type TranscodeVideoMediaMissionStep = { export type TranscodeVideoMediaMissionStep = {
+step: 'video_ffmpeg_transcode', +step: 'video_ffmpeg_transcode',
+success: boolean, +success: boolean,
+exceptionMessage: ?string, +exceptionMessage: ?string,
+time: number, // ms +time: number, // ms
+returnCode: ?number,
+newPath: ?string, +newPath: ?string,
+stats: ?FFmpegStatistics, +stats: ?FFmpegStatistics,
}; };
export type VideoGenerateThumbnailMediaMissionStep = { export type VideoGenerateThumbnailMediaMissionStep = {
+step: 'video_generate_thumbnail', +step: 'video_generate_thumbnail',
+success: boolean, +success: boolean,
+time: number, // ms +time: number, // ms
▲ Show 20 Lines • Show All 698 Lines • Show Last 20 Lines
View Options

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

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines
export type FFmpegStatistics = { export type FFmpegStatistics = {
// seconds of video being processed per second // seconds of video being processed per second
+speed: number, +speed: number,
// total milliseconds of video processed so far // total milliseconds of video processed so far
+time: number, +time: number,
// total result file size in bytes so far // total result file size in bytes so far
+size: number, +size: number,
+videoQuality: number,
+videoFrameNumber: number,
+videoFps: number,
+bitrate: number,
}; };
export type TranscodeVideoMediaMissionStep = { export type TranscodeVideoMediaMissionStep = {
+step: 'video_ffmpeg_transcode', +step: 'video_ffmpeg_transcode',
+success: boolean, +success: boolean,
+exceptionMessage: ?string, +exceptionMessage: ?string,
+time: number, // ms +time: number, // ms
+returnCode: ?number,
+newPath: ?string, +newPath: ?string,
+stats: ?FFmpegStatistics, +stats: ?FFmpegStatistics,
}; };
export type VideoGenerateThumbnailMediaMissionStep = { export type VideoGenerateThumbnailMediaMissionStep = {
+step: 'video_generate_thumbnail', +step: 'video_generate_thumbnail',
+success: boolean, +success: boolean,
+time: number, // ms +time: number, // ms
▲ Show 20 Lines • Show All 698 Lines • Show Last 20 Lines
View Options

lib/permissions/thread-permissions.js

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines
export type FFmpegStatistics = { export type FFmpegStatistics = {
// seconds of video being processed per second // seconds of video being processed per second
+speed: number, +speed: number,
// total milliseconds of video processed so far // total milliseconds of video processed so far
+time: number, +time: number,
// total result file size in bytes so far // total result file size in bytes so far
+size: number, +size: number,
+videoQuality: number,
+videoFrameNumber: number,
+videoFps: number,
+bitrate: number,
}; };
export type TranscodeVideoMediaMissionStep = { export type TranscodeVideoMediaMissionStep = {
+step: 'video_ffmpeg_transcode', +step: 'video_ffmpeg_transcode',
+success: boolean, +success: boolean,
+exceptionMessage: ?string, +exceptionMessage: ?string,
+time: number, // ms +time: number, // ms
+returnCode: ?number,
+newPath: ?string, +newPath: ?string,
+stats: ?FFmpegStatistics, +stats: ?FFmpegStatistics,
}; };
export type VideoGenerateThumbnailMediaMissionStep = { export type VideoGenerateThumbnailMediaMissionStep = {
+step: 'video_generate_thumbnail', +step: 'video_generate_thumbnail',
+success: boolean, +success: boolean,
+time: number, // ms +time: number, // ms
▲ Show 20 Lines • Show All 698 Lines • Show Last 20 Lines
View Options

lib/types/thread-permission-types.js

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines
export type FFmpegStatistics = { export type FFmpegStatistics = {
// seconds of video being processed per second // seconds of video being processed per second
+speed: number, +speed: number,
// total milliseconds of video processed so far // total milliseconds of video processed so far
+time: number, +time: number,
// total result file size in bytes so far // total result file size in bytes so far
+size: number, +size: number,
+videoQuality: number,
+videoFrameNumber: number,
+videoFps: number,
+bitrate: number,
}; };
export type TranscodeVideoMediaMissionStep = { export type TranscodeVideoMediaMissionStep = {
+step: 'video_ffmpeg_transcode', +step: 'video_ffmpeg_transcode',
+success: boolean, +success: boolean,
+exceptionMessage: ?string, +exceptionMessage: ?string,
+time: number, // ms +time: number, // ms
+returnCode: ?number,
+newPath: ?string, +newPath: ?string,
+stats: ?FFmpegStatistics, +stats: ?FFmpegStatistics,
}; };
export type VideoGenerateThumbnailMediaMissionStep = { export type VideoGenerateThumbnailMediaMissionStep = {
+step: 'video_generate_thumbnail', +step: 'video_generate_thumbnail',
+success: boolean, +success: boolean,
+time: number, // ms +time: number, // ms
▲ Show 20 Lines • Show All 698 Lines • Show Last 20 Lines